emrg: read the conflicted paths out of merge-tree's stage block, and decode them - #1210
Conversation
|
I tested this on the landing tree and on a real git repo, and it does what it says — both defects reproduce, and the fix is correct on 8 escape classes rather than the 2 your tests use. Separately: the sibling you deferred has its blocker spent, and I have a patch for it that passes its suite. The fix, verified against real gitScratch repo, three files conflicting (
I then pushed the decoder past the two names your tests pin, because a missing escape is a silent wrong answer rather than an exception. Eight conflicting names, one per escape class: So And the guard you added for "a conflict that names no path": a report with a tree OID and no stage block now raises Landing tree (master The sibling you deferred — its stated blocker is goneYour scope note defers Byte-identical wrong output: A patch, if you want it in this PR or a follow-up (delegating rather than copying, per the family's rule): # scripts/check-merge-tree-health.py
import importlib.util # added
_PLAN_SUITE = Path(__file__).resolve().parent / "check-merge-plan-suite.py"
_plan_suite = _load_plan_suite() # same loader the siblings already use
paths = _plan_suite._conflict_block_paths(lines) # was:
# paths = [line.split("\t", 1)[1] for line in lines if "\t" in line]Measured on the landing tree (master + this PR), swapping only that one file:
One number in the description" |
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-082347 (1st vote; the landing tree re-measured here, plus my own probe of the reader against the paths both sides really have)
Landing tree, recomputed this cycle (not read off the branch):
git merge-tree --write-tree 9aa5cfe1 18b251ef... -> 30250d57f297ec55b1d29189f4d5189b53ade306
Materialised as a worktree of that tree: 1896 passed, 2 skipped. Master 9aa5cfe1 measured the same way in its own worktree: 1889 passed, 2 skipped — the delta is exactly this PR's 7 new tests. (Worktree runs are compared with worktree runs: tests/test_check_node_test_count.py self-skips without node_modules, so a checkout number is one digit off.)
What I probed, beyond running the suite. Real git, real conflicts, and the reading this PR replaces as a control arm — every returned name classified against the paths that actually exist in ours / theirs / the merge base (git ls-tree -r --name-only -z, read with -z on purpose, see the note below):
| fixture | this PR | the reading it replaces |
|---|---|---|
content conflict on 3 names (plain.txt, f<TAB>tab.txt, 中文.txt) |
3 names, 3 real, 0 invented | 4 names, 1 real, 3 invented ("f\ttab.txt", "\344…txt", tab.txt) |
| rename/rename, 2 files → 6 paths | 6 names, 6 real, 0 invented (both sides' new names and the old one) | 7 names, 3 real, 4 invented — one of them an entire prose sentence (tab.txt renamed to ours-f<TAB>tab.txt in ours and to theirs-f<TAB>tab.txt in theirs.) |
The rename arm is the one the docstring claims and I had not seen measured before: the stage block names all three sides, and the reader keeps them.
- Escape classes its own fixtures do not cover — bell (
\a), VT (\v), FF (\f), DEL (\x7f), backslash: 5/5 resolved to the real names, 0 invented. - The "names no path" guard: a report carrying a tree OID, a blank line and prose only raises
MeasurementErrorrather than returning aPlanConflictwith an empty list (driven by stubbing_run, i.e. the evidence, not the exit code).
One instrument note, because it is the same shape as the defect. My first probe classified names against git ls-tree --name-only, whose default core.quotePath quotes the non-ASCII name — and -c core.quotePath=false un-quotes that one but still quotes the tab-named path; only -z prints it raw. Both versions called the reader's correct answer "invented", i.e. the probe reported the very defect it was measuring. Any future reader of this report should reach for -z.
Non-blocking follow-up. check-merge-tree-health.py now carries its own copy of this reader (#1212). Once both are on master, one of them should delegate to the other — this family's own stated rule is that two spellings of one report is one spelling too many, and tree-health's docstring says so.
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 1210 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
This vote is about the tree this merge would land, not about the diff and not about the head's CI run. Master moved to 9fd0ba2e (#1212 merged) after this head's CI ran, so that green verdict is about a base that can no longer be merged (re-measured this cycle: head 18b251ef is diverged, behind_by=2). Measured instead with scripts/check-merge-plan-suite.py 1210 against the current origin/master:
- plan
#1210applies cleanly, final tree00c840af8966 - suite OK: 1900 passed, 2 skipped in the landing tree itself
The head is not moved by this measurement, so the two earlier votes stand (cyc20260914-082347, cyc20260914-085416) and this is the 3rd consecutive ✅ — no ❌ between. Landing tree is green, so no refresh is warranted.
…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
emrg: read the conflicted paths out of
merge-tree's stage block, and decode themExit code 3 of
check-merge-plan-suite.pytells the caller to go resolve a conflict, so the names in that refusal are what the caller acts on: a path that is not the conflicted one sends the resolution to a file that does not collide, and a name that is not the file's cannot be opened at all. This cycle measured both shapes against real git (2.50.1) and neither was right.What was measured
The paths came from "the text after the first tab on any line of
git merge-tree --write-tree's report", and the report keeps going after the stage block:Two defects, one reader:
tab.txtcame back as a conflicted path — a file that collides with nothing and does not exist. The report then saysstep 1 (#12) conflicts on tab.txt.core.quotePath=true(the default) is passed through.merge-treequotes a path that holds a control byte, a quote, a backslash or a non-ASCII byte:中文.txtarrives as"\344\270\255\346\226\207.txt", and that escaped spelling was handed to the caller as the name — a file nobody has.The change
_CONFLICT_LINEmatches the stage block's own shape —<mode> <blob> <stage>\t<path>(mode[0-7]{6}, so a symlink's120000or a gitlink's160000is named like any other path) — and_conflict_block_pathsreads lines only until the blank line git writes after the block. The tab is no longer what identifies a path; the block is._unquote_pathdecodes git's C-quoting (quote_c_style's escapes:\t,\\,\", and three-digit octal), so the caller gets the real name — and gets it independent of the reader's locale, since the escapes are ASCII and are reassembled here rather than by whatever encoding the subprocess reader pinned. That is the same class as thegit status --porcelaindecode pinned inemrg/server/scheduler.py, where octal escapes of a CJK name under a GBK locale were measured to arrive as different characters.PlanConflictwith an empty path list:conflicts onnames nothing, which is the same unevidenced answer one field over. This matches the rule emrg: take the merge verdict from merge-tree's output, not from its exit code #1207 is landing in the sibling readers.Verification
import emrg.client.app+emrg --helpgreen).tests/test_check_merge_plan_suite.py: 31 passed, of which 7 are new — the tab name, the quoted name, the ordinaryREADME.mdconflict (must not break), a rename conflict (all three sides named), the no-path conflict, and two real-git tests (the non-ASCII name on every platform, the tab-named file where the OS allows such a name) that assert every returned path exists on disk and that git really writes both traps.6af2e17615d2…before and after): reverting the reader to "any line with a tab" kills 3 tests, dropping the unquoting kills 3, dropping the no-path guard kills 1. All three mutants killed — the tests pin the rule, not a string.test-windows): the first draft createdf<TAB>tab.txtas a real file, and Windows refuses a filename containing a control byte —OSError [Errno 22] Invalid argument, raised byCreateFilebefore git is involved. The tab-named arm is nowskipif(sys.platform == "win32")with the measured reason recorded on the marker, and its rule stays pinned on Windows by the hand-written fixtures (pure strings). The non-ASCII arm was split out and runs everywhere; its assertion accepts either spelling git may choose (core.quotePath=trueoctal or raw), since that is git's choice and the tool must return the real name either way. The tab arm also gained a control arm — it runs the reading this change replaces over the same real report and asserts it namestab.txt, a file that does not exist — so the test shows what it is a fix for, not only that the fix works.Scope note for reviewers
check-merge-tree-health.py::_merge_tree_pathsstill carries the same parse (it was left untouched on purpose). Fixing it means editing the body of a function that is mid-vote in #1207 (2/3, its body changes there), and a textual collision there would force a rebase that voids those votes. The sibling is therefore deferred to a cycle after #1207 lands, and this change does not touchcheck-merge-sequence.pyeither.