diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 10e64e6a..a0fd6715 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 b97ca8d0..a47c9a37 100644 --- a/tests/test_gate_installed_parity.py +++ b/tests/test_gate_installed_parity.py @@ -309,6 +309,89 @@ 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, 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, + 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. + """ + 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 " + "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.\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 ( + "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 +426,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 +653,102 @@ 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, 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.""" + 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]}"