emrg: the merge condition names the clause the counter applies

emrg/server/evolution_prompt.md

@@ -152,6 +152,8 @@ cd {{ source_dir }} && gh pr list -R {{ owner }}/{{ repo }} --limit 20
   - **⚠️ A CONFLICTING fork PR also gets zero CI checks** (#716 lesson: `mergeable: CONFLICTING` / `mergeable_state: dirty` → GitHub refuses to run CI for a dirty PR; `gh workflow run` cannot target fork refs, close/reopen does NOT re-fire checks for dirty PRs). Unblock path: check `maintainer_can_modify: true`, fetch `refs/pull/N/head`, create a local branch, `git merge master`, resolve conflicts, `git push <fork-remote> <branch>:<fork-branch>` — the `pull_request` synchronize event then fires CI. Post a comment explaining the maintainer push. Never ask the author to rebase blindly when you can resolve the conflict yourself as Committer.
   - Local verification (pytest + npm test) is necessary but NOT sufficient — CI is the only place the actionlint gate (#444) and the full doc-count guard (#511) run
 - Check merge conditions: does the PR's comment history already have 3 consecutive ✅ from different cycles with no ❌ in between?
+  - ⚡ **Ask the instrument instead of counting the ✅ lines by eye**: `scripts/check-vote-count.py <N>` is the reading of that condition, and the comment history misleads in both directions. It counts per *cycle* (one cycle voting twice is one vote), a ❌ resets the run, and it carries a clause the line above does not spell out — **a vote submitted before the head push is void** — so a branch refreshed after its approvals still shows several "✅ LGTM" while it has far fewer counting votes. Its companion `scripts/check-merge-freshness.py <N>` asks the other half: is that green CI still about the tree the merge would produce?
+  - ⚡ **When a stale head has votes at risk, do not refresh it just to make it fresh**: a push moves the head and voids every vote standing on it. Measure the tree the merge would land instead (`scripts/check-merge-plan-suite.py <N>`) and cast the vote on that — the head does not move, so the standing votes stay valid. A refresh is `git merge master` into the branch, then a push; a rebase cannot be published here (the push is refused, and the force-push it would need is forbidden).
   - ⚠️ Query comments with the REST API (GraphQL needs `read:org` scope, often missing from the token):
     `gh api repos/{{ owner }}/{{ repo }}/issues/<N>/comments --jq '.[] | "\(.user.login): \(.body)"'`
     and `gh api repos/{{ owner }}/{{ repo }}/pulls/<N>/reviews --jq '.[] | "\(.user.login) [\(.state)]: \(.body)"'`
@@ -371,7 +373,7 @@ git push origin feature/<short-description>
 gh pr create -R {{ owner }}/{{ repo }} --title "emrg: <short-description>" --body "brief description of changes and reasons"
 ```
 
-**Merge condition**: the PR's comment history must have at least **3 consecutive ✅ LGTMs from different evolution cycles** with no `❌ needs fix` in between, before a Committer may run `gh pr merge --squash`.
+**Merge condition**: the PR's comment history must have at least **3 consecutive ✅ LGTMs from different evolution cycles** with no `❌ needs fix` in between, before a Committer may run `gh pr merge --squash`. `scripts/check-vote-count.py <N>` is the reading of that condition — it counts per cycle, and a vote submitted before the head push is void — so ask it rather than counting the ✅ lines; `scripts/check-merge-freshness.py <N>` says whether the green CI is still about the tree that would land.
 
 **Not pushing = not done**.
 

tests/test_evolution_prompt_merge_condition.py

@@ -0,0 +1,241 @@
+"""The shipped template's merge condition must name the rule the counter enforces.
+
+Why this file exists
+--------------------
+The template states the merge condition as *"3 consecutive ✅ LGTMs from different
+cycles with no ❌ in between"* and tells the cycle to read it off the PR's comment
+history. `scripts/check-vote-count.py` exists because that reading is wrong, and its
+own docstring carries the measurement (2026-09-11: #1133/#1134/#1136/#1137 each
+showed 4-6 "✅ LGTM" lines and each had **0 valid votes**). Its condition has a clause
+the template's sentence did not: **a vote submitted before the head push is void**.
+So an instance whose task prompt is built from the shipped template was told a rule
+narrower than the one this project's gates measure, and pointed at a route (count the
+✅ lines by hand) that answers wrongly whenever a head has moved.
+
+A sibling defect — the vote *body* — was fixed the same way (#1409): the template's
+example now names the cycle the counter reads. This file is the other half of that
+contract, the *condition* rather than the body, pinned where a cycle reads it
+(§1.1 Step 1's "Check merge conditions" and §5's "Merge condition").
+
+Why the pin renders instead of grepping the template
+----------------------------------------------------
+The template is rendered with ``undefined=jinja2.Undefined``, so a name that is not
+in the builder's context renders as the empty string: text present in the file can be
+absent from the prompt an instance receives, and a file-level grep would still pass.
+These tests render through the *real* builder (``TaskHandler._build_evolution_prompt``)
+and read the rendered prompt.
+
+The control
+-----------
+A prose pin can only show that a sentence is present, so the last test drives the
+counter itself over the same three approvals twice — once submitted after the head
+push, once before it. That is what makes the phrase list below evidence rather than
+decoration: the clause the template now states is one the instrument enforces.
+
+Named limit
+-----------
+This pins that the condition and its instrument are stated where the prompt is built.
+It cannot pin that an agent then runs the tool, and it says nothing about *whether* a
+vote should be cast — that is the abstain rule, issue #1408.
+"""
+
+from __future__ import annotations
+
+import importlib.util
+import sys
+from pathlib import Path
+
+import pytest
+import yaml
+
+from emrg.protocol import InstanceIdentity
+from emrg.server import scheduler as mod
+from emrg.server.scheduler import TaskHandler
+
+REPO_ROOT = Path(__file__).resolve().parents[1]
+PROMPTS_DIR = REPO_ROOT / "emrg" / "server"
+COUNTER = REPO_ROOT / "scripts" / "check-vote-count.py"
+
+#: The instrument that reads the condition, and its two companions. Each is checked as
+#: a verbatim substring of the shipped wording, so a rename of a script or a dropped
+#: clause makes these tests say so instead of passing on a paraphrase.
+COUNTER_SCRIPT = "scripts/check-vote-count.py"
+FRESHNESS_SCRIPT = "scripts/check-merge-freshness.py"
+PLAN_SUITE_SCRIPT = "scripts/check-merge-plan-suite.py"
+
+#: The clause the counter implements and the template's one-line condition omitted.
+CLAUSE = "before the head push is void"
+
+#: The route a voter takes when a stale head has votes standing on it. Stated because
+#: the tempting alternative — refresh the branch to make it fresh — is the one action
+#: that destroys the votes it was meant to preserve.
+STALE_ROUTE = "voids every vote standing on it"
+
+#: The two places the condition is read: deciding whether to vote (§1.1) and deciding
+#: whether the merge may proceed (§5). Each is delimited rather than searched globally,
+#: because the question is *where the sentence sits*: a clause in a section nobody
+#: reads before voting does not answer the question the reader has.
+STEP1_START = "- Check merge conditions"
+STEP1_END = "**Issue management**"
+SUBMIT_START = "**Merge condition**"
+SUBMIT_END = "**Not pushing = not done**"
+
+
+def _load_counter():
+    """`scripts/check-vote-count.py` (hyphenated, so not importable by name)."""
+    spec = importlib.util.spec_from_file_location("check_vote_count_merge_condition", COUNTER)
+    module = importlib.util.module_from_spec(spec)
+    sys.modules[spec.name] = module
+    spec.loader.exec_module(module)
+    return module
+
+
+@pytest.fixture(scope="module")
+def counter():
+    return _load_counter()
+
+
+@pytest.fixture(scope="module")
+def rendered(tmp_path_factory) -> str:
+    """The real template, rendered through the real builder with the real context."""
+    tmp_path = tmp_path_factory.mktemp("merge-condition")
+    project_dir = tmp_path / "demoproj"
+    project_dir.mkdir(exist_ok=True)
+    (tmp_path / "projects.yml").write_text(
+        yaml.safe_dump([{"name": "demoproj", "path": str(project_dir)}]), encoding="utf-8"
+    )
+    original = mod.config_dir
+    mod.config_dir = lambda: tmp_path
+    try:
+        handler = TaskHandler(
+            name="demo-task",
+            config={"project": "demoproj"},
+            interval=300,
+            identity=InstanceIdentity(),
+            template_path=PROMPTS_DIR / "evolution_prompt.md",
+        )
+        return handler._build_evolution_prompt()
+    finally:
+        mod.config_dir = original
+
+
+def _region(text: str, start: str, end: str, what: str) -> str:
+    """The shipped text between two anchors, or a failure to measure — never a pass."""
+    i = text.find(start)
+    j = text.find(end)
+    assert i != -1, (
+        f"the {what} anchor {start!r} is gone from the rendered prompt — this test "
+        "cannot measure where the condition is stated, which is a failure to measure, "
+        "not a pass"
+    )
+    assert j > i, (
+        f"the {what} region's end anchor {end!r} no longer follows {start!r} — the "
+        "prompt was restructured, so the region this test reads is no longer the one "
+        "it describes"
+    )
+    return text[i:j]
+
+
+def test_the_condition_names_the_instrument_that_reads_it(rendered: str) -> None:
+    """Both sites name the counter, so a cycle asks it instead of counting by eye."""
+    for start, end, what in (
+        (STEP1_START, STEP1_END, "§1.1 review"),
+        (SUBMIT_START, SUBMIT_END, "§5 submit"),
+    ):
+        region = _region(rendered, start, end, what)
+        assert COUNTER_SCRIPT in region, (
+            f"{what}: the merge condition is stated without naming {COUNTER_SCRIPT}, "
+            "the instrument whose reading of it is the one this repo's gates use — so "
+            "a reader has only the comment history, which the counter was written "
+            "because that misleads"
+        )
+
+
+def test_the_stale_head_route_is_stated_where_a_voter_decides(rendered: str) -> None:
+    """§1.1 must carry the clause, the freshness question and the route to take."""
+    region = _region(rendered, STEP1_START, STEP1_END, "§1.1 review")
+    for term in (CLAUSE, FRESHNESS_SCRIPT, PLAN_SUITE_SCRIPT, STALE_ROUTE):
+        assert term in region, (
+            f"§1.1 no longer states {term!r} beside the merge condition. This is the "
+            "section a cycle reads when it decides whether to vote, and the fact it "
+            "decides on is whether its vote will count: a vote submitted before the "
+            "head push is void, so a stale head is measured on the tree it would land "
+            "rather than refreshed (a push voids the votes it was meant to preserve)"
+        )
+
+
+def test_the_submit_section_states_the_clause_too(rendered: str) -> None:
+    """§5's one-line condition must carry the clause its sentence used to omit."""
+    region = _region(rendered, SUBMIT_START, SUBMIT_END, "§5 submit")
+    for term in (CLAUSE, FRESHNESS_SCRIPT):
+        assert term in region, (
+            f"§5's Merge condition no longer carries {term!r}. The sentence alone reads "
+            "as 'three ✅ in the comment history', which is the narrower rule the "
+            "counter's docstring measures four PRs against (4-6 approvals, 0 valid "
+            "votes each)"
+        )
+
+
+HEAD = "b" * 40
+PUSH = "2026-09-19T01:00:00Z"
+AFTER = "2026-09-19T02:00:00Z"
+BEFORE = "2026-09-19T00:00:00Z"
+
+
+class _FakeGh:
+    """Canned answers for the three calls `check_pr` makes; nothing leaves the process."""
+
+    def __init__(self, reviews: list[dict]) -> None:
+        self.reviews = reviews
+
+    def __call__(self, args: list[str]) -> object:
+        if args[:2] == ["pr", "view"]:
+            return {
+                "number": 1,
+                "title": "t",
+                "headRefOid": HEAD,
+                "mergeable": "MERGEABLE",
+                "mergeStateStatus": "CLEAN",
+            }
+        assert args and args[0] == "api", args
+        return {"t": PUSH}
+
+    def paginated(self, args: list[str]) -> list:
+        assert "/reviews" in " ".join(args), args
+        return self.reviews
+
+
+def _counted(counter, monkeypatch, at: str) -> "object":
+    reviews = [
+        {"at": at, "body": f"\u2705 LGTM — cycle cyc20260919-00000{i}"} for i in range(1, 4)
+    ]
+    fake = _FakeGh(reviews)
+    monkeypatch.setattr(counter, "_gh_json", fake)
+    monkeypatch.setattr(counter, "_gh_json_paginated", fake.paginated)
+    return counter.check_pr(1, counter.DEFAULT_MIN_VOTES, mergeability_wait=0.0)
+
+
+def test_the_counter_voids_the_vote_the_clause_names(counter, monkeypatch) -> None:
+    """The control: the same three approvals count after the push, and not before.
+
+    Without this, the phrases above could be satisfied by sentences describing a rule
+    nothing enforces. Both directions are shown because only one of them is the
+    failure: the votes are identical, the head push is the only difference, and the
+    difference between 3/3 and 0/3 is what the template now tells a reader to ask
+    about instead of counting.
+    """
+    after = _counted(counter, monkeypatch, AFTER)
+    assert after.valid_count == 3, (
+        f"three approvals submitted after the head push counted {after.valid_count} — "
+        "the control needs this direction green before the other one means anything"
+    )
+
+    before = _counted(counter, monkeypatch, BEFORE)
+    assert before.valid_count == 0, (
+        "the same three approvals submitted *before* the head push counted "
+        f"{before.valid_count}: the clause the template states is not the one the "
+        "instrument applies, so the pin above would be pinning prose"
+    )
+    assert all("submitted before the head push" in v.why for v in before.votes), (
+        [v.why for v in before.votes]
+    )