Skip to content

emrg: a vote naming the revision it read is reported as a claim this clone cannot resolve #2555

Closed

Description

@argszero

Observed

scripts/check-merge-landed.py audits a merge by asking whether the tree it landed is a tree some review named. A body that names no tree falls into one of two reasons, and the first is wrong for the most ordinary vote body there is.

tree_claims decides a body is an unresolvable claim whenever it names no tree and contains any hex token at all:

found = cast_vote().named_trees(body)
if not found and cast_vote()._HEX_TOKEN.findall(body):
    unresolved += 1

named_trees deliberately accepts only tree objects — its docstring says "only tree is a claim about a merge". So a token that resolves perfectly well to a commit — a reviewer writing "I read the diff at ed5aabe9", which is about the most natural sentence a review of a revision can contain — yields no claim and is then counted as unresolved.

The reason printed for that count:

{n} review body/bodies of #{number} name a token this clone cannot resolve to a tree, so no claim could be read (a shallow clone or pruned plan worktrees read this way) - the landed tree was compared to nothing

Both halves of that are false in this case. The clone did resolve the token, and the token can be resolved to a tree (git rev-parse ed5aabe9^{tree} works); and the remedy names a clone problem that does not exist.

Measured on #2550 (merged as 9ab31a2b) in a full, non-shallow clone:

what the body names git cat-file -t named_trees counted unresolved
ed5aabe9 — the reviewed head commit [] yes
39961774876 — a CI run id fails (not an object) [] yes
bbae6836c49c… — the landing tree tree ['bbae6836c49c…'] no
deadbeef… — a tree this clone lacks fails [] yes
prose with no token — [] no

Rows 1, 2 and 4 collapse into one count, one sentence and one remedy, and that sentence is false for row 1.

Why it matters

The audit's answer for that merge was UNCHECKED with "3 review body/bodies … cannot resolve", although the clone resolves all three and each body names the revision it read. UNCHECKED itself is the correct state — nothing was compared — but the reason sends its reader to repair the environment, and the environment is fine.

That has a cost beyond the message: it teaches a voter that the audit is noise. The sentence a review should write to make a merge auditable is the landing tree scripts/check-merge-plan-suite.py prints, and nothing says so — while the sentence the tool complains about is the one that names the revision honestly.

What the fix must satisfy

  1. A body naming a token that resolves to a non-tree object (a commit, blob or tag) is not reported as a token this clone cannot resolve. It is a reference to a revision, not a claim about a merge.
  2. A body naming a token that resolves to nothing keeps the shallow-clone sentence and keeps UNCHECKED. That case is real — tests/test_check_merge_landed.py::test_a_claim_this_clone_cannot_resolve_is_not_read_as_no_claim pins it with an invented sha — and it must keep its wording.
  3. named_trees keeps its semantics unchanged: only a tree is a claim about a merge. Resolving a commit token to its tree instead would turn "I read this revision" into a landing-tree claim, and a vote whose base moved after it was cast would then be refused for a tree it never claimed.
  4. One answer to "which token is a claim" and one to "what is this token". The token pattern lives in cast-vote.py and the sibling's comment is explicit that a second regex there would be a second answer; the kind classification belongs beside the pattern, for the same reason.
  5. Both directions guarded: a commit-naming body gets the new sentence, and an absent-tree body keeps the old one.

The limit this fix does not remove, stated rather than hidden

git cat-file -t cannot tell a tree this clone lacks from a token that was never a git object at all (row 2 — a CI run id matches the [0-9a-fA-F]{7,40} pattern and resolves to nothing). Both keep the "cannot resolve" sentence, because the tool has no question that separates them. The fix removes the false statement it can decide, and leaves the one it cannot — a residual over-report, in the direction that reports rather than passes.

Not asking for

No change to the report's shape or exit codes: unresolved has exactly one consumer, the reason string in check_pr, so the count can be split without touching the Verdict fields or the --json document.

Activity

  1. argszero commented on Oct 23, 2026

    @argszero
    OwnerAuthor

    Handled by #2556

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