base_tree does not bind ancestry, so the merge can contain different content than was reviewed
Found by the cross-model adversarial pass on #521's design, against shipped
code, and reproduced with real git objects rather than argued.
The hole
scripts/merge-pr.sh (#540) enforces sha + candidate_tree + base_tree.
A PR diff is three-dot — it depends on the merge base, not on the two
endpoint trees. The merge base can move while all three enforced fields stay
byte-identical.
Reproduction
- Base commit
B, tree T.
- Feature commit
F1 adds x.
- Feature commit
F2 adds y. PR head is F2.
- Reviewers clear
B...F2 — they read x + y.
- Main then takes
F1 and reverts it. Main's tree returns to exactly T.
- PR head unchanged.
candidate_tree unchanged. base_tree unchanged —
the revert restored the identical tree.
- But the merge base moved
B → F1, so the PR now contributes only y.
Measured with git merge-tree:
- reviewed base tree == live base tree:
4b36dfd…
candidate_tree: 4de2a293… (contains x + y)
- prospective merge tree:
f4e750e2… (contains only y)
Every check at scripts/merge-pr.sh:818 passes. The content that merges is not
the content that was cleared.
Why #540 did not catch this
#540's whole thesis was content identity over SHA identity, and that was
right. But it bound the content at the two endpoints and treated the base as a
tree. The base is a position in a history, and a tree does not carry ancestry.
A revert is the case where those two come apart while looking identical.
Smallest fix
Add base_sha to the clearance artifact and require the live base SHA to
match exactly. More conservative and simpler than the alternative
(merge_base_sha recomputed at merge time alongside base_tree).
Explicitly not a per-round chain — that question belongs to #521 and is
independent of this.
Severity
This is the merge boundary, and the failure is silent: nothing is reported,
the wrong content merges at exit 0. But note the trigger is narrow — it needs
the base to move and return to an identical tree, which in practice means a
revert of a commit the PR also contains. Rare, not impossible.
Verification
Must be proven by mutation like the rest of #540's guards: a fixture where the
merge base has moved but every currently-enforced field matches must merge at
exit 0 before the fix, and be refused after.
Related: #540 (shipped the three-field contract), #521 (whose design pass
surfaced this).
base_treedoes not bind ancestry, so the merge can contain different content than was reviewedFound by the cross-model adversarial pass on #521's design, against shipped
code, and reproduced with real git objects rather than argued.
The hole
scripts/merge-pr.sh(#540) enforcessha+candidate_tree+base_tree.A PR diff is three-dot — it depends on the merge base, not on the two
endpoint trees. The merge base can move while all three enforced fields stay
byte-identical.
Reproduction
B, treeT.F1addsx.F2addsy. PR head isF2.B...F2— they readx+y.F1and reverts it. Main's tree returns to exactlyT.candidate_treeunchanged.base_treeunchanged —the revert restored the identical tree.
B→F1, so the PR now contributes onlyy.Measured with
git merge-tree:4b36dfd…candidate_tree:4de2a293…(containsx + y)f4e750e2…(contains onlyy)Every check at
scripts/merge-pr.sh:818passes. The content that merges is notthe content that was cleared.
Why #540 did not catch this
#540's whole thesis was content identity over SHA identity, and that was
right. But it bound the content at the two endpoints and treated the base as a
tree. The base is a position in a history, and a tree does not carry ancestry.
A revert is the case where those two come apart while looking identical.
Smallest fix
Add
base_shato the clearance artifact and require the live base SHA tomatch exactly. More conservative and simpler than the alternative
(
merge_base_sharecomputed at merge time alongsidebase_tree).Explicitly not a per-round chain — that question belongs to #521 and is
independent of this.
Severity
This is the merge boundary, and the failure is silent: nothing is reported,
the wrong content merges at exit 0. But note the trigger is narrow — it needs
the base to move and return to an identical tree, which in practice means a
revert of a commit the PR also contains. Rare, not impossible.
Verification
Must be proven by mutation like the rest of #540's guards: a fixture where the
merge base has moved but every currently-enforced field matches must merge at
exit 0 before the fix, and be refused after.
Related: #540 (shipped the three-field contract), #521 (whose design pass
surfaced this).