From 14d86f5e41e62b71c64c0dad549d8501a367e57d Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 26 Aug 2026 17:15:13 -0500 Subject: [PATCH 1/2] test(gate): the parity failure asked the reader to attribute the drift (BACKLOG #1367) The message was already right, and that is the point. It refuses to name a culprit, correctly, and tells the reader to work out which copy is older first, warning that installing from an older checkout downgrades a machine-global file. Three seats skipped that step in one day. One reinstalled a gate that was already correct and left 121 worktrees ungoverned for some minutes. An instruction that must be followed under time pressure is one that will not be. So the attribution is computed here instead of requested. STRICTLY ADDITIVE. What is compared does not change, the assertion does not move. A verdict is added to the failure text from origin/main's blob of the same path: installed == main, checkout != main YOUR CHECKOUT is the odd one out -- behind, or legitimately changing the gate. Do not reinstall. checkout == main, installed != main THE INSTALLED COPY is the odd one out. What the test exists for. neither == main Unattributable, the strongest reading. main unreadable ATTRIBUTION UNAVAILABLE, said plainly. THE TWO REJECTED SHAPES, because rejecting them is most of the work. Comparing against origin/main INSTEAD would go green when the installed copy is TAMPERED WITH, so long as it matches main -- strictly worse than the problem being solved -- and would red a PR legitimately changing the gate. Skipping when the checkout is behind risks a FALSE GREEN, and this file's own negative control exists to prevent exactly that: its docstring says folding CRLF out of the comparison was "precisely the edit that could turn a false RED into a false GREEN". Spending a guard to quiet a message is the trade this project refuses everywhere else. ADDITIVE ATTRIBUTION CANNOT PRODUCE A FALSE GREEN, BECAUSE THE ASSERTION IS UNTOUCHED. That is the whole argument. TWO PREMISES IN THE DISPATCH WERE WRONG AND ARE MEASURED IN THE ROW. Distance from main is not the predicate -- 11 behind, 7 passed, 0 gate files changed in those commits. The gate SOURCE has to have moved. And "2 failed, 22 passed" is 24 tests where this file has 7, so it describes a wider selection. The unreadable branch is the one most easily dropped for tidiness and has its own test saying why it must not be: a computed-looking verdict is trusted more than a request to go and check. A fifth test asserts the four branches are mutually distinct, since four branches returning one string would pass every other assertion in the block. A sixth drives the REAL assertion through a temporary installed path to prove the message actually calls the verdict -- the machine- global file is never read or written for it. Mutation-tested: dropping the verdict from the message reds only the wiring test; collapsing the unreadable branch reds only its own. Co-Authored-By: Claude Opus 5 --- docs/BACKLOG.md | 71 ++++++++++++ tests/test_gate_installed_parity.py | 164 ++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 10e64e6a1..a0fd67151 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -16737,3 +16737,74 @@ share a shape. **Related:** #1307 (the retirement flag itself), #1363 (the ordering defect in the same path), #1242 (the carry-through's silent-drop incident, whose fix this must not undo). **Source:** routed by the Liaison. Their brief said `apply.py` never reads the field and that it appears only in argparse help; both are wrong -- the reader is built dynamically as `retired_{sub}`, which a literal grep for `retired_absence` cannot see. Their brief also said the ledger had zero mentions of it; #1363's evidence block carries two. + +## 1367. The gate-parity failure asks the reader to attribute the drift instead of computing it + +> 🔢 **Filed 2026-08-26 -- FIXED IN THIS CHANGE; the status banner is deliberately unchanged** +> (same handling as #1026, #1361 and #1365). `test_the_installed_gate_matches_the_committed_source` +> compares the installed gate against THIS CHECKOUT, which is the right question. Its failure text +> then correctly REFUSES to name a culprit and hands the reader a `git log` to run. Three seats did +> not run it. + +**Cluster:** Tooling / worktree gate. **Priority:** P3. **Verdict:** build (small, test-only). +**Severity:** a misread of this message left 121 worktrees ungoverned for some minutes. + +**The message was already right, and that is the point.** It says, verbatim: *"WORK OUT WHICH COPY IS +OLDER FIRST. Installing from a checkout older than the installed gate DOWNGRADES a machine-global file +for every session on this box"*, and *"A re-install would clear any of the three ... but it clears a +genuine rule difference the same way, so the fact that it worked tells you NOTHING about which one you +had."* That is exactly the trap that was then fallen into, warned about in advance, in the text being +looked at. **An instruction that must be followed under time pressure is one that will not be.** The +defect is that the test asks for an attribution it can compute itself. + +**Two premises in the dispatch were wrong, and both were measured rather than argued.** + +*"Every seat with an open PR is behind main, so every seat sees this."* Distance is not the predicate: + +``` +wtcsartifact 0 behind 7 passed +builder-1 11 behind 7 passed <- behind, and PASSES +gate files changed on main in those 11 commits: 0 +``` + +**The gate SOURCE has to have moved.** A seat 40 commits behind whose gate files did not move is +green; a seat 1 behind whose gate file did move is red. The true statement is *"every seat is exposed +in the window after a gate change lands"*, which is a much smaller claim. + +*"2 failed, 22 passed"* is 24 tests and this file has 7 (control: `grep -c '^def test_'` = 7), so that +reading is from a wider selection and does not describe this file. + +**The fix is STRICTLY ADDITIVE. What is compared does not change and the assertion does not move.** A +computed verdict is added to the failure text, from `origin/main`'s blob of the same path: + +| installed | checkout | verdict | +|---|---|---| +| == main | != main | YOUR CHECKOUT is the odd one out -- behind, or legitimately changing the gate. **Do not reinstall.** | +| != main | == main | THE INSTALLED COPY is the odd one out. This is what the test exists for. | +| != main | != main | Neither matches. Unattributable, and the strongest reading. | +| main unreadable | | **ATTRIBUTION UNAVAILABLE** -- says so rather than guessing. | + +**The two rejected shapes, recorded because rejecting them is most of the work.** + +*Comparing against `origin/main` INSTEAD* would go green when the installed copy is **tampered with**, +so long as it matches main -- strictly worse than the problem being solved -- and would red a pull +request that is legitimately CHANGING the gate. + +*Skipping when the checkout is behind* risks a **false green**, and this file's own negative control +exists to prevent exactly that: its docstring says folding CRLF out of the comparison was *"precisely +the edit that could turn a false RED into a false GREEN"*. Spending a guard to quiet a message is the +trade this project refuses everywhere else. + +**Additive attribution cannot produce a false green, because the assertion is untouched.** That is the +whole argument for this shape. + +**The unreadable branch is the one that matters most and is easiest to lose.** A missing fetch, a +detached ref and a network-less runner all land there. If it degraded into silently picking one of the +other three, the message would look complete while naming a culprit nothing established -- worse than +the original, because a computed-looking verdict is trusted more than a request to go and check. It +has its own test saying so, and a fifth test asserts the four branches are mutually distinct, since +four branches returning the same text would pass every other assertion in the block. + +**Driven against the PURE verdict function with injected hashes**, never by mutating the installed +gate: that file is machine-global and every PreToolUse hook on this box reads it, so a test may not +take it out from under a concurrent session. Same reasoning the existing negative control gives. diff --git a/tests/test_gate_installed_parity.py b/tests/test_gate_installed_parity.py index b97ca8d07..e2874bce9 100644 --- a/tests/test_gate_installed_parity.py +++ b/tests/test_gate_installed_parity.py @@ -309,6 +309,77 @@ def source_is_committed() -> bool: return out.returncode == 0 and not out.stdout.strip() +def _source_rel() -> str: + """The gate's repo-relative posix path -- the form ``git show :`` wants.""" + return SOURCE_GATE.relative_to(ROOT).as_posix() + + +def drift_verdict(installed: str, source: str, main: str | None) -> str: + """WHICH copy is the odd one out, decided rather than left to the reader. + + The assertion below already told the reader to work out which copy was older before reinstalling, + and warned that installing from an older checkout DOWNGRADES a machine-global file. That warning + was correct and three seats skipped it anyway, because it asks for a `git log` under time pressure. + One of them reinstalled a gate that was already correct and left 121 worktrees ungoverned for some + minutes. So this computes the attribution instead of requesting it. + + PURE, and separated from the git read on purpose: the verdict is what needs testing, and a test + that had to mutate the installed gate to reach it would be taking a machine-global file out from + under every concurrent session. + + ``main`` is the content hash of the same path at ``origin/main``, or None when it could not be + read. A None says so in the text -- an attribution that quietly disappears when its input is + missing would be worse than none, because the message would read as complete. + """ + if main is None: + return ( + "ATTRIBUTION UNAVAILABLE: origin/main's copy of this file could not be read, so which " + "side drifted is NOT established here. Work it out by hand before acting." + ) + if installed == main and source != main: + return ( + "ATTRIBUTED: THE INSTALLED GATE MATCHES origin/main. YOUR CHECKOUT IS THE ODD ONE OUT. " + "Either it is behind (fetch and rebase), or it is legitimately CHANGING the gate, in " + "which case this is expected until the change lands and is installed. DO NOT REINSTALL " + "FROM HERE -- the installed copy is current and installing an older one downgrades it " + "for every session on this box." + ) + if source == main and installed != main: + return ( + "ATTRIBUTED: YOUR CHECKOUT MATCHES origin/main AND THE INSTALLED COPY DOES NOT. This is " + "the case this test exists for: the running gate is not what the repository says it " + "should be. Diff them, and only then reinstall from a PLAIN terminal." + ) + if installed != main and source != main: + return ( + "ATTRIBUTED: NEITHER COPY MATCHES origin/main. Nothing here can say which is intended, " + "and that is the strongest of the three readings -- treat it as unexplained until you " + "have diffed both against origin/main by hand." + ) + # installed == source == main is not a drift at all; the assertion cannot have fired. + return "NO DRIFT: both copies match origin/main." + + +def main_content_hash(rel_path: str) -> str | None: + """``content_hash`` of ``rel_path`` at ``origin/main``, or None if it cannot be read. + + Returns None rather than raising: this is diagnostics attached to a failure, and turning a + diagnostics problem into a different failure would obscure the one being reported. + """ + try: + out = subprocess.run( + ["git", "show", "origin/main:" + rel_path], + cwd=ROOT, + capture_output=True, + timeout=30, + ) + except (OSError, subprocess.SubprocessError): + return None + if out.returncode != 0 or not out.stdout: + return None + return content_hash(out.stdout) + + def test_the_installed_gate_matches_the_committed_source() -> None: # Announce the target BEFORE any skip. A print after a skip never runs, and with no -rs in the pytest # config the reason is not shown either -- the file then renders as a bare "sss." on CI, which is the @@ -343,6 +414,10 @@ def test_the_installed_gate_matches_the_committed_source() -> None: f"CONTENT DRIFT: the RUNNING gate is not this checkout's script.\n" f" installed: {INSTALLED_GATE} content={installed[:12]}\n" f" source : {SOURCE_GATE} content={source[:12]}\n" + # Computed, not requested. The paragraph below already tells the reader to work out which + # copy is older first; that instruction is correct and was skipped three times in one day, + # so the answer is worked out here instead. Evaluated only when the assertion fires. + f"{drift_verdict(installed, source, main_content_hash(_source_rel()))}\n" f"Line endings are folded out of this comparison, so CRLF vs LF alone cannot have produced it. " f"That is the only difference the fold hides, which is NOT the same as this being a difference " f"in rules or logic: the fold rewrites \\r\\n and nothing else, so at least three non-rule " @@ -566,3 +641,92 @@ def test_the_opt_in_list_only_names_tools_the_gate_actually_has() -> None: assert handled >= OPT_IN_TOOLS, ( f"OPT_IN_TOOLS names {sorted(OPT_IN_TOOLS - handled)}, which the gate no longer implements" ) + + +# --- the drift attribution (BACKLOG #1367) ---------------------------------------- +# +# Driven against the PURE verdict function with injected hashes. The alternative -- mutating the +# installed gate to produce each case -- would take a machine-global file out from under every +# concurrent session on this box, which is the same reasoning the negative control above gives for +# testing its predicate directly rather than the installed copy. + +_INSTALLED, _CHECKOUT, _MAIN, _THIRD = "aaa", "bbb", "ccc", "ddd" + + +def test_the_verdict_blames_the_checkout_when_the_installed_copy_matches_main() -> None: + """The false alarm three seats acted on. A behind checkout, or one legitimately CHANGING the gate, + both land here -- and both must be told NOT to reinstall, because installing an older copy + downgrades a machine-global file for every session.""" + text = drift_verdict(installed=_MAIN, source=_CHECKOUT, main=_MAIN) + assert "YOUR CHECKOUT IS THE ODD ONE OUT" in text + assert "DO NOT REINSTALL" in text + + +def test_the_verdict_blames_the_installed_copy_when_the_checkout_matches_main() -> None: + """The case the test exists for: the running gate is not what the repository says it should be.""" + text = drift_verdict(installed=_INSTALLED, source=_MAIN, main=_MAIN) + assert "THE INSTALLED COPY DOES NOT" in text + assert "DO NOT REINSTALL" not in text # here a reinstall IS the remedy, once diffed + + +def test_the_verdict_refuses_to_choose_when_neither_side_matches_main() -> None: + text = drift_verdict(installed=_INSTALLED, source=_CHECKOUT, main=_THIRD) + assert "NEITHER COPY MATCHES" in text + + +def test_the_verdict_says_so_rather_than_guessing_when_main_cannot_be_read() -> None: + """THE BRANCH MOST LIKELY TO BE DROPPED FOR TIDINESS, AND THE ONE THAT MUST NOT BE. + + A missing fetch, a detached ref and a network-less runner all land here. If this degraded into + silently picking one of the other three readings, the message would look complete while naming a + culprit nothing established -- which is a worse failure than the one this attribution fixes, + because a computed-looking verdict is trusted more than a request to go and check. + """ + text = drift_verdict(installed=_INSTALLED, source=_CHECKOUT, main=None) + assert "ATTRIBUTION UNAVAILABLE" in text + assert "NOT established" in text + # It must not blame either side. + assert "ODD ONE OUT" not in text + assert "THE INSTALLED COPY DOES NOT" not in text + + +def test_the_verdict_branches_are_distinguishable_from_each_other() -> None: + """A control on the four above: they are only worth asserting if they differ. Four branches that + returned the same text would pass every test in this block.""" + texts = [ + drift_verdict(_MAIN, _CHECKOUT, _MAIN), + drift_verdict(_INSTALLED, _MAIN, _MAIN), + drift_verdict(_INSTALLED, _CHECKOUT, _THIRD), + drift_verdict(_INSTALLED, _CHECKOUT, None), + ] + assert len(set(texts)) == 4, f"branches collapsed: {len(set(texts))} distinct of 4" + + +def test_the_failure_message_actually_carries_the_verdict( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """WIRING, not verdict. The five tests above prove ``drift_verdict`` is correct; none of them + proves the assertion CALLS it. A typo in the call site, or a later edit dropping the interpolation, + leaves every one of them green while the message loses the attribution -- which is the whole fix. + + Drives the real assertion by pointing INSTALLED_GATE at a TEMPORARY copy with altered bytes. The + machine-global file is never read for this and never written: every PreToolUse hook on this box + reads that path, and a test may not take it out from under a concurrent session. + """ + import tests.test_gate_installed_parity as mod + + fake = tmp_path / "worktree_gate.ps1" + fake.write_bytes(SOURCE_GATE.read_bytes() + b"\n# drift planted by a test\n") + monkeypatch.setattr(mod, "INSTALLED_GATE", fake) + + with pytest.raises(AssertionError) as caught: + mod.test_the_installed_gate_matches_the_committed_source() + + text = str(caught.value) + assert "CONTENT DRIFT" in text, "the assertion fired but the message is not the drift message" + # The planted copy matches neither side, so the verdict must be one of the attributed readings -- + # asserted as "some verdict is present", because which one depends on this checkout's distance + # from origin/main, and pinning that would make this test pass or fail on the tree it runs in. + assert any( + marker in text for marker in ("ATTRIBUTED:", "ATTRIBUTION UNAVAILABLE", "NO DRIFT:") + ), f"the failure message carries no attribution at all:\n{text[:400]}" From 3fc2d2e8b4ad6a670a8d320c66231c46a04c9881 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 26 Aug 2026 17:34:47 -0500 Subject: [PATCH 2/2] test(gate): the verdict now names the file and the control that decides it (BACKLOG #1367) The attribution said WHICH SIDE drifted. It did not say which FILE, or how to check -- and the obvious instrument does not answer the question. DISTANCE IS NOT THE PREDICATE. Measured: an 11-commits-behind tree passes all seven of these, because the gate source did not move in those eleven. A tree 40 behind whose gate files are untouched is green; a tree 1 behind whose gate file moved is red. So a reader reaching for git rev-list --count HEAD..origin/main gets a number that decides nothing. A fleet note recorded "31 commits behind" as though the number were the cause; it was the circumstance that made it true once. The checkout-is-odd branch now prints the file and the control that does decide: git diff --quiet origin/main -- # exit 0 = your tree matches main Suggested by the Cleaner, whose own episode is why the attribution exists, and whose note carried the commit-count framing this replaces. It is worth more than when it was built: since the cause is not distance, a reader cannot derive it from a commit count at all, so the test has to say it. Asserted, with a negative arm: the verdict must NOT offer rev-list, because that is the instrument that does not answer this. Mutation-tested -- deleting the control line reds only that test. Co-Authored-By: Claude Opus 5 --- tests/test_gate_installed_parity.py | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/tests/test_gate_installed_parity.py b/tests/test_gate_installed_parity.py index e2874bce9..a47c9a371 100644 --- a/tests/test_gate_installed_parity.py +++ b/tests/test_gate_installed_parity.py @@ -314,7 +314,9 @@ def _source_rel() -> str: return SOURCE_GATE.relative_to(ROOT).as_posix() -def drift_verdict(installed: str, source: str, main: str | None) -> str: +def drift_verdict( + installed: str, source: str, main: str | None, rel_path: str | None = None +) -> str: """WHICH copy is the odd one out, decided rather than left to the reader. The assertion below already told the reader to work out which copy was older before reinstalling, @@ -331,6 +333,7 @@ def drift_verdict(installed: str, source: str, main: str | None) -> str: read. A None says so in the text -- an attribution that quietly disappears when its input is missing would be worse than none, because the message would read as complete. """ + rel_path = _source_rel() if rel_path is None else rel_path if main is None: return ( "ATTRIBUTION UNAVAILABLE: origin/main's copy of this file could not be read, so which " @@ -342,7 +345,16 @@ def drift_verdict(installed: str, source: str, main: str | None) -> str: "Either it is behind (fetch and rebase), or it is legitimately CHANGING the gate, in " "which case this is expected until the change lands and is installed. DO NOT REINSTALL " "FROM HERE -- the installed copy is current and installing an older one downgrades it " - "for every session on this box." + "for every session on this box.\n" + # NAME THE FILE AND THE CONTROL, because a commit count cannot answer this. Measured + # 2026-08-26: an 11-commits-behind tree passes all seven of these, because the gate + # source did not move in those eleven. Distance is not the predicate -- whether THIS + # FILE moved is. A reader reaching for `git rev-list --count HEAD..origin/main` gets a + # number that does not decide anything, which is how a note came to record "31 commits + # behind" as though the number were the cause. + f" THE FILE: {rel_path}\n" + f" THE CONTROL, which answers it where a commit count does not:\n" + f" git diff --quiet origin/main -- {rel_path} # exit 0 = your tree matches main" ) if source == main and installed != main: return ( @@ -657,10 +669,20 @@ def test_the_verdict_blames_the_checkout_when_the_installed_copy_matches_main() """The false alarm three seats acted on. A behind checkout, or one legitimately CHANGING the gate, both land here -- and both must be told NOT to reinstall, because installing an older copy downgrades a machine-global file for every session.""" - text = drift_verdict(installed=_MAIN, source=_CHECKOUT, main=_MAIN) + text = drift_verdict(installed=_MAIN, source=_CHECKOUT, main=_MAIN, rel_path="a/b/gate.ps1") assert "YOUR CHECKOUT IS THE ODD ONE OUT" in text assert "DO NOT REINSTALL" in text + # NAME THE FILE AND HAND OVER THE CONTROL THAT ACTUALLY DECIDES IT. A reader reaching for a + # commit count gets a number that decides nothing: an 11-commits-behind tree passes all seven of + # these when the gate source did not move, and a 1-behind tree reds when it did. The Cleaner's + # note recorded "31 commits behind" as though the number were the cause; it was the circumstance. + assert "a/b/gate.ps1" in text, "the verdict must name the file, not just the side" + assert "git diff --quiet origin/main -- a/b/gate.ps1" in text + assert "rev-list" not in text, ( + "do not offer a commit count here -- it is the instrument that does not answer this question" + ) + def test_the_verdict_blames_the_installed_copy_when_the_checkout_matches_main() -> None: """The case the test exists for: the running gate is not what the repository says it should be."""