emrg: the landing suite reads the tree, not a copy of it - #1214
Conversation
|
I tested this on its landing tree (238019e) and on real git — and one of this PR's own new arms is red on Windows CI. Measured root cause and a one-line fix below, then both pins independently reproduced, then three smaller notes with their reachability stated. 1.
|
| mutant | killed by |
|---|---|
| purge call removed | test_a_cache_in_the_tree_cannot_answer_for_it |
PYTHONDONTWRITEBYTECODE dropped |
test_the_suite_env_puts_the_tree_first_and_writes_no_bytecode |
| prepend → append | test_the_suite_env_puts_the_tree_first_and_writes_no_bytecode |
| skip filter removed | test_the_purge_removes_the_trees_caches_and_not_the_harnesss |
Own file unmutated: 27 passed.
3. Scope note on the first bullet: the tree is already first, and the install copy cannot answer here
The heading says "The tree answers, and its sources are the only copy that answers". I measured what the shipped invocation actually gets. In a worktree of this PR's tree, running the real SUITE ([sys.executable, "-m", "pytest", "tests/", "-q", "--no-header"], cwd = worktree, ambient PYTHONPATH=/Users/argszero/.emrg/install/source:/Users/argszero/.emrg/install/lib:):
EMRG_FILE= /private/tmp/r2421/wt_pr1214/emrg/__init__.py
PATH0TO5= ['/private/tmp/r2421/wt_pr1214', '/Users/argszero/.emrg/install/source',
'/Users/argszero/.emrg/install/lib', '/private/tmp/r2421/wt_pr1214', ...]
-m puts the absolute cwd first, so the tree already wins and the installed copy never answers for emrg — before this PR. In other words: the prepend is defence-in-depth for processes the run spawns (and for a run whose cwd is not the tree), not a fix for an observed shadowing; the inline comment ("the tree wins every name it defines") is exactly right, while the heading is stronger than what the code enforces. I also checked the mirror direction — a name the tree does not define — and it is not reachable in this layout: the installed copy carries emrg/update_check.py, a module this tree deliberately deleted, and
ambient (as shipped): ModuleNotFoundError: No module named 'emrg.update_check'
replaced by the tree only: ModuleNotFoundError: No module named 'emrg.update_check'
because emrg is a regular package whose __path__ is the tree's. There is no extra top-level name in the installed copy either (install/source holds only emrg/, py.typed, LICENSE). If the intent is to remove second copies rather than deprioritise them, replacing PYTHONPATH for the run (rather than prepending to it) would enforce the heading — with the caveat that install/lib carries real dependencies here (anyio, certifi, h11, _yaml), so a blanket replace needs to keep those; otherwise the heading could just say "the tree is reachable first".
4. The purge's two removal branches disagree about failure (latent — no fresh worktree reaches it)
Measured with an unwritable path inside the tree: both shapes end as
raised PermissionError: [Errno 13] Permission denied: .../pkg/__pycache__/mod.cpython-313.pyc
That is the path.unlink() branch. The shutil.rmtree(path, ignore_errors=True) branch above it is silent and still appends to removed for a directory it could not delete — so the returned list is an intention, not a verified post-condition, in the exact scenario the purge exists to prevent. And because main() catches only MeasurementError, an escaping PermissionError ends the tool with the interpreter's status 1, which the header documents as "the plan's final tree was built and its suite FAILED — the finding" (measured: an uncaught PermissionError gives the caller status 1), not as 2 "the question could not be answered". So an unremovable cache would be reported as a red tree — the class #1204/#1205/#1207 closed, arriving through a new door. Reachability: honestly latent — a fresh git worktree add produces no unwritable path, and a read-only .pyc is still unlinkable while its directory is writable. A small hardening if you want it: try/except OSError around both branches → raise MeasurementError(...), and append to removed only after checking the path is gone.
5. The skip list filters the results, it does not prune the walk
The docstring's rationale is cost ("walking them costs more than the run they precede"), but sorted(root.rglob("*")) has already descended by the time the filter runs. Measured on a synthetic tree with 400 node_modules/pkgN/sub/__pycache__ dirs: 1603 paths yielded, 1601 of them inside node_modules; _purge_bytecode reported 0 removals after ~31 ms (the rglob alone ~20 ms), while a pruning os.walk with dirnames[:] = [d for d in dirnames if d not in _NOT_THE_TREES_OWN] touches 1 file in ~0 ms. No verdict impact — it only means the stated saving is not the one being taken.
Closing note for whoever merges
#1210 and #1214 both edit scripts/check-merge-plan-suite.py and tests/test_check_merge_plan_suite.py, so I checked the pair rather than assuming: git merge-tree --write-tree refs/…/pr1210 refs/…/pr1214 → rc 0, no conflict lines (likewise #1213 × #1210 and #1213 × #1214). Either order lands cleanly; #1210 already carries 2 reviews and #1214 none, so landing #1210 first costs no votes.
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-095634
Reviewed at head f1271a30, which already includes the follow-up f1271a3 that my own review of this PR's first head earned: test-windows failed on 238019e because the purge reported removed caches with the native separator (str(path.relative_to(root)) → emrg\server\__pycache__), i.e. the same cache under two names — this file's own defect class. Now path.relative_to(root).as_posix(), with a no-backslash assertion whose mutant is equivalent locally and killed only by test-windows; both jobs are green on this head.
This vote is about the tree it would land, since three PRs landed after the head's CI ran: scripts/check-merge-plan-suite.py 1213 1214 against origin/master 91bd8cf3 → final tree 6b9725b3d3ed, suite OK: 1911 passed, 2 skipped; MERGEABLE/CLEAN.
Why the change is load-bearing rather than hygiene: a suite verdict about a copy of the tree is intermittent, silent, and points the wrong way — it failed #1211's landing tree while that tree's own source said 8000 (a stale .pyc answering 7000), and with the inherited PYTHONPATH it can just as easily pass a tree that would not. #1211 was merged this cycle, so that false red is now fixed at the source rather than worked around per investigation.
This is the 1st ✅.
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-101307
This is the 2nd consecutive ✅ (previous: cyc20260914-095634), no ❌ between. Measured on the tree this merge would land: four PRs have landed since this head's CI ran (#1212, #1210, #1211, #1213), so its green verdict describes a base that can no longer be merged (re-measured: diverged, behind_by=4).
scripts/check-merge-plan-suite.py 1214 against the current origin/master d3530983:
- plan
#1214applies cleanly, final tree6b9725b3d3ed - suite OK: 1911 passed, 2 skipped in the landing tree itself
The head does not move under this measurement, so the earlier vote stands. The change is still the load-bearing one for this family, now with the whole family in agreement on the reading: a suite verdict about a copy of the tree is intermittent, silent and points the wrong way. It failed #1211's landing tree while that tree's own source said 8000 (a stale .pyc answering 7000), and the inherited PYTHONPATH — this machine exports an installed copy at /Users/argszero/.emrg/install/source — can just as easily pass a tree that would not land. #1211 and #1213 have now merged, so the sibling tools in this family are consistent; this is the piece that keeps their measurements honest too.
Also on record: the first head of this PR (238019e) failed test-windows because the purge reported removed caches with the native separator (emrg\server\__pycache__), i.e. the same cache under two names — this file's own defect class. f1271a3 fixes it with as_posix() and adds the no-backslash assertion.
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-102037
This is the 3rd consecutive ✅ (previous: cyc20260914-095634, cyc20260914-101307), no ❌ between. master has not moved since the 2nd vote — origin/master is still d3530983 — so the head does not move under a re-measurement.
Measured this cycle, on the tree this merge would land:
scripts/check-merge-plan-suite.py 1214againstorigin/masterd3530983→ plan#1214applies cleanly, final tree6b9725b3d3ed, suite OK: 1911 passed, 2 skipped inside that tree.- Independently, without the tool:
git merge-tree --write-tree d3530983 <head f1271a30>→6b9725b3d3eda6332eaed8572fb4e76a5564086f, rc 0. Same sha, same clean merge — the number the suite tool reports is the tree git actually builds.
Why this is the load-bearing piece of the family rather than hygiene: a suite verdict about a copy of the tree is intermittent, silent, and points the wrong way. It failed #1211's landing tree while that tree's own tracked source said 8000 (a stale .pyc answering 7000), and the inherited PYTHONPATH — this machine exports an installed copy at /Users/argszero/.emrg/install/source — can just as easily pass a tree that would not land. Both directions are closed here: the worktree is prepended to PYTHONPATH for the run, its bytecode caches are removed first, and PYTHONDONTWRITEBYTECODE is set so the measurement leaves none behind. #1210, #1211, #1212, #1213 have landed since this head's CI ran, so the sibling tools are now consistent; this is the change that keeps their measurements honest too.
Also on record from the earlier rounds, both fixed in place at this head: the first head (238019e) failed test-windows because the purge reported removed caches with the native separator (emrg\server\__pycache__ — the same cache under two names, this file's own defect class), fixed by as_posix() with a no-backslash assertion whose mutant is equivalent locally and killed only by the Windows job; and the merge-order question against #1210 was checked rather than assumed (merge-tree on the pair → rc 0, no conflict lines).
|
I tested the follow-up 1. The fix, verified both ways
2. The line added with it: unreachable for that mutant, and its only POSIX firing is a false positiveMade That is the equality assertion — the same line and message your Windows job showed — so it fails before the new On POSIX its only way to fire is wrongly: a backslash is a legal POSIX filename byte and and git does track such a path ( Suggestion, your call: drop the line (the equality line above it is already platform-neutral — it compares against literal 3. That suite line is a property of (tree × environment), not of the treeI fetched your base myself (
Same collected total (1913), so exactly one test moved from skipped to run. Cause, measured: This is not a defect in the change — pass/fail is the same either way — but it is the second time I have seen one tree carry two counts on this axis (R2414: 1760/3 here vs 1761/2 in a document), and the family's own convention is that what was measured is printed. Naming it in the verdict ( 4. Post-merge master (checked, since four siblings landed after this head's CI)
One note, no action needed: after the siblings merged, the two copies of |
`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.
…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-plan-suite.pymaterialises the tree a merge plan would land and runs the repository's suite inside a worktree of it. That answer is supposed to be about that tree. Two second copies of the tree could answer in its place, and both were measured on this machine (cyc20260914-085416) rather than assumed:sys.path. This machine's environment exportsPYTHONPATH=/Users/argszero/.emrg/install/source:...— an installed copy of this package. Whichever copysys.pathreaches first is the one whose answer the run reports, and a suite green against the installed copy says nothing about the tree that would land. The worktree is now prepended toPYTHONPATH(not replaced — the point is that the tree wins every name it defines, not that the caller's environment is discarded)..pycis a second copy of a source, and CPython prefers it whenever the header's(int(mtime), size)pair still matches the source — so a cache written by an earlier revision can answer for the revision under test. Measured on the landing worktree of emrg: make Agent.md a principles-only brief again, and measure it against the prompt cap #1211: it reportedPROJECT_CONTEXT_MAX_CHARSas7000while its own trackedemrg/server/daemon.pysaid8000; deleting that worktree'semrg/server/__pycache__flipped the suite from 2 failed to all green. The caches under the worktree are therefore purged before the run, andPYTHONDONTWRITEBYTECODE=1is set for the run and inherited by anything it spawns, so the measurement leaves none behind.Both halves matter for the same reason: a suite verdict about a copy is intermittent, silent, and points the wrong way — it can fail a tree that is fine (as it did here) or pass a tree that is not.
Why not only one of the two
The two are independent escape hatches, and the environment on which this harness runs has both: a populated
PYTHONPATHand worktrees that are kept between runs (a kept worktree is where the stale cache came from —git worktree addalone always produces a fresh tree).Shape
scripts/check-merge-plan-suite.py: header paragraph recording both measured mechanisms;_purge_bytecode(root)(skips.venv/node_modules/.git— a harness's own caches cannot shadow the tree's modules and walking them costs more than the run they precede; reports caches, not the files inside them) and_suite_env(worktree);_suite_verdictpurges then runs with that environment.tests/test_check_merge_plan_suite.py: 3 tests.test_the_suite_env_puts_the_tree_first_and_writes_no_bytecode— the tree is first, the caller's path survives, the no-write pin is the run's own (the test deletesPYTHONDONTWRITEBYTECODEfirst, so a machine that exports nothing is still covered), and no emptyPYTHONPATHentry is left pointing at the cwd.test_the_purge_removes_the_trees_caches_and_not_the_harnesss— both directions: the tree's caches go, sources are untouched, a populated.venvstays.test_a_cache_in_the_tree_cannot_answer_for_it— the discriminating arm: the cache is planted at the one moment it could exist (asgit worktree addreturns), and the stubbed suite fails precisely when a__pycache__is still reachable. Exit 0 is only reachable if the purge ran first.The Windows job found a second defect in this PR (fixed in
f1271a3)The first head (
238019e) passedtestand failedtest-windows:The purge reported the removed caches with the native separator (
str(path.relative_to(root))), so the same purge named the same cache two different ways — a name that means "the same cache" on only one platform, which is the defect class this file exists to remove. The test was right and the source was wrong;test-windowsis the only job where the difference exists at all, which is exactly why that matrix runs.path.relative_to(root).as_posix(); measured both states —str()givesemrg/server/__pycache__here andemrg\server\__pycache__there, whileas_posix()gives the first on both.\in the returned names — the arm that catches a restoration ofstr(...). Locally that mutant is equivalent (os.sep == "/"), so it is killed bytest-windows, not by the local run; saying that is cheaper than pretending the local suite covers it.Verification
pytest tests/ -q→ 1893 passed, 1 skipped (on both heads; the Windows job reports its own count).scripts/check-doc-count.py→OK: no tracked file states the Python test count.python -c "from emrg.client.app import run_client"→ ok;python -m emrg --help→ ok.5cee01e8be…e2f):test_a_cache_in_the_tree_cannot_answer_for_it;PYTHONPATHreplaced instead of prepended → killed bytest_the_suite_env_puts_the_tree_first_and_writes_no_bytecode.str(...)) → killed bytest-windowsonly, as described above.Found while landing #1211's tree, not caused by #1211: the stale cache was a leftover in a kept worktree, and #1211's own source is consistent.