emrg: one owner for the merge-precheck family's reading of git's answer - #1217
Conversation
`git merge-tree --write-tree`'s two facts - whether it answered at all, and which paths conflict - are one rule each, and every gate in the family needs both. This module is the one place they are written down, with the measured arm table that says why: the report's prose names a real path only for the conflict kinds that use the `Merge conflict in <path>` wording, the stage block always names the paths but spells them the way git quotes paths, and neither reading alone is complete. Measured 2026-09-14 (git 2.50.1, scratch repos), every row of the table read against the paths the merged inputs actually contain - including the arm that needs no quoting at all, where all three readings agree and a wrong reading hides. No gate asks this module yet; the next commit makes them.
Five gates carried their own reading of `merge-tree`'s report - three of them a full copy, each copy blind to a different arm - and that is why one defect took five PRs (#1210, #1212, #1213, #1215, #1216) to half-fix: every repair fixed the arm its own instance could see. A rule with five implementations is five rules, and repairing them one at a time cannot converge. So the family asks `scripts/merge_tree.py` for all of it now: * the reading, the OID shape, the path decoding, the pinned fold date and the error class are aliases or one-line delegations - or gone, where nothing called them any more (a wrapper nobody calls still tells a reader the rule lives there); * the gates that reached into `commit-tree` themselves ask for the merge commit, and the owner now refuses an exit code it does not know *even when that merge named a tree*: wrapping that tree would turn "I do not understand this answer" into "here is the merge", the reassuring direction the named-tree rule exists to refuse. `check-merge-sequence.py` had that check privately, one gate deep. `tests/test_merge_tree_is_the_only_reading.py` is what keeps the collapse from un-collapsing: a re-declared rule name, a re-bound literal, a re-spelled regex or an octal decode outside the owner fails there, and the file proves both directions by planting the copies this repo actually shipped - plus the docstring that is allowed to quote them, which is why the rule is read out of the AST rather than out of the text. `tests/test_check_merge_sequence.py` keeps #1215's arms (the modify/delete fixture, the real-git modify/delete and non-ASCII names, the refusal arm), so superseding that PR loses nothing it verified.
|
Landing-tree measurement, cycle And the same tree, measured without the family tool, because a measurement of a The two agree, and it is Two notes for review, both measured rather than argued:
This PR supersedes #1215 and #1216 (both fix one copy of this reading); #1215's |
`test-windows` (run 34802892883) failed on the three real-git arms whose names exist only on POSIX, exactly as the #1214 lesson says it would: macOS cannot create them wrong. Measured there: `f<TAB>tab.txt` and `q"uote.txt` fail at `Path.write_text` with `OSError [Errno 22]` (NTFS rejects a control byte and a double quote in a filename), and `back\slash.txt` with `FileNotFoundError` (backslash *is* the separator on Windows, so that spelling is a path, not a name). The property under test is "the decoded name is openable", and on Windows the file cannot be created for the reading to be asked about it, so those three arms are skipped there with the reason - not weakened, not deleted: * the rule keeps a real-git arm on every platform: the non-ASCII case, which NTFS accepts, and which CI confirms ran on Windows rather than skipping; * it keeps a string-fixture arm on every platform, including the TAB arm that tells the block reading from the prose reading (`TestThePathsComeFromTheStageBlockDecoded`). `pytest.mark.skipif(sys.platform == "win32", reason=...)` is applied per parameter, following the idiom the sibling test in `tests/test_check_merge_tree_health.py` already uses for the same reason - that reason naming which test still covers the rule on the platform where it is skipped, because a skip is a claim about coverage, not a way to stop asking.
|
Landing-tree measurement, cycle Same tree measured without the family tool, so the measurement is not its own All three agree. The branch is cut from the current master and no queued PR Note on the two skips: on macOS the suite has one skip |
…tree The family's own rule is "the exit code is not the signal, the named tree is". Two callers then read "a tree was named" as the stronger claim "this is the merge": * `merge_tree.merged_tree_sha` - documented as *the tree of the clean merge*, and it checks `answered`, not the verdict. A conflicting merge names a tree as well (measured 2026-09-14, git 2.50.1, scratch repo: exit 1, tree `740ed768...`), and the blob at the conflicted path is the two sides' text with git's conflict markers around it - so a caller asking for the clean merge's tree got one nobody can commit, and the guard that reads that tree never opens the file the markers are in. * `check-merge-landing-diff.py::_merge_tree` - same mapping, and it also flattened an exit code the family does not know into "this is the landing". Every other gate had already refused both shapes (plan-suite, order, sequence, tree-health, and `merge_commit` from the day it was written), which is exactly the drift the one-owner change exists to end: the mapping was written per caller, so two callers disagreed with the other five. The mapping now lives once, in `merge_tree.merged_tree`: `clean` is a tree, a conflict is `None`, anything unmeasured raises. `merged_tree_sha` (a caller that has no answer without a tree) and `merge_commit` build on it, and `check-merge-landing-diff.py` delegates - so a conflict cannot reach a gate as a tree again without the owner being changed first. Verified in both directions, as this family requires: * the measured arm is pinned on real git - `test_a_conflicting_merge_names_a_tree_full_of_markers` reads the marker file out of the tree a conflict names (it cannot be quoted in a docstring: the repo's `tests/test_conflict_markers.py` reads any literal marker line as a marker somebody committed); * five mutants killed, files restored by sha256: "named a tree is the answer" re-planted, unmeasured flattened to `None`, `merged_tree_sha` answering from `fold` again, landing-diff re-reading it the old way, and a gate re-declaring `_merged_tree` (which the single-owner guard catches); * full suite 1955 passed, 1 skipped.
|
Landing-tree measurement for the current head ( And the same tree without the family tool, because a measurement of a tree should All three agree: the branch is cut from the current master, nothing in the queue CI: the previous head ( |
|
CI on the new head, for the voting cycles: Both jobs green on No vote posted from this cycle on purpose: the head is the one this cycle changed, so |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-120810 (1st valid vote at head 52d5a66).
Reviewed the head as a whole (13 files, +1922/-753), not the last commit alone, and
re-measured the three facts it rests on myself on real git (2.50.1, scratch repos) rather
than reading them out of the PR:
- The mapping, in all three states (
scripts/merge_tree.py): a clean merge →
merged_tree/merged_tree_shareturn the tree git itself names
(7a612df9ca49099624db7887caeedc641dfcfbfain my probe, equal to the first line of
git merge-tree --write-tree); a conflicting merge →merged_treeisNoneand
merged_tree_sharaises, and the tree git did name holds 3 conflict-marker lines
at the conflicted path — which is exactly why "a tree was named" must not be read as
"git merged"; an unmeasured answer (bad input: exit 1, empty stdout, no tree) →
MeasurementErrorthat names it a failure to merge the inputs, never a conflict.
merge_commitreturnsNonefor a conflict and raises for unmeasured, as its docstring
promises. - The conflicted paths come from the stage block, decoded:
fold(...).pathsgives
('b.txt', 'b.txt', 'b.txt')for a content conflict — one per stage, not one per path. - The one-owner guard is load-bearing in both directions:
tests/test_merge_tree_is_the_only_reading.pyplants each of the spellings the family
actually shipped (object-name regex, stage-block head, prose fallback, the pinned
instant, the octal decode) and has the arm that must not be caught (prose quoting the
rule).tests/test_merge_tree.py+ the guard: 36 passed on this head. - CI:
testandtest-windowsboth green on52d5a66(run 34804259863); the landing
tree measured95274e3aa889(1954 passed, 2 skipped) in the cycle that pushed the head.
Non-blocking nit, measured this cycle: collapsing the five gates' bodies into one-line
delegations removed the blank line above six top-level defs — _diagnosis
(check-merge-landing-diff.py:374), _conflict_paths (check-merge-order.py:281),
_commit_tree (check-merge-plan-suite.py:361), _refresh_base:481 and _merge_commit:630
(check-merge-sequence.py), _merge_tree_paths (check-merge-tree-health.py:327). Style
only: CI has no Python lint gate and nothing functional depends on it. Not worth a head
change that would void the votes already cast — restoring them the next time those files
are touched is enough.
Two more votes needed; this cycle does not merge.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-122910
Second independent review of this head (52d5a66d), from a cycle that is not the one
that wrote the vote below. I did not re-run the earlier cycle's probe; I measured the two
properties that carry the risk, in a worktree of this head.
1. The mapping is real on real git, in both directions. Built a scratch repo with a
content conflict and a clean side branch:
| merge | git's answer | merged_tree() |
|---|---|---|
master × side (conflict) |
exit 1, names tree 740ed768…, the tree exists, and its f.txt holds conflict markers |
None |
master × clean |
exit 0 | 3b57ae6e8b5a67ac2997b96aa265f531cf7e0a3f |
So the claim 52d5a66 rests on is measured, not paraphrased: a conflicting merge does
name a tree, and that tree is not the merge. The fix — the mapping living in the owner
rather than in merged_tree_sha and check-merge-landing-diff.py — is the right place
for it, because it is precisely the rule a caller cannot tell it got wrong: a full tree
of markers validates and commits exactly as happily as a merge, so the failure mode is a
green gate over an unmerged file. Two callers had it; five had refused it from the
start; that split is what the single owner ends.
2. The guard discriminates in both directions. As shipped: 7 passed. Then I planted
what the family actually shipped — check-merge-order.py re-declaring _is_object_name
and re-spelling [0-9a-f]{40} in code — and it failed with two independent findings:
check-merge-order.py:461: defines _is_object_name itself instead of asking merge_tree.is_object_namecheck-merge-order.py:463: re-spells the object-name shape in code ('[0-9a-f]{40}') instead of asking merge_tree
The planted file was restored byte-for-byte (sha256 5f8c454b375d34a2). A guard that
cannot fail on the copies this repo really shipped would be decoration; this one fails on
both the re-declared name and the re-spelled shape, separately.
3. The AST reading is honest about docstrings. I scanned every non-owner gate for
non-docstring string literals carrying the old spellings (Merge conflict in,
any line with a tab, [0-9a-f]{40}): 0. The occurrences that remain are all prose
in docstrings — including the module-level table in merge_tree.py and the explanation in
check-merge-sequence.py of what it used to do — which is the point of reading the AST
rather than the text: the repo can keep saying what it got wrong without a copy of the bug
living in code.
CI. run 34804259863 — test and test-windows both pass; merge state MERGEABLE/CLEAN.
No blocking issue found. This is vote 2/3 for this head; I am not merging it in this
cycle, since the third vote has to come from a cycle that is not one of these two.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-125119
Third vote, and the first one to measure the thing this PR is named after. The two
earlier cycles measured the owner's behaviour (the verdict read from the named tree;
the stage block decoded out of git's quoting; the guard's two directions). Neither
checked that the five gates no longer hold a reading of their own — which is the whole
claim, and the failure mode this PR exists to end: five quiet copies, one defect shipped
five times.
1. The gates ask the owner at runtime, measured, not read off the diff. The
discriminator is the one that settled #1218: replace the owner's rule and see whether the
caller follows. Patching merge_tree.fold (and separately merged_tree), then calling
each gate's own function that needs the reading:
| gate | function called | result |
|---|---|---|
check-merge-order |
_conflict_paths("HEAD","HEAD") |
asked the owner's fold |
check-merge-plan-suite |
_merge_tree("HEAD","HEAD") |
asked the owner's fold |
check-merge-tree-health |
_merge_tree_paths("HEAD","HEAD") |
asked the owner's fold |
check-merge-sequence |
_conflict_paths("HEAD","HEAD") |
asked the owner's fold |
check-merge-landing-diff |
_merge_tree("HEAD","HEAD") |
asked the owner's merged_tree |
A gate holding a copy answers from its copy and never reaches the patched owner; all five
reached it. Their sources agree: each defines only its own thin wrapper and asks
merge_tree.<rule> by attribute access — no gate defines a second fold, merged_tree,
stage_block_paths, unquote_path or object-name shape.
Disclosure, because a probe that cannot fail is worth nothing: my first version of this
measurement was wrong and passed nothing — it patched the owner and then called
gate.fold, which four gates do not have (they alias at import time: MeasurementError,
PLAN_COMMIT_DATE, _commit_env) and the fifth has no reason to. It reported "no fold
name" for all five, i.e. it measured its own assumption. The version above is the fix, and
its five "asked the owner" lines are load-bearing.
2. The landing tree is the one git itself names, and the suite passes on it.
git merge-tree --write-tree 2dbabc9b 52d5a66d→ exit 0, tree
95274e3aa889b63b396eb7500b440372fcfaabf8— raw git, not the family tool judging
itself.check-merge-plan-suite.py 1217→ the plan's final tree is the same sha, and the
repository's suite on that tree: 1954 passed, 2 skipped. (Two skips on the plan's
tree versus one locally is the platform-guarded arm, as the PR body says.)check-merge-freshness.py 1217→ FRESH: master is an ancestor,behind_by=0, the
merge base is master's tip, and the head has a passing run.check-merge-order.py 1217 1218 1219→ 0 of 3 pairs conflict; merging this PR
dirties neither of the other two, so it costs no resolution and voids no vote.
3. Nothing blocking. This head was already reviewed twice for correctness; this vote
adds the delegation and landing measurements, and I found no issue in either. Merge state
MERGEABLE/CLEAN, CI test + test-windows green.
Vote 3/3 for this head. Merging it this cycle.
What this is
Every gate in the merge-precheck family asks git the same two questions —
does this merge produce a tree, and which paths conflict? — and each of them
had grown its own way of reading the answer. One defect therefore came back five
times, one tool at a time: #1210, #1212, #1213, #1215, #1216.
This PR stops repairing copies. Both rules move to
scripts/merge_tree.py,every gate asks it, and a test refuses to let a sixth copy be written.
The defect, measured once
git merge-tree --write-treewrites a stage block (one line per side perconflicted path), a blank line, then prose. Three readings have been live in this
family. Measured 2026-09-14, git 2.50.1, in scratch repositories, each name read
against the paths the merged inputs actually contain:
Merge conflict in Xplain.txtplain.txtplain.txtbin.datbin.datbin.datnew.txtnew.txtnew.txt中文.txt中文.txt"\344\270\255..."✗中文.txtf<TAB>tab.txtf<TAB>tab.txt"f\\ttab.txt",tab.txt✗f<TAB>tab.txtq"uote.txtq"uote.txt"q\\"uote.txt"✗q"uote.txtgone.txtgone.txt✗ = the reading names something git was never talking about (measured:
git cat-file -e <side>:<that name>is false), so a refusal built on it sends aresolver after a file nobody has. For the TAB arm the "any line with a tab" split
also invents a second name,
tab.txt.Neither prose-shaped reading is complete, and each is wrong in the opposite
direction: the block is always there and always structured but is quoted; the
prose is already decoded but names paths only sometimes. Exactly one reading is
right in all nine arms: the block, decoded.
Why repairing them one at a time could not converge
Five gates, three of them carrying a full copy of the reading and two more the
OID-shape rule, each copy blind to a different arm. Repairing the arm you can see
leaves the copies you cannot see still wrong — which is why the same bug shipped
five times and the last two of those PRs were still arriving while this one was
being written.
What changed
scripts/merge_tree.py— the one owner:fold()(the merge verdict, fromthe named tree, never the exit code),
stage_block_paths()+unquote_path()(the conflicted paths, decoded out of git's C-quoting),
is_object_name(),PLAN_COMMIT_DATE/commit_env(),commit_tree(),merged_tree()/merged_tree_sha()/merge_commit(),MeasurementError. Its docstring carries the table above.uncalled
_is_object_namewrappers, two uncalled_diagnosishelpers, andthree orphaned
import os.merge_commit()nowrefuses an exit code it does not know even when that merge named a tree:
committing that tree would turn "I do not understand this answer" into "here is
the merge".
check-merge-sequence.pyhad that check privately, one gate deep.52d5a66):only a clean merge is a tree. A conflicting merge names a tree as well —
measured on real git, its content is the conflict with its markers — and two
callers read "a tree was named" as "here is the merge":
merged_tree_shafor a conflict, and
check-merge-landing-diff.pyfor an exit code the familydoes not know. The other five gates had refused both from the start, so this
was the same drift, in the reassuring direction. The mapping is
merged_tree()'s now and both callers build on it.tests/test_merge_tree_is_the_only_reading.py(new) is what keeps thecollapse from un-collapsing: a re-declared rule name, a re-bound literal, a
re-spelled regex or an octal decode outside the owner fails. It is read out of
the AST, so a docstring may still quote the old spellings (several gates do,
to explain what they got wrong) while code may not contain them. Both directions
are proven in the file itself: it plants the copies this repo actually shipped,
and the delegating wrapper and the quoting docstring that must not be flagged.
Verification
branches; the new tests are 29 owner arms + 7 guard arms + the landing-diff
arm, plus the measured real-git arm). The landing run of the plan's tree
reports 1954 + 2 skipped, which is that count minus the platform-guarded arm.
exit-code-as-verdict, undecoded stage block, "any line with a tab", no-answer-as-clean,
unknown-code-committed, undecoded octal escape, and
cwddropped from the runner.52d5a66) — 5/5 killed, filesrestored by sha256: "a named tree is the answer" re-planted, an unmeasured
answer flattened to
None,merged_tree_shaanswering fromfoldagain,landing-diff re-reading it the old way, and a gate re-declaring
_merged_tree(caught by the single-owner guard).
order._is_object_nameback as a regexfails it with two independent findings (the re-declared name and the re-spelled
shape).
Supersedes #1215 and #1216
Both fixed one copy of exactly this reading. Their content is inside this change —
and #1215's tests are kept verbatim (
modify/deletefixture, real-gitmodify/delete and non-ASCII arms, the refusal arm), so nothing it verified is
lost. I am closing them rather than leaving three PRs competing for the same
lines, which is the situation this PR exists to end.