diff --git a/Agent.md b/Agent.md index a86e2e6d..b5defadb 100644 --- a/Agent.md +++ b/Agent.md @@ -119,7 +119,7 @@ Community needs voiced in HN agent-UI discussions map directly to EMRG's design: pkill -f "emrg.server"; rm -f ~/.emrg/emrgd.token; python -m emrg ``` -Python: `uv run pytest tests/ -v` (1410) — import check: `uv run python -c "from emrg.client.app import run_client" +Python: `uv run pytest tests/ -v` (1433) — import check: `uv run python -c "from emrg.client.app import run_client" GUI: `cd emrg/gui && npm test` (100: 44 daemon_client + 20 conn-manager + 7 integration + 7 nav-policy + 7 gui-state + 6 build-config + 4 boot-contract + 3 preload-api + 2 theme-guard) — syntax: `node --check main.js preload.js daemon_client.js` Renderer: `cd emrg/gui/renderer && npm run typecheck && npm test` (514: 5 snapshot-store + 9 utils + 3 ErrorBoundary + 2 App smoke + 11 commands + 4 copywriting + 11 i18n + 13 markdown + 21 transcript + 11 TranscriptView + 15 history + 31 composer + 41 Composer + 6 LinkDialog + 16 sidebar + 17 Sidebar + 9 fileTree + 9 FileTree + 16 resultPanel + 8 ResultPanel + 27 workspaceView + 10 WorkspaceView + 10 dialog + 6 Dialog + 9 ConfirmDialog + 9 RenameDialog + 10 dialogLists + 3 HelpDialog + 9 MemoryDialog + 6 SkillsDialog + 8 openSession + 6 WelcomeDialog + 9 OpenSessionDialog + 7 NewSessionDialog + 7 rewind + 8 RewindDialog + 7 GithubDeviceDialog + 18 daemonBridge + 7 DaemonBridgeProvider + 30 Shell + 15 DialogHost + 20 SettingsPanel + 6 TaskFormDialog + 5 RantDialog + 4 vendorMarkdown) + `npm run build` → `renderer/dist/` CI: `uv run pytest` (ubuntu + **windows-2025 matrix** — Windows pytest 回归在 PR CI 即失败,v0.2.29 教训 #725) + GUI tests + **actionlint workflow lint** (`rhysd/actionlint@v1.7.12` gate, #444 — workflow 解析错误在 PR CI 即失败,如 `if:` secrets 上下文) @@ -127,6 +127,7 @@ Re-trigger: `scripts/re-trigger-ci.sh [branch]` (workflow_dispatch, #527 — 替 Release bump: `python3 scripts/bump-version.py ` — 一次改齐 8 处版本声明(`emrg/__init__.py`、`pyproject.toml`、`emrg/gui/package.json`、`emrg/gui/package-lock.json` 根 + `packages[""]`、`uv.lock`、`packaging/{build-runtime,make-installer,make-run-installer}.sh`);`--check` 只报告漂移(宿主侧自检,与 CI 的 test_version_sync 对称),`--dry-run` 预览不落盘。锚点缺失/数量不符即 fail-loud,绝不猜测;`uv.lock` 只改 `name = "emrg"` 那一行(v0.2.94 教训:直接 `uv run` 会把 lock 里所有 registry URL 重写成镜像,556 行环境噪声)。bump 后用 `uv run --no-sync pytest` 避免 uv 重生成 lock。详见 Agent.md「Releasing」 Doc count sync: `uv run --no-sync python3 scripts/check-doc-count.py [--write|--dry-run|--resolve-conflict]` — 测量当前树的 pytest 收集数并校验 Agent.md 的 Python 测试数(宿主侧自检,与 CI 的 test_python_count_matches_docs 对称);`--write` 仅单 token 替换那个数字(前后遮罩对比,其余字节不变),锚点缺失/出现多次/收集失败均 fail-loud(exit 2)。`--resolve-conflict` 专治合并冲突态:冲突块的**两侧按构造都是陈旧的**,`--write` 会(正确地)拒绝猜哪一侧为真;该模式去掉冲突标记后**在合并后的树上重新测量**写入,绝不选边——实测两侧写 1335/1337 而合并树实收 1339。冲突出现在计数行以外的位置即拒绝(不是什么通用「删标记」按钮)。用途:合并冲突解决后一条命令同步(#1119/#1120/#1121/#1122 四次冲突均在这一行;2026-09-11 #1130 落地后三个 PR 转 DIRTY,逐个解冲突都靠这条) Node count sync: `uv run --no-sync python3 scripts/check-node-test-count.py [--write|--dry-run]` — 直接问**真实运行器**(vitest / node --test)并校验 Agent.md 的 Renderer 与 GUI 两个总数:`tests/test_doc_counts.py` 只能**静态**数 `it(`/`test(` 定义(pytest 作业没有 node_modules),而静态计数只是运行器的**模型**——R2254(445→448)、#1120(同 stem 文件整份被吞)、#1125(`it.each`/`test.skip` 正则看不见)三次都是模型与实践脱节。GUI 侧按 CI 环境(EMRG_SKIP_INTEGRATION=1)运行后减去 1 个模块级 `skip()` 原因条目(该条目数会先断言为 1,形状变了就停手而不是报个看着像对的数)。缺 node_modules 即报该原因,绝不报假数 +Vote count: `uv run --no-sync python3 scripts/check-vote-count.py ...` — 合并前数**仍然算数**的 LGTM 票,而不是评论里看得见的 ✅ 行数。三条规则让手工计数每次都错:① **票早于 head push 即作废**(rebase/解冲突推新 head 后,旧 ✅ 说的是已经不存在的 commit,而评论历史照样显示五六行 ✅,PR 看着能合实则零票——2026-09-11 实测 #1133/#1134/#1136/#1137 各显示 4-6 张 LGTM,解锁后**有效票全为 0**);② **❌ 重置计数**(三次 ✅ 后一个 needs-fix,再一个 ✅ 只算一票);③ **同一周期只算一票**(否则一个周期独自把 PR 送过门槛)。判票读正文**首字符**(`gh pr review --comment` 让 GitHub 把 ✅ 和 ❌ 都记成 `COMMENTED`,state 字段在这里毫无用处),周期号从正文里取(`cyc20260911-091230`)。push 时间取**该 SHA 上最早一次 workflow run 的创建时间**(就是 GitHub 收到 push 事件的时刻);没有 run 时退回 commit 日期并**在输出里注明**(commit 日期可能早于 push,是乐观方向,不许默默采信)。exit 0 每张 PR 都够票 / 1 有 PR 缺票 / 2 问不出来(gh 失败、响应不可解析)——问不出来的问题绝不报数。评论**逐页取全**(reviews 端点默认只给 30 条且**按时间正序**,超限时丢掉的正是**最新的**票,也就是唯一算数的那几张;#1134 解锁一次就有 7 条评论),并在本地按时间排序(「❌ 重置计数」和「同周期只算一票」都是**位置相关**的规则,依赖服务端返回顺序等于把判决权外包)。两个自暴露的坑:① gh 的 `--jq` **后者覆盖前者**,helper 若自己追加 `--jq` 会吃掉调用方投影,字段名退回原名、`at` 读成空串,而 `"" <= push_time` 恒真 ⇒ **全部票作废**、有 2 票的 PR 显示 0/3(计数方向偏「安全」,没人会去查);故 helper 只管 `--paginate`,投影归调用方,并对载荷形状**运行时断言**(缺 `at` 即 exit 2,不报数) Git-over-https 兜底: `python scripts/sync-master-from-api.py [--repo owner/name] [--ref master]` — 受限网络下 github.com:443 不可达而 api.github.com 可达时,用 Git Data API 的 verification payload + signature 字节级重建上游 commit(含 web-flow GPG 签名 squash merge,reconstruct_commit 经 hermetic 测试验证 sha 一致)并推进本地 refs;内容对象缺失时 fail-loud 提示改用 git fetch(10+ 周期实证的恢复路径) Git-over-https push 兜底: `python scripts/push-branch-from-api.py --branch feature/x [--ref HEAD] [--force]` — 同一宕机场景下的 push 方向(#988 配对):从本地 ref 沿一父链找到远端基点(已有分支头或首个远端已知祖先),自底向上上传 blobs(原始字节)/trees(`git mktree` 语义复算)/commits(结构化创建,author/committer 携带原始 +0800 偏移、消息去尾随换行——GitHub 规范化行为),更新远端 ref 后把本地分支 ref 重写为远端 sha 并 `git diff` 验证内容一致;失败即止不触碰 refs(hermetic 测试经忠实假 API 验证字节级 sha 一致) diff --git a/scripts/check-vote-count.py b/scripts/check-vote-count.py new file mode 100644 index 00000000..ecaf2c9b --- /dev/null +++ b/scripts/check-vote-count.py @@ -0,0 +1,378 @@ +#!/usr/bin/env python3 +"""Count a PR's *valid* LGTM votes: the ones that are still about the current head. + +The class this exists for +------------------------ +The merge rule for this repo is "3 consecutive ✅ LGTMs from different evolution +cycles with no ❌ in between". Reading that off the comment history is misleading, +and every recent cycle has re-derived it by hand and got it wrong at least once: + +* **A vote that predates the head push is void.** When a cycle re-bases and pushes, + every earlier ✅ becomes a statement about a commit that no longer exists. The + comment history still shows five or six "✅ LGTM" lines, so the PR *looks* ready + while it has zero counting votes. Measured on 2026-09-11: #1133/#1134/#1136/#1137 + each showed 4-6 LGTMs and each had **0 valid votes** after being unblocked. +* **A ❌ resets the run.** Three ✅, then a "needs fix", then a ✅ is one vote, not + four. +* **One cycle voting twice is one vote.** Votes are counted per cycle, not per + comment, or a single cycle could carry a PR to the threshold alone. + +So the counting is mechanical, and doing it by hand is exactly the kind of repeated +work that should be a tool. This is the companion to `check-merge-freshness.py`: +that one asks "is the CI verdict still about the tree that would merge?", this one +asks "do we have the votes to merge it at all?". + +How a vote is recognised +------------------------ +By the **first line** of the review body, because the review `state` cannot be used: +every vote in this repo is posted with `gh pr review --comment`, so GitHub records +`COMMENTED` for both "✅ LGTM" and "❌ needs fix". The state field is useless here, +which is worth knowing before writing something that trusts it. + +* first line names ❌ -> a veto +* first line names ✅ (or says LGTM) -> an approval +* anything else -> an ordinary comment, ignored + +The cycle id is read from the body (`cyc20260911-091230`); a vote without one is +reported as unattributable rather than counted, since distinctness cannot be shown. + +Push time, and the honest bound +------------------------------- +A vote counts only if it was submitted *after the head was pushed*. The push time is +taken from the earliest workflow run created for that exact SHA, because that is +when GitHub received the push event - precisely the moment the earlier votes stopped +being about the current head. When no run exists for the head, this falls back to +the head commit's committer date and **says so in the output**: a commit date can +precede the push, so the fallback is the optimistic direction and must not be +silently trusted. The fallback is also the case where the PR has no CI at all, +which is not mergeable anyway. + +Usage +----- + uv run --no-sync python3 scripts/check-vote-count.py [ ...] + uv run --no-sync python3 scripts/check-vote-count.py --json + uv run --no-sync python3 scripts/check-vote-count.py --min-votes 2 + +Exit codes +---------- + 0 every PR has >= --min-votes (default 3) valid votes + 1 at least one PR is short + 2 the check could not be made (gh failed, unparseable response) - fail loud; + never report a count for a question that was not answered + +`gh` and network access to GitHub are required; there is no offline mode. +""" + +from __future__ import annotations + +import argparse +import json +import re +import subprocess +import sys +from dataclasses import dataclass, field + +REPO = "argszero/emrg" + +# The runnable form, as Agent.md documents it. A constant (the same convention as +# check-doc-count.py) so the doc line and the guard that checks it cannot drift +# into agreeing on a string that no longer runs anything. +INVOCATION = "uv run --no-sync python3 scripts/check-vote-count.py" + +# `cyc20260911-091230` - the cycle id the vote comments carry. +_CYCLE_RE = re.compile(r"cyc\d{8}-\d{6}") + +# A veto wins over an approval on the same line: "✅ but ❌ on the second point" +# is a request for changes, and undercounting the veto is the dangerous direction +# (it would let a PR merge on a review that asked for a fix). +_VETO_MARK = "\u274c" # ❌ +_LGTM_MARK = "\u2705" # ✅ + + +def _gh_json(args: list[str]) -> object: + """Run `gh` and parse JSON, failing loud rather than guessing. + + `args` excludes the program name, which is prepended here so no call site can + omit it (a call site that passed bare gh arguments once ran the POSIX `pr` + utility instead, whose error names neither gh nor the mistake). + """ + proc = subprocess.run( + ["gh", *args], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + if proc.returncode != 0: + raise RuntimeError( + f"gh failed (rc={proc.returncode}): gh {' '.join(args)}\n{proc.stderr.strip()}" + ) + return json.loads(proc.stdout) + + +def _gh_json_paginated(args: list[str]) -> list: + """Every page of a list endpoint, as one list. + + Needed because a single request truncates silently: the reviews endpoint + returns 30 by default and orders **oldest first**, so a PR with more than 30 + reviews would drop its *newest* votes - precisely the ones that count, since + the whole rule is about votes cast after the head push. #1134 already carries + 7 reviews and #1136 has been through three unblocks; this is a merge gate, so + the count must not depend on how busy a PR has been. + + `--paginate` with a `.[] | {…}` filter emits one JSON object per line across + pages; parsed per line rather than as one document, since concatenated page + arrays are not valid JSON. + + **The filter belongs to the caller.** An earlier version appended + `--jq ".[]"` here, so a call site that also passed `--jq` gave gh two of them: + the later flag won, the projection was dropped, and every review came back + with its raw field names. The tool then read `at` as `None` for every vote, and + since `"" <= push_time` is true it voided **all** of them - a PR with two valid + votes reported 0/3. It was invisible in the tests because the fake returns + dicts directly and never models the jq contract, and invisible in the count + (which only looked short, a plausible state). Found by running it against the + live PRs after the change; the caller now owns the filter, and the shape is + asserted instead of assumed. + """ + proc = subprocess.run( + ["gh", *args, "--paginate"], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + if proc.returncode != 0: + raise RuntimeError( + f"gh failed (rc={proc.returncode}): gh {' '.join(args)} --paginate\n" + f"{proc.stderr.strip()}" + ) + return [json.loads(line) for line in proc.stdout.splitlines() if line.strip()] + + +def _classify(body: str) -> str: + """`veto`, `approve` or `comment`, from the mark the body *begins* with. + + Leading mark, not "a mark somewhere in the first line": measured 2026-09-11, + the first version searched the line and read this approval as a veto - + + ✅ **LGTM — third vote at this head** ... (two prior ✅; no ❌ at this head) + + The body begins with ✅ (it is a vote), and the ❌ is prose *about* the absence + of a veto. Searching anywhere in the line cannot tell those apart, and the + failure direction is bad: it silently voids a real vote, so an 11-vote PR looks + like it has 9. The repo's convention is that the verdict mark is the first + character, so that is what is read. + """ + stripped = body.lstrip() + if stripped.startswith(_VETO_MARK): + return "veto" + if stripped.startswith(_LGTM_MARK): + return "approve" + # A body that does not open with a mark is an ordinary comment unless its first + # line claims LGTM; those are the human-written reviews this repo also has. + first = next((ln for ln in body.splitlines() if ln.strip()), "") + if "LGTM" in first.upper(): + return "approve" + return "comment" + + +@dataclass +class Vote: + at: str + kind: str + cycle: str | None + valid: bool + why: str + + +@dataclass +class Verdict: + pr: int + title: str + head_sha: str + push_time: str + push_time_exact: bool + votes: list[Vote] = field(default_factory=list) + valid_count: int = 0 + needed: int = 3 + + @property + def short(self) -> bool: + return self.valid_count < self.needed + + +def _head_push_time(head: str) -> tuple[str, bool]: + """Earliest CI run creation time for this SHA, else the commit date. + + Returns `(timestamp, is_exact)`. The run's `createdAt` is the push event time; + a commit date can precede the push, so the fallback is flagged rather than + silently used. + """ + runs = _gh_json( + [ + "api", + f"repos/{REPO}/actions/runs?head_sha={head}&per_page=100", + "--jq", + '{t: ([.workflow_runs[].created_at] | sort | .[0] // "")}', + ] + ) + assert isinstance(runs, dict) + created = runs.get("t") + if isinstance(created, str) and created: + return created, True + + commit = _gh_json(["api", f"repos/{REPO}/commits/{head}", "--jq", "{t: .commit.committer.date}"]) + assert isinstance(commit, dict) + committer_date = commit.get("t") + if not isinstance(committer_date, str) or not committer_date: + raise RuntimeError(f"cannot determine a push time for head {head[:8]}") + return committer_date, False + + +def check_pr(number: int, needed: int) -> Verdict: + view = _gh_json( + ["pr", "view", str(number), "-R", REPO, "--json", "number,title,headRefOid"] + ) + assert isinstance(view, dict) + head = str(view["headRefOid"]) + + push_time, exact = _head_push_time(head) + + # Every page, not the first 30: a truncated list drops the newest reviews, + # which are exactly the votes that count (and the endpoint orders oldest + # first, so the loss is invisible in the output - it just looks short). + reviews = _gh_json_paginated( + [ + "api", + f"repos/{REPO}/pulls/{number}/reviews", + "--jq", + ".[] | {at: .submitted_at, body: .body}", + ] + ) + reviews.sort(key=lambda r: str(r.get("at") or "")) + + votes: list[Vote] = [] + for r in reviews: + assert isinstance(r, dict) + # The projection must have applied: without it the fields arrive under + # their raw names and `at` reads as "", which makes `"" <= push_time` true + # and voids every vote - a full PR reporting 0/3, indistinguishable in the + # count from a genuinely unvoted one. Cheap to check, and it is the exact + # failure the caller-owns-the-filter rule above was written for. + if not r.get("at"): + raise RuntimeError( + f"review payload for #{number} has no `at` field (keys: " + f"{sorted(r)[:5]}) - the --jq projection did not apply, so vote " + "times are unknown; refusing to report a count" + ) + body = str(r.get("body") or "") + at = str(r.get("at")) + kind = _classify(body) + if kind == "comment": + continue + match = _CYCLE_RE.search(body) + cycle = match.group(0) if match else None + if at <= push_time: + votes.append( + Vote(at, kind, cycle, False, f"submitted before the head push ({push_time})") + ) + elif cycle is None: + votes.append(Vote(at, kind, cycle, False, "no cycle id in the vote body")) + else: + votes.append(Vote(at, kind, cycle, True, "")) + + # Walk the votes in order, resetting the run on a veto, and counting each + # cycle at most once inside the trailing run. + run = 0 + seen: set[str] = set() + for v in votes: + if v.kind == "veto": + run = 0 + seen.clear() + elif v.valid and v.cycle is not None and v.cycle not in seen: + seen.add(v.cycle) + run += 1 + # invalid approvals and repeat-cycle approvals leave the run untouched + + return Verdict( + pr=number, + title=str(view["title"]), + head_sha=head, + push_time=push_time, + push_time_exact=exact, + votes=votes, + valid_count=run, + needed=needed, + ) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + prog="check-vote-count.py", + description="Count the LGTM votes that are still about a PR's current head.", + ) + parser.add_argument("prs", nargs="+", type=int, help="pull request number(s)") + parser.add_argument("--min-votes", type=int, default=3, help="votes required (default 3)") + parser.add_argument("--json", action="store_true", help="emit JSON instead of prose") + args = parser.parse_args(argv) + + try: + verdicts = [check_pr(n, args.min_votes) for n in args.prs] + except (RuntimeError, KeyError, ValueError, AssertionError) as exc: + print(f"error: {exc}", file=sys.stderr) + return 2 + + if args.json: + print( + json.dumps( + [ + { + "pr": v.pr, + "head": v.head_sha, + "push_time": v.push_time, + "push_time_exact": v.push_time_exact, + "valid_votes": v.valid_count, + "needed": v.needed, + "ready": not v.short, + } + for v in verdicts + ], + indent=2, + ) + ) + else: + for v in verdicts: + mark = "READY" if not v.short else "SHORT" + src = "" if v.push_time_exact else " (no CI run: push time approximated by commit date)" + print( + f"#{v.pr} {mark} {v.valid_count}/{v.needed} valid votes " + f"(head {v.head_sha[:8]}, pushed {v.push_time}){src}" + ) + for vote in v.votes: + # The mark column reports *counting*, not the vote's kind: an + # approval that predates the head push is a real ✅ and still does + # not count, and rendering it "OK ... VOID" says both at once + # (measured 2026-09-11 on #1133: four lines read "OK - VOID"). The + # kind is already visible in the reason, so the column is free to + # answer the only question the reader has. + if vote.valid: + mark, note = "OK ", "counts" + elif vote.kind == "veto": + mark, note = "NO ", vote.why + else: + mark, note = "VOID", vote.why + print(f" {vote.at} {mark} {vote.cycle or '(no cycle id)'} - {note}") + + if any(v.short for v in verdicts): + print( + f"\nNot enough votes yet (need {args.min_votes} consecutive, from different " + "cycles, none predating the head push). A rebase voids every earlier vote.", + file=sys.stderr, + ) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_check_vote_count.py b/tests/test_check_vote_count.py new file mode 100644 index 00000000..47a87e75 --- /dev/null +++ b/tests/test_check_vote_count.py @@ -0,0 +1,454 @@ +"""Tests for scripts/check-vote-count.py - are a PR's LGTM votes still valid? + +Background (cycle cyc20260911-091230) +------------------------------------- +The merge rule is "3 consecutive ✅ LGTMs from different evolution cycles with no ❌ +in between". Reading that off a comment history is misleading, and every recent +cycle re-derived it by hand: a rebase pushes a new head, which voids **every** +earlier vote, while the history still shows five or six "✅ LGTM" lines. Measured +2026-09-11: #1133/#1134/#1136/#1137 each displayed 4-6 LGTMs and each had **0** +valid votes before re-counting, then 2 after this cycle's votes. + +The three rules that make the count non-obvious are each pinned below: + +* a vote submitted before the head push is void; +* a ❌ resets the run (three ✅ then ❌ then ✅ is one vote); +* a repeat cycle inside the run is one vote, so a single cycle cannot carry a PR. + +The four states of the vet. This is a gate, so both sides are pinned (#455): the +approving state must reach READY, and each of the four ways a vote fails to count +must keep it SHORT - never infer the rule from the failing case alone. + +Nothing here touches the network: `_gh_json` is replaced, and the fake is asserted +to be called, so a test cannot pass by never querying. +""" + +from __future__ import annotations + +import importlib.util +import json +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +SCRIPT = REPO_ROOT / "scripts" / "check-vote-count.py" + +HEAD = "a" * 40 +T0 = "2026-09-11T00:00:00Z" # the head push time +BEFORE = "2026-09-10T00:00:00Z" # any vote before it + + +def _load_module(): + spec = importlib.util.spec_from_file_location("check_vote_count", SCRIPT) + mod = importlib.util.module_from_spec(spec) + # Register before exec: the module declares dataclasses, and dataclasses + # resolves through sys.modules[cls.__module__] at class-creation time. + sys.modules[spec.name] = mod + spec.loader.exec_module(mod) + return mod + + +@pytest.fixture +def mod(): + return _load_module() + + +class FakeGh: + """Routes gh calls to canned answers and records them.""" + + def __init__(self, reviews: list[dict], push_time: str = T0, exact: bool = True): + self.reviews = reviews + self.push_time = push_time + self.exact = exact + self.calls: list[list[str]] = [] + + def __call__(self, args: list[str]) -> object: + self.calls.append(list(args)) + assert args and args[0] in {"pr", "api", "run"}, args + if args[:2] == ["pr", "view"]: + return {"number": 1, "title": "t", "headRefOid": HEAD} + if args[0] == "api": + joined = " ".join(args) + if "actions/runs" in joined: + return {"t": self.push_time if self.exact else ""} + if "/commits/" in joined: + return {"t": self.push_time} + raise AssertionError(f"unexpected gh call: {args}") + + def paginated(self, args: list[str]) -> list: + """The reviews endpoint, which the tool reads page by page.""" + self.calls.append(list(args)) + assert args[0] == "api" and "/reviews" in " ".join(args), args + return self.reviews + + +def _review(at: str, body: str) -> dict: + return {"at": at, "body": body} + + +def _approve(cycle: str, at: str) -> dict: + return _review(at, f"\u2705 LGTM - cycle `{cycle}`") + + +def _veto(cycle: str, at: str) -> dict: + return _review(at, f"\u274c Needs fix - cycle `{cycle}`") + + +def _run(mod, monkeypatch, fake: FakeGh, argv: list[str] | None = None) -> int: + monkeypatch.setattr(mod, "_gh_json", fake) + monkeypatch.setattr(mod, "_gh_json_paginated", fake.paginated) + return mod.main(argv if argv is not None else ["1"]) + + +# --- classification: the leading mark, not a mark anywhere ----------------- + + +def test_a_vote_that_merely_mentions_the_absence_of_a_veto_is_still_an_approval(): + """The bug this classifier shipped with. + + Measured 2026-09-11 on #1134: the first version searched the first *line* for + the veto mark and read a real approval as a veto, because the body says + "(two prior ✅ at this head; **no ❌ at this head**)". That PR then reported 9 + usable votes where it had 11, silently discarding two - and the failure + direction matters: an under-count looks like "not ready yet", which is a + plausible-enough state that nobody investigates. + """ + from_check = _load_module() + body = ( + "\u2705 **LGTM - third vote at this head**, from cycle `x` " + "(two prior \u2705 at this head; **no \u274c at this head**).\n\nVerified from scratch:" + ) + assert from_check._classify(body) == "approve" + + +def test_the_verdict_is_read_from_the_first_character(mod): + assert mod._classify("\u2705 LGTM - cycle `c`") == "approve" + assert mod._classify("\u274c Needs fix: something") == "veto" + assert mod._classify(" \u2705 LGTM - cycle `c`") == "approve", "leading whitespace is common" + assert mod._classify("Just a comment about the code") == "comment" + assert mod._classify("") == "comment" + + +def test_a_body_that_does_not_open_with_a_mark_but_claims_lgtm_counts(mod): + """Human-written reviews in this repo sometimes omit the mark.""" + assert mod._classify("LGTM, verified locally.") == "approve" + assert mod._classify("## Review\nThis needs work") == "comment" + + +# --- the run rule ---------------------------------------------------------- + + +def test_three_consecutive_votes_from_distinct_cycles_are_ready(mod, monkeypatch, capsys): + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z"), + _approve("cyc20260911-020000", "2026-09-11T02:00:00Z"), + _approve("cyc20260911-030000", "2026-09-11T03:00:00Z")]) + assert _run(mod, monkeypatch, fake) == 0 + out = capsys.readouterr().out + assert "READY 3/3" in out + assert len(fake.calls) == 3, fake.calls + + +def test_a_vote_before_the_head_push_is_void(mod, monkeypatch, capsys): + """The rebase rule - the one that makes a 6-LGTM PR have zero votes.""" + fake = FakeGh([_approve("cyc20260911-010000", BEFORE), + _approve("cyc20260911-020000", BEFORE), + _approve("cyc20260911-030000", "2026-09-11T01:00:00Z")]) + rc = _run(mod, monkeypatch, fake) + out = capsys.readouterr().out + assert rc == 1 + assert "SHORT 1/3" in out + assert out.count("VOID") == 2 + assert out.count("- submitted before the head push") == 2 + + +def test_a_voided_approval_is_not_labelled_ok(mod, monkeypatch, capsys): + """A ✅ that predates the head is still a ✅ - and it still does not count. + + Measured 2026-09-11 on #1133: four lines rendered as "OK ... VOID: ...", + the kind column and the validity column disagreeing in one row. The mark now + answers the only question the reader has (does this vote count?), so the two + can no longer contradict each other. + """ + fake = FakeGh([_approve("cyc20260911-010000", BEFORE)]) + _run(mod, monkeypatch, fake) + out = capsys.readouterr().out + assert "OK" not in out, out + assert "VOID" in out + + +def test_a_veto_resets_the_run(mod, monkeypatch, capsys): + """Three ✅ then ❌ then ✅ is one vote, not four.""" + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z"), + _approve("cyc20260911-020000", "2026-09-11T02:00:00Z"), + _veto("cyc20260911-030000", "2026-09-11T03:00:00Z"), + _approve("cyc20260911-040000", "2026-09-11T04:00:00Z")]) + rc = _run(mod, monkeypatch, fake) + assert rc == 1 + assert "SHORT 1/3" in capsys.readouterr().out + + +def test_a_repeat_cycle_inside_the_run_counts_once(mod, monkeypatch, capsys): + """A cycle cannot carry a PR to the threshold by voting repeatedly.""" + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z"), + _approve("cyc20260911-010000", "2026-09-11T02:00:00Z"), + _approve("cyc20260911-010000", "2026-09-11T03:00:00Z")]) + assert _run(mod, monkeypatch, fake) == 1 + assert "SHORT 1/3" in capsys.readouterr().out + + +def test_a_cycle_that_voted_before_a_veto_counts_again_after_it(mod, monkeypatch, capsys): + """The veto resets the run, so the same cycle may vote again in the new run. + + This is the mirror of the repeat rule: distinctness is per-run, not per-PR. + """ + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z"), + _veto("cyc20260911-020000", "2026-09-11T02:00:00Z"), + _approve("cyc20260911-010000", "2026-09-11T03:00:00Z"), + _approve("cyc20260911-030000", "2026-09-11T04:00:00Z"), + _approve("cyc20260911-040000", "2026-09-11T05:00:00Z")]) + assert _run(mod, monkeypatch, fake) == 0 + assert "READY 3/3" in capsys.readouterr().out + + +def test_a_vote_without_a_cycle_id_is_reported_not_counted(mod, monkeypatch, capsys): + """Distinctness cannot be shown, so the vote is void rather than counted.""" + fake = FakeGh([_review("2026-09-11T01:00:00Z", "\u2705 LGTM, looks good"), + _approve("cyc20260911-020000", "2026-09-11T02:00:00Z"), + _approve("cyc20260911-030000", "2026-09-11T03:00:00Z")]) + rc = _run(mod, monkeypatch, fake) + out = capsys.readouterr().out + assert rc == 1 + assert "SHORT 2/3" in out + assert "no cycle id" in out + + +# --- honest bounds --------------------------------------------------------- + + +def test_the_push_time_fallback_is_disclosed_not_silently_used(mod, monkeypatch, capsys): + """With no CI run, the commit date stands in - and the output must say so. + + A commit date can precede the push, so the fallback is the optimistic + direction: it can let a vote count that should not. It is flagged rather than + silently trusted, which is also the case where the PR has no CI at all. + """ + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z")], exact=False) + _run(mod, monkeypatch, fake) + out = capsys.readouterr().out + assert "push time approximated by commit date" in out + + +def test_an_exact_push_time_is_not_flagged(mod, monkeypatch, capsys): + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z")], exact=True) + _run(mod, monkeypatch, fake) + assert "approximated" not in capsys.readouterr().out + + +# --- machine output and failure modes -------------------------------------- + + +def test_json_mode_reports_the_count_and_readiness(mod, monkeypatch, capsys): + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z")]) + rc = _run(mod, monkeypatch, fake, ["1", "--json"]) + payload = json.loads(capsys.readouterr().out) + assert rc == 1 + assert payload[0]["valid_votes"] == 1 + assert payload[0]["ready"] is False + assert payload[0]["push_time_exact"] is True + + +def test_min_votes_lowers_the_threshold(mod, monkeypatch, capsys): + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z"), + _approve("cyc20260911-020000", "2026-09-11T02:00:00Z")]) + assert _run(mod, monkeypatch, fake, ["1", "--min-votes", "2"]) == 0 + assert "READY 2/2" in capsys.readouterr().out + + +def test_a_gh_failure_exits_2_with_the_reason(mod, monkeypatch, capsys): + def boom(args): + raise RuntimeError("gh failed (rc=1): gh api ...\nreason") + + monkeypatch.setattr(mod, "_gh_json", boom) + assert mod.main(["1"]) == 2 + assert "gh failed" in capsys.readouterr().err + + +def test_the_helper_invokes_gh_by_name(mod, monkeypatch): + """`_gh_json` must prepend the program name itself. + + Measured 2026-09-11: a call site that passed `["pr", "view", ...]` to a helper + which also omitted the program name ran the POSIX `pr` utility, whose failure + message (`pr: cannot open view`) names neither gh nor the real mistake. + """ + import subprocess as sp + + seen: list[list[str]] = [] + + class _Done: + returncode = 0 + stdout = "{}" + stderr = "" + + def fake_run(cmd, **kwargs): + seen.append(list(cmd)) + return _Done() + + monkeypatch.setattr(sp, "run", fake_run) + mod._gh_json(["pr", "view", "1"]) + assert seen and seen[0][0] == "gh", seen + assert seen[0][1:] == ["pr", "view", "1"] + + +# --- discoverability ------------------------------------------------------- + + +def test_agent_md_documents_the_canonical_invocation(mod) -> None: + """The tool must be findable, in the runnable form. + + Same convention as the node-count tool's twin test: the Agent.md line is how + a host (or the next cycle) learns the tool exists, and a bare name is not + enough - it has to be the invocation that actually runs. + """ + doc = (REPO_ROOT / "Agent.md").read_text(encoding="utf-8") + assert mod.INVOCATION in doc, ( + "Agent.md must document the vote-count tool with the runnable form - that " + "line is how a host learns the tool exists" + ) + + +def test_the_invocation_constant_matches_the_usage_block(mod) -> None: + """`INVOCATION` is what the Agent.md guard searches for, so it must be runnable. + + Two traps, both measured on 2026-09-11 by mutating this file: + + * `INVOCATION in source` is **circular** - the constant is defined in that + very file, so it passes whatever the documented command says. + * `INVOCATION in docstring` is a **substring** test, and a shortened constant + hides inside the real command: `"python3 scripts/check-vote-count.py"` is a + substring of `"uv run --no-sync python3 scripts/check-vote-count.py"`, so a + constant pointing at a bare `python3` - no uv, no `--no-sync` - passed. + + So the usage line must *begin* with the constant. That is the shape a reader + copies, and a truncated constant cannot be a prefix of the full command. + """ + docstring = mod.__doc__ or "" + usage_lines = [ln.strip() for ln in docstring.splitlines() if ln.strip()] + assert any(ln.startswith(mod.INVOCATION) for ln in usage_lines), ( + f"no usage line in the tool's docstring starts with {mod.INVOCATION!r} - " + f"the constant and the documented command have drifted apart.\n" + f"Usage lines seen: {[ln for ln in usage_lines if 'check-vote-count' in ln]}" + ) + + +# --- the list must be complete --------------------------------------------- + + +def test_reviews_are_read_page_by_page(mod, monkeypatch, capsys): + """The reviews endpoint truncates at 30 and orders **oldest first**. + + So a busy PR loses its *newest* reviews - exactly the votes that count, since + the rule is about votes cast after the head push. Measured 2026-09-11: #1134 + already had 7 reviews after one unblock, #1136 had been through three. It is + a merge gate, so the count must not depend on how busy a PR has been. + + This is the same defect class pm25coder caught in the sibling freshness tool + (#1138): querying a bounded window and treating it as the whole set. + """ + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z")]) + _run(mod, monkeypatch, fake) + # The paginated helper is used for reviews; the plain one never sees them. + assert any("/reviews" in " ".join(c) for c in fake.calls), fake.calls + + +def test_a_vote_beyond_the_first_page_still_counts(mod, monkeypatch, capsys): + """The newest vote decides, and it is the one truncation would drop. + + `FakeGh.paginated` is what the tool receives, so a fixture of 40 reviews + models what `--paginate` returns: all of them, not the first 30. + """ + older = [_approve(f"cyc20260910-{i:06d}", "2026-09-10T10:00:00Z") for i in range(40)] + newest = [ + _approve("cyc20260911-010000", "2026-09-11T01:00:00Z"), + _approve("cyc20260911-020000", "2026-09-11T02:00:00Z"), + _approve("cyc20260911-030000", "2026-09-11T03:00:00Z"), + ] + fake = FakeGh(older + newest) + assert _run(mod, monkeypatch, fake) == 0 + assert "READY 3/3" in capsys.readouterr().out + + +def test_the_paginated_helper_passes_paginate_and_flattens_pages(mod, monkeypatch): + """`--paginate` + `--jq '.[]'` yields one object per line; the helper parses + every line. Concatenated page arrays are not valid JSON, so parsing the whole + stdout as one document would be the bug this guards against.""" + import subprocess as sp + + seen: list[list[str]] = [] + + class _Done: + returncode = 0 + stdout = '{"at": "a", "body": "b"}\n{"at": "c", "body": "d"}\n' + stderr = "" + + def fake_run(cmd, **kwargs): + seen.append(list(cmd)) + return _Done() + + monkeypatch.setattr(sp, "run", fake_run) + out = mod._gh_json_paginated(["api", "repos/x/pulls/1/reviews", "--jq", "{at, body}"]) + assert out == [{"at": "a", "body": "b"}, {"at": "c", "body": "d"}] + assert "--paginate" in seen[0], seen[0] + assert seen[0][0] == "gh", seen[0] + + +def test_reviews_are_ordered_before_the_run_is_walked(mod, monkeypatch, capsys): + """The run rule is positional, so the walk needs chronological order. + + The walk resets on a veto and counts a cycle once per run - both depend on + reading the votes in time order. GitHub's reviews endpoint happens to return + oldest-first, so concatenating pages preserves that today; that is a property + of the *endpoint*, not of this code, and a `direction=desc` query would invert + every verdict without changing a byte of the classifier. Sorted here so the + rule does not silently depend on the server's ordering. + + The fixture is deliberately newest-first. Chronologically the veto sits + between B and C, so the run after it is one vote; walked in the fixture's own + order (A and B land *after* the veto) it would be two. `SHORT 1/3` versus + `SHORT 2/3` is what makes this discriminating - both are short, so a test + asserting only the verdict would pass either way. + """ + fake = FakeGh([ + _approve("cyc20260911-040000", "2026-09-11T03:00:00Z"), + _veto("cyc20260911-030000", "2026-09-11T02:00:00Z"), + _approve("cyc20260911-020000", "2026-09-11T01:00:00Z"), + _approve("cyc20260911-010000", "2026-09-11T00:30:00Z"), + ]) + rc = _run(mod, monkeypatch, fake) + assert rc == 1 + assert "SHORT 1/3" in capsys.readouterr().out + + +def test_a_lost_jq_projection_fails_loud_instead_of_voiding_every_vote(mod, monkeypatch, capsys): + """The bug this shipped with, found by running it against the live PRs. + + `_gh_json_paginated` appended `--jq ".[]"` while the call site passed its own + `--jq ".[] | {at, body}"`. gh honours the *last* `--jq`, so the projection was + dropped, every review came back under its raw field names, and `at` read as + `""`. Since `"" <= push_time` is true, **every** vote was voided: a PR with two + valid votes printed `SHORT 0/3`. + + That is the worst shape of failure in a merge gate - not an exception, but a + plausible-looking count in the safe direction, so a reviewer would simply wait + for votes that already existed. The test fake cannot catch it (it returns + dicts and never models the jq contract), which is why the check is a runtime + assertion on the payload's shape rather than a unit-test expectation. + """ + bad = [{"submitted_at": "2026-09-11T01:00:00Z", "body": "\u2705 LGTM - cycle `cyc20260911-010000`"}] + fake = FakeGh(bad) + rc = _run(mod, monkeypatch, fake) + assert rc == 2, "a missing `at` field must be a could-not-check, not a zero count" + err = capsys.readouterr().err + assert "did not apply" in err + assert "0/3" not in err