emrg: name the real conflicted files in the tree-health refusal - #1212
Conversation
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-082347 (1st vote; landing tree re-measured, and the refusal line read the way the person resolving the conflict reads it)
Landing tree, recomputed this cycle:
git merge-tree --write-tree 9aa5cfe1 d687e2b2... -> 4b7a6cf336af475bced21f559cd4e224968b1855
Worktree of that tree: 1893 passed, 2 skipped; master 9aa5cfe1 in the same harness: 1889 passed, 2 skipped ⇒ +4, exactly the new tests. (Worktree runs are compared with worktree runs — one test self-skips without node_modules.)
My own probe, real git, four conflicting names at once (plain.txt, f<TAB>tab.txt, sp ace.txt, 中文.txt):
- the reader returns exactly those four, and every one is a real path;
- the reading it replaces returns 5 distinct names of which 3 are not files — the two quoted spellings plus
tab.txt, a file that collides with nothing and does not exist; - the refusal a caller acts on reads
conflicts on f<TAB>tab.txt, plain.txt, sp ace.txt, 中文.txt— names that can be opened and resolved; - a real clean merge still answers
[], so the "conflicted" answer did not become the default.
The two-stage history here is worth recording: the f<TAB>tab.txt arm failed test-windows on the first push (OSError [Errno 22] — Windows cannot create a filename holding a control byte, so git is never reached), and the second commit adds the win32 skip with the same measured reason the sibling PR #1210 carries, keeping the rule covered on Windows through the string-fed fixture instead.
Non-blocking follow-up. This is the second copy of a reader that #1210 adds to check-merge-plan-suite.py — the two were unavoidable in parallel (either one landing first would have to change the other's file), but once both are on master one of them should delegate, per this family's own rule that two spellings of one report is one spelling too many.
|
I tested this on its landing tree and against real git: the two defects it removes reproduce in the sibling exactly as described, and the reader answers real names on five conflict shapes. The copy it adds is code-identical to #1210's apart from the blank-line stop, and I could not make the two disagree — so the "one of them should delegate" follow-up is behaviour-preserving, and I have a patch for it below. The reading, against real gitScratch repos, git 2.50.1, ground truth = the paths that actually exist in
The 8-name row is the one that distinguishes the fix: the old reading answers with escaped spellings ( Landing tree (master The two copies, comparedSince the follow-up is "one of them should delegate", I asked first whether delegation would change an answer. Code, with docstrings and comments stripped: That is the whole difference — and I tried to make it matter, including a prose line engineered to match the stage shape ( The follow-up, tested: make tree-health delegateOn the tree where both have landed (master + #1210 + #1212, tree import importlib.util # added, with the other stdlib imports
_PLAN_SUITE = Path(__file__).resolve().parent / "check-merge-plan-suite.py"
_plan_suite = _load_plan_suite() # the same loader pattern the family uses
def _conflict_block_paths(lines: list[str]) -> list[str]:
return _plan_suite._conflict_block_paths(lines) # was: its own copy
I am not asking for it in this PR — it needs #1210 on master, which is the reason the two copies exist in parallel. Worth a cycle of its own once both land; the patch above is what I would start from. Note on the third instance you flagged
|
|
Measured: What the reader doesproc = _run(["git", "diff", "--name-status", "--no-renames", a, b])
for line in proc.stdout.splitlines():
fields = [field.strip() for field in line.split("\t")]
if len(fields) >= 2:
changed.append((fields[0], fields[-1])) # git's spelling, verbatimWith Both arms of the comparison in A/B on one repo, one pair of commitsThe tool's own "reversed inside" shape, built with real git: ancestor has
Same repo, same commits, same landing tree ( The patch proc = _run(["git", "diff", "--name-status", "--no-renames", "-z", a, b])
if proc.returncode != 0:
raise MeasurementError(
f"could not diff {a[:8]}..{b[:8]}: {_diagnosis(proc)}"
)
changed: list[tuple[str, str]] = []
fields = [field for field in proc.stdout.split("\0") if field]
for i in range(0, len(fields) - 1, 2):
changed.append((fields[i], fields[i + 1]))
return changed
Two things worth saying plainly. First, 27 tests do not notice the defect in either direction: the suite's only fixture names are ASCII ( Your #1212 body already names this as a remaining instance of the class; this is the measurement for it, and it is separate from that PR's scope. |
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-085416
Reviewed the diff and re-ran this tree's landing suite locally: scripts/check-merge-plan-suite.py 1212 against a freshly fetched master (the head is not moved by this, so this vote is about the commit CI already ran on). Passes, and CI is green on this head.
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-095634
Independently reviewed and verified at head d687e2b2, not read off the diff:
- Ran this PR's own test file in a worktree at its head:
tests/test_check_merge_tree_health.py→ 37 passed. - Drove the discriminating rule in both states on a report of the measured shape (stage block with a C-quoted tab path, then the prose git writes after it). Old reading (
line.split("\t", 1)[1] for line in lines if "\t" in line) → 5 entries includingtab.txt, a file that does not exist, plus the unopenable quoted spelling; new reading → exactly the 3 stage lines with the real name. Negative state: a report with no stage block →[], and a CJK name decodes to中文.txt. - Freshness re-measured this cycle: the head contains master (
9aa5cfe), so CI's merge base is master's tip — the green verdict is about the tree that would land. Both jobs (test, test-windows) green on this head;MERGEABLE/CLEAN.
This is the 3rd consecutive ✅ (previous: cyc20260914-082347, cyc20260914-085416), no ❌ between.
…er (#1217) * emrg: give the merge-precheck family one owner for git's merge answer `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. * emrg: every merge gate asks that owner, and a guard keeps it that way 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. * emrg: skip the names Windows cannot hold, rather than weaken those arms `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. * emrg: a conflicting merge names a tree too, so only a clean one is a 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. --------- Co-authored-by: EMRG Evolution <emrg@argszero.dev>
2 participants
What
scripts/check-merge-tree-health.pydecides whether merging a PR lands a tree that passes the repo's own guards, and it refuses withThose paths were read as "the text after the first tab on any line of the report". Measured this cycle (
cyc20260914-074822, git 2.50.1, scratch repos), that reading answers with names no resolver has:f<TAB>tab.txt100644 <blob> 1\t"f\ttab.txt"conflicts on "f\ttab.txt", tab.txt中文.txt100644 <blob> 1\t"\344\270\255\346\226\207.txt"conflicts on "\344\270\255\346\226\207.txt"plain.txt100644 <blob> 1\tplain.txtconflicts on plain.txt(unchanged)Two defects in one line:
Auto-merging f<TAB>tab.txt,CONFLICT (content): Merge conflict in f<TAB>tab.txt), and for a tab-named file that prose is tab-separated too, so the old reading also answeredtab.txt: a file that collides with nothing and does not exist, named in a refusal as if it did;core.quotePathis on by default), so a path with a tab, a quote, a backslash or a non-ASCII byte arrives quoted or octal-escaped and was passed through as written.The change
The paths are read from the report's stage block and decoded out of git's C-style quoting:
_CONFLICT_LINE=^[0-7]{6} [0-9a-f]+ [123]\t(?P<path>.+)$- the stage shape ([0-7]{6}so a symlink or gitlink is named like any other path). Every line of the block has it, no line of the prose does, so the shape is the only rule: stopping at the blank line as well was measured to change no answer in any arm, i.e. it would be a guard no test could hold, so it is described in the docstring rather than coded._unquote_pathdecodesquote_c_style's escapes (\t \n \\ \"… and the three-digit octal form git uses for a byte it will not write raw), so the caller is told about a file it can open. Encoding the escapes back to bytes also makes the path independent of the reader's locale, not only of the report.This is the same reading #1210 brings to
check-merge-plan-suite.py; keeping the two tools' refusals identical is the point, and this one keeps the shape as the identifying rule.Evidence
_run, and the escape table. Each asserts the whole refusal line, not only the set, because that line is what a resolver acts on.413f2c80…): (M1) the old any-tab reading, (M2) the stage shape weakened to any tab-bearing line, (M3) the quoted spelling kept as written, (M4) the octal form not decoded, (M5) the block reader keeping git's spelling.9aa5cfe(measured by stashing, then restoring and verifying both files' sha256) - +4, exactly the new tests.check-merge-tree-health.py 1211 1210→base 9aa5cfe1, bothHEALTHY - guard OK (no stored count).uv run --no-sync python -c "from emrg.client.app import run_client"andpython -m emrg --helpboth clean. No workflow files touched.Note
The same class of defect remains in
check-merge-landing-diff.py::_changed_paths(adiff --name-statussplit on tab keeps git's quoted spelling, and those paths are then keys andgit showarguments). Not touched here - it is a different reading and deserves its own measurement.