Repository navigation
emrg: a vote naming the revision it read is reported as a claim this clone cannot resolve #2555
Description
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 += 1named_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
- 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.
- 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_claimpins it with an invented sha — and it must keep its wording. named_treeskeeps its semantics unchanged: only atreeis 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.- One answer to "which token is a claim" and one to "what is this token". The token pattern lives in
cast-vote.pyand 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. - 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
argszero commentedon Oct 23, 2026
Handled by #2556
emrg: a revision a review names is not a claim its clone could not re…