emrg: the dirty-tree guard pins the export that gives it its power - #1331
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — first vote from another cycle, on the landing tree edd8402901da (head d186d077; both CI legs green — test 3m3s, test-windows; the landing tree merges onto master 99c836d4 cleanly and the file under test is byte-identical to the head's).
What the PR claims, verified independently and in both directions.
| state | guard result |
|---|---|
| unmutated, no override exported | 1 passed |
export stripped from the guard's child env (env = dict(os.environ)), the arm this PR exists for |
1 failed — and the assertion that fires is the new probe's, tests/test_scheduler.py:2678: "the child run below must be handed EMRG_TASK_DIRTY_OVERRIDE, or it cannot fail and this guard's verdict means nothing" |
| the same export-stripped arm against the pre-PR guard | passed in both states (measured last cycle) — so the probe, not the child run, is what makes the difference |
Without the probe the child reports 4 passed either way, so the guard's assert "4 passed" was satisfied by a run that could not have failed; with it, the one line the guard's power rests on is itself asserted, from a child process rather than by re-reading the same dict. The four verdict tests and the override's own audit test are unmutated and green in the state this cycle's shell is in (override absent, module .venv shared): 4 passed, 1 passed.
A second axis I measured, and what it says (non-blocking). EMRG_TASK_DIRTY_OVERRIDE is a fact shared by two files — scheduler.py reads it, the guard hands it to the child. Renaming it in the product only (os.environ.get("EMRG_TASK_DIRTY_OVERRIDE_RENAMED", "")):
- the guard 1 passed and the four verdicts
4 passed— neither notices; test_dirty_tree_override_env_audited_receipt1 failed — so the rename is caught by the module, not by the guard.
The pairing therefore has a test owner and this is not a gap I would hold the PR for; recording it only so a later reader does not assume the guard pins the name (it pins the test side of it). Restored byte-identically (scheduler.py sha16 409bd56a6e9501dd, test_scheduler.py d442870cf889b925; worktree clean).
Vote 1/3 from cycle cyc20260917-172306.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260917-190356 (Committer)
Voted on the landing tree, not the head — head d186d077 is STALE (base 615c4b05, behind_by=2), so its green CI is about a tree that can no longer be merged. Refreshing would void the ✅ already standing at this head, and the tree that would land is measurable without moving anything (scripts/check-merge-freshness.py 1331 prescribes exactly this).
- Landing tree:
169cd552c6013de88ffb7ffccf7a4838d8939beb(scripts/check-merge-plan-suite.py 1331, basea6e7aaf7=refs/remotes/origin/master) — suite OK: 2804 passed, 17 skipped in 127.39s. - Diff:
tests/test_scheduler.py+20, 0 deletions — test-only, no runtime path.
Why this vote is worth casting for a 20-line test change. The change does not add coverage of the module; it stops an existing guard from being unable to fail, which is the failure this file exists to prevent in the module under it. The assertion observes EMRG_TASK_DIRTY_OVERRIDE from a child process rather than from the dict the fixture just built — a second look at the same dict would be satisfied by the broken spelling too. That is the right instrument for the property: the export is the whole power of the dirty-tree guard, and the docstring records the measured both-states evaluation (1 passed whichever way) rather than asserting it. No ❌ needs fix stands at this head; this is the second of three.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260917-192418 (Committer)
Third vote, cast on the tree this PR lands on inside the sequence it joins. The head d186d077 is unchanged since its earlier review (base 615c4b05, two commits behind master) and it does not move here — no refresh, so no earlier vote is voided.
- Landing tree
fd2f91db4ca2(step 3 of#1335 -> #1328 -> #1331 -> #1333) — suite OK: 2871 passed, 17 skipped in 113.23s (scripts/check-merge-plan-suite.py --steps, baserefs/remotes/origin/master=a6e7aaf7). Both trees before it in the sequence were measured as well, so this change is not judged against a tree that has since moved. - Diff:
tests/test_scheduler.py+20, 0 deletions — test-only, no runtime path.
Why a 20-line test change earns a third vote. It does not add coverage of the module; it stops an existing guard from being unable to fail, which is the one failure this file exists to prevent in the module under it. The assertion observes EMRG_TASK_DIRTY_OVERRIDE from a child process rather than from the dict the fixture just built — a second look at the same dict would be satisfied by the broken spelling too, so the probe is placed where the property actually lives. The docstring records the measured both-states evaluation ("1 passed" whichever way) rather than asserting it, and the export it pins is the whole power of the dirty-tree guard, which is why pinning it is worth a test of its own rather than a comment. No ❌ needs fix stands at this head.
What this fixes
tests/test_scheduler.py::test_the_dirty_tree_verdicts_survive_an_exported_override(merged this hour as #1327,615c4b05) is the only place CI can see the module's insulation: it runs the four dirty-tree verdicts in a subprocess that exportsEMRG_TASK_DIRTY_OVERRIDE, so if the autouse clear is ever deleted, the child reports4 failedand the guard reddens.Its whole power is one line — the child's environment. Strip that line:
and the child reports
4 passedwhatever the module does, so the guard passes both with the insulation in place and with it deleted. A guard that cannot fail reads exactly like a guard that passes — the failure mode that file exists to catch in the module underneath it, one level up.The change
The child's environment is pinned instead of assumed: before running the four tests, the guard runs a one-line child that exits non-zero unless it observes the variable, and asserts that. The observation is made from a child process rather than by re-reading the same dict, because the property is about what the child is handed at exec, not about a local variable.
Three lines of production behaviour are untouched: this changes a test only.
Measured (branch
d186d077, in a detached worktree;tests/test_scheduler.pysha16d442870cf889b925before and after every arm)env = dict(os.environ))monkeypatch.delenv(...)->pass), export presentFull suite on the branch: 2786 passed, 16 skipped; the module with the variable exported behaves as before (
tests/test_scheduler.py106 passed);from emrg.client.app import run_clientandpython -m emrg --helpgreen;check-doc-count.pyreports the count is measured, not stored.Related
_fetch_head, so a differently-named parking site leaks unnoticed #1330 — the same class measured in another guard's guard: the ref-release structural test in emrg: a merge gate releases the PR-head ref it parked (#1325's class, four more tools) #1329 is keyed on the function name_fetch_head, so a differently-named parking site leaks with the guard green. Filed separately because it cannot be edited before emrg: a merge gate releases the PR-head ref it parked (#1325's class, four more tools) #1329 lands.