From 2910cf92c1a4af32479ea6c3c011be97f4881f43 Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Sat, 12 Sep 2026 18:38:43 +0800 Subject: [PATCH 1/4] emrg: report mergeability, so enough votes is not printed as ready to merge --- Agent.md | 4 +- scripts/check-vote-count.py | 182 ++++++++++++++++++++++++-- tests/test_check_vote_count.py | 227 ++++++++++++++++++++++++++++++++- 3 files changed, 399 insertions(+), 14 deletions(-) diff --git a/Agent.md b/Agent.md index 9fa1cc5c..f91a8d1c 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` (1530) — import check: `uv run python -c "from emrg.client.app import run_client" +Python: `uv run pytest tests/ -v` (1542) — 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 上下文) @@ -128,7 +128,7 @@ Merge freshness: `uv run --no-sync python3 scripts/check-merge-freshness.py 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，逐个解冲突都靠这条）。**测的是「你站着的那个 checkout」**（2026-09-11 实测的坑：解冲突要在 git worktree 里干活，而 root 原先取自 `__file__` ⇒ 跑主 checkout 的脚本、量的却是**主树**，worktree 自己的 Agent.md 写 1401 而工具报 `OK: ... documents 1420`——**读错了树还说一致**，`--write` 更会去改那个你根本没在看的 checkout）。现在 root 从 cwd 解析（cwd 同时有 `Agent.md` 与 `scripts/` 才算 checkout），拿不准就退回脚本自身 root（文档里的 `python3 scripts/...` 照旧可用），并在输出首行**报出所测的树** 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 即报该原因，绝不报假数。**同样测「你站着的那个 checkout」**（与 doc count 同一处修复：root 原先取自 `__file__`，在 worktree 里跑主 checkout 的脚本会量到主树，`--write` 会改错树；现从 cwd 解析并报出所测的树） -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 字段在这里毫无用处）；该字符**透过 markdown 装饰**读取（`**❌`/`- ❌`/`> ❌`/`## ❌`/`1. ❌` 都算，装饰字符本身不可能是判决），并**由它决定整行**——✅ 开头即使后文提到 ❌ 仍是批准（那些提及是在说「没有 ❌」，说法是开放集合），整行扫描只作为无开头标记时的兜底（散文式「Not LGTM」为否决、「Result: ❌ needs fix」为否决）。周期号从正文里取（`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，不报数） +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 字段在这里毫无用处）；该字符**透过 markdown 装饰**读取（`**❌`/`- ❌`/`> ❌`/`## ❌`/`1. ❌` 都算，装饰字符本身不可能是判决），并**由它决定整行**——✅ 开头即使后文提到 ❌ 仍是批准（那些提及是在说「没有 ❌」，说法是开放集合），整行扫描只作为无开头标记时的兜底（散文式「Not LGTM」为否决、「Result: ❌ needs fix」为否决）。周期号从正文里取（`cyc20260911-091230`）。**票够 ≠ 能合**：合并规则是三段合取（3 ✅ + `MERGEABLE`/`CLEAN` + CI 绿），本工具以前只答第一段却印 `READY`——2026-09-12 实测 **6 张** PR（#1152/#1151/#1145/#1142/#1141/#1136）各印 `READY 3/3` 而 `mergeable=CONFLICTING`/`mergeStateStatus=DIRTY`，全是 `gh pr merge` 会拒的，队列看着像六张在等手续实则是死锁；故现读 `gh pr view --json mergeable,mergeStateStatus`，**只降不升**：`CONFLICTING` ⇒ 印 `BLOCKED`（与 `SHORT` 分开，因为药方相反：缺票要回来看，冲突要先把分支追上 master 解掉，而解冲突必然推新 head、把上面所有票作废 ⇒ 先投票是白投；两者都中时 `BLOCKED` 压过 `SHORT`，否则又是在指错药方）；`UNKNOWN`（GitHub 尚未算完，刚 push 常见）与任何不认识的值 ⇒ exit 2 绝不猜；`MERGEABLE` 只用于「不冲突」这一个方向，**不当作安全证据**（GitHub 答的是「文本能否合」，而两个同值计数行干净自动合并正是危险那种——见 `check-merge-freshness.py`）。push 时间取**该 SHA 上最早一次 workflow run 的创建时间**（就是 GitHub 收到 push 事件的时刻）；没有 run 时退回 commit 日期并**在输出里注明**（commit 日期可能早于 push，是乐观方向，不许默默采信）。exit 0 够票且可合 / 1 有 PR `SHORT`（缺票）或 `BLOCKED`（合不了）/ 2 问不出来（gh 失败、响应不可解析、mergeability 尚未算出）——问不出来的问题绝不报数。评论**逐页取全**（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 一致） Conflict triage: `python3 scripts/classify-conflict.py ... [--all]` — 合并冲突的**分类器（只报告，绝不改文件，exit 1 = 有需要人判的块）**，把「两边都动了同一个文件」拆成五种可判定的形状并给出证据支持的解法：`count-line`（两侧只差一个**带括号的计数**数字 ⇒ 必须在合并后的树上**测量**，永不选边，交给 `check-doc-count.py --resolve-conflict`）、`duplicate`（两侧**声明符号**互为真超集**且共有符号的正文全同** ⇒ 取超集那侧）、`disjoint`（声明符号不相交 ⇒ **两边都保留**）、`overlapping`（同名符号，或超集但共有符号正文不同，或单行无判据 ⇒ 必须人读）、`identical`。存在理由（cyc20260911-112155 实测）：同一轮里同一形状需要**相反**的解法——#1136 的分支带着 #1134 的**未合并副本**（分支基于 #1134 而后者已被 squash，祖先关系不可见），master 是严格超集故「取 theirs」正确；#1140 两侧是**不相交新增**，取任何一侧都会静默丢掉另一侧 4 个探针，唯一正解「两边都留」。一条 `--theirs` 对前者对、对后者错。两条判别轴都是被实测打出来的：① 必须是**声明符号**而非文本行——初版按内容行比较，因两侧都含 ` """` 与 ` )` 这种样板行，把两个真实冲突全判成 `overlapping`；② 名字子集**不等于**内容包含——只比名字时「master 声明了分支所有名字」被读成「master 含分支全部内容」，而两侧同名 `test_alpha` 正文不同时取超集会**静默丢掉分支对它的修改**（正是本工具要防的数据丢失）；③ `count-line` 必须限定为**带括号的计数**，否则 `x = compute(1)` vs `compute(2)` 这种纯代码差异也会被答「测量、永不选边」并 exit 0，把唯一该人看的情况关掉。三条均已钉为变异验证的回归测试（cyc20260911-120717）。**第四类：读不了的布局即拒绝**（2026-09-11 实测）——`merge.conflictStyle = diff3` 会在 ours 与分隔符之间插入 `||||||| ` 段，而 `CONFLICT_BLOCK` **照样匹配**（把 base 段吞进 ours），于是判出来的两侧是（ours+base）vs theirs：一段 `disjoint` 被判成 `KEEP BOTH` 并 exit 0，而「都保留」恰好会把**谁都不想要的 base 副本**拼回去；本仓库的真实形态（Agent.md 计数行）更隐蔽——判成 `duplicate - ours is a strict superset - take OURS`，因为 base 段（带着**陈旧**计数的第三份拷贝）被算成 ours 多出来的行。故先于解析检查该标记并报 `unparsed-layout` + exit 1，与 `check-doc-count.py` 对同一布局的具名拒绝一致（那条路是 read-only 无损失，只会不必要地拦住一个本可解的 PR；这条路的错误建议会让解冲突的人**删掉自己的改动**，代价不对称）。**第五类：跨行聚合与「同一段落的两个修订」**（cyc20260911-190629 实测，两个缺陷都在**本工具自己**里，且都靠「拿全部真实冲突块做差分」而非手写 fixture 才暴露出来——手写 fixture 只会覆盖我本来就相信的形状）：① 一条 `count-line` 原先是「两侧各一行」，但 51 次改 Agent.md 的提交里有 **3 次一次动掉 2 个以上计数**，git 会把它们并成**一个**块，该块因此落到内容行兜底、被判「两侧无共同行 ⇒ KEEP BOTH（拼接）」并 exit 0——拼接恰好把每个计数行**各留两份**，正是本仓库 `_duplicated_count_line_kinds` 守卫要拒的状态；现改为**对齐的、逐对只差数字的**块一律 `count-line`（任意长度，且每对都要满足带括号计数）。② 「两侧无共同行」**不等于**「各自新增」——同一段落的新旧两个修订永远不相等。当前 #1140 的 Agent.md 真实块里，ours 的两行段落正是 master 两行的**严格前缀**（890 vs 539、601 vs 471 字符），兜底判 KEEP BOTH 会同时留下陈旧与当前两份段落；现改为见到严格前缀关系即报 `overlapping` 交人读（前缀证据弱于符号子集，误差不对称：升级只是多读一次，「取 theirs」会静默丢一行）。两个修复各配变异验证（还原旧行为即测试变红）。**第六类：计数行与正文修订混在一个块里**（cyc20260911-194733 实测，发生在刚合入的第五类修复**自身**上）：第五类要求「两侧等长且**每一对**都只差数字」，而真实块常把**计数行**和**正文行**配在一起——cb651a4 的真实块正是 ours「Python 计数(1393) + Doc count sync 正文」对 theirs「同一计数行的另一个值 + 同一正文的另一次改写」，两侧等长却**有一对不是计数对**，于是仍落到内容行兜底、判 KEEP BOTH 并 exit 0，而拼接结果里有两个 ``Python: `uv run pytest` `` 行（即文档声称两个 pytest 数），正是 `_duplicated_count_line_kinds` 要拒的状态；且计数数字**在行内**，前面的严格前缀判据（`b.startswith(a)`）对它**结构性不可见**——它只认「短的是长的前缀」，而这里数字之后的内容被改写，两侧互不为前缀。改判据为**按下标配对**：只看对应位置上的行对，任一对「两边都带括号计数且遮罩数字后相等」即报 `overlapping` 交人读（不再要求等长）；证据强度就是遮罩本身——遮罩后相等意味着**同一个事实被重新测量过**，而同一事实出现两个值正是仓库守卫要拒的，故 KEEP BOTH 对它必错。实测收益（最近 400 次改 Agent.md 的提交，逐 hunk 差分）：7 个 hunk 在兜底里带上计数行且**全部**给出**重复内容**的建议——e46c160/0c8a212（对齐，已由第五类改判 count-line）、cb651a4(2v2)/5c039b4(3v3)（等长但混了正文修订，第五类看不见）、3335877(1v2)/444e1d5(1v2)/18fd0af(1v13)（不等长）；本条修复把余下 5 个全部转为 `overlapping`，全仓 150 次提交 1039 个 hunk 差分仅 6 处类别变化（5 个 Agent.md + 1 个 `preload-api.test.js`，后者也是真阳性：两侧写着 54/53 与 55/54 两个不同成员数）。变异验证：退回「只查前缀」即新增测试变红；对面（真实互不相干的两个新增）仍判 disjoint。 diff --git a/scripts/check-vote-count.py b/scripts/check-vote-count.py index c415599c..8fdf3c5c 100644 --- a/scripts/check-vote-count.py +++ b/scripts/check-vote-count.py @@ -51,6 +51,40 @@ 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. +Votes are necessary, not sufficient: the mergeable clause +--------------------------------------------------------- +The merge rule has three conjuncts - 3 consecutive ✅ from different cycles, the PR +is `MERGEABLE`/`CLEAN`, and CI is green. Counting votes answers only the first, and +until now this tool printed `READY` on the strength of it alone. Measured +2026-09-12: **six** PRs (#1152/#1151/#1145/#1142/#1141/#1136) each printed +`READY 3/3` while `mergeable` was `CONFLICTING` and `mergeStateStatus` was `DIRTY` - +every one of them a merge `gh pr merge` refuses. The queue read as six PRs waiting +on a formality; it was a deadlock, and the one tool built to answer "can we merge +this" was the thing saying yes. + +So the mergeable clause is read here, from `gh pr view --json +mergeable,mergeStateStatus`, and it can only ever **downgrade** a verdict: + +* `CONFLICTING` - Git cannot merge the text, so the answer is no regardless of the + votes. Reported as `BLOCKED`, which is a *different* state from `SHORT`: short + means "come back after more review", blocked means "review is done and this still + cannot land" - the state that sat invisible behind six `READY` lines. +* `UNKNOWN` - GitHub has not computed mergeability yet (usual right after a push). + Not a yes and not a no, so this fails loud (exit 2) rather than printing either. + An unrecognised value fails loud for the same reason. +* `MERGEABLE` - the text merges. This is deliberately **not** taken as evidence that + merging is safe: as `check-merge-freshness.py` documents, GitHub answers "does this + textually merge", and a clean auto-merge of two same-valued count lines is the + *dangerous* case, not the safe one. The clause is used only in the direction where + it is decisive (a conflict is a hard no), and the states are printed so a reader + sees them without being told they are fine. + +Two sibling questions stay with their own tools, and are named here so this one does +not quietly pretend to answer them: `check-merge-freshness.py` asks whether the CI +verdict is still about the tree that would merge (so this tool does not re-check CI), +and `check-merge-tree-health.py` (PR #1155) asks whether the merged tree passes the +repo's own guards. + Push time, and the honest bound ------------------------------- A vote counts only if it was submitted *after the head was pushed*. The push time is @@ -70,10 +104,17 @@ 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 + 0 every PR has >= --min-votes (default 3) valid votes **and** is mergeable + 1 at least one PR is SHORT (too few votes) or BLOCKED (cannot be merged) + 2 the check could not be made (gh failed, unparseable response, mergeability + not computed yet) - fail loud; never report a count for a question that was + not answered + +The count is printed either way, so a PR that is BLOCKED still reports its votes; +`--json` carries `valid_votes` and the merge states as separate fields for a +caller that wants to act on them. There is deliberately no flag that drops the +mergeable clause: a mode in which this tool says "ready" about a PR that cannot +merge is the exact reading it was just fixed for. `gh` and network access to GitHub are required; there is no offline mode. """ @@ -89,6 +130,24 @@ REPO = "argszero/emrg" +# GitHub's three-valued mergeability, as `gh pr view --json mergeable` reports it. +# Named rather than compared inline so an unrecognised value (a new GitHub state) +# falls through to the fail-loud branch instead of being read as one of these. +# +# `UNKNOWN` is what GitHub returns before it has computed mergeability at all - +# usual for a minute or two after a push. It is neither a yes nor a no, so it is +# exit 2 (could not check) rather than either verdict. +_MERGEABLE = "MERGEABLE" +_CONFLICTING = "CONFLICTING" +_UNKNOWN_MERGEABILITY = "UNKNOWN" + +# The merge-state states of the vet, in the sense of #455: the blocking state must +# block, and the permissive state must not be read as a *health* claim. +# `mergeStateStatus` is displayed but never *branched on*: it is a finer-grained view +# of the same fact (DIRTY/BEHIND/BLOCKED/UNSTABLE/...), and pinning behaviour to a +# value GitHub keeps adding states to is how a gate rots. `mergeable` is the decisive +# field because it answers the one question `gh pr merge` itself refuses on. + # 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. @@ -351,14 +410,49 @@ class Verdict: head_sha: str push_time: str push_time_exact: bool + mergeable: str = "" + merge_state: str = "" votes: list[Vote] = field(default_factory=list) valid_count: int = 0 needed: int = 3 @property def short(self) -> bool: + """Too few votes. A statement about review, not about mergeability.""" return self.valid_count < self.needed + @property + def blocked(self) -> bool: + """Enough votes, but Git cannot merge the text - so `gh pr merge` refuses. + + Kept distinct from `short` because the two call for opposite responses: + short means "come back after more review", blocked means "review is done and + this still cannot land". Collapsing them is what let six CONFLICTING PRs sit + behind six `READY` lines, each looking like it was waiting on a formality. + """ + return self.mergeable == _CONFLICTING + + @property + def ok(self) -> bool: + return not self.short and not self.blocked + + @property + def mark(self) -> str: + """`BLOCKED` whenever the text cannot merge, `SHORT` for a vote deficit. + + Blocked wins even when the votes are also short. The conflict is the + blocking fact: it has to be resolved first, and resolving it pushes a new + head, which voids every vote counted here. Reporting `SHORT` in that state + would read as "come back after more review" and send a reviewer to do work + that the next rebase throws away - the same misdirection as the original + bug, one layer down. + """ + if self.blocked: + return "BLOCKED" + if self.short: + return "SHORT" + return "READY" + def _head_push_time(head: str) -> tuple[str, bool]: """Earliest CI run creation time for this SHA, else the commit date. @@ -388,13 +482,52 @@ def _head_push_time(head: str) -> tuple[str, bool]: return committer_date, False +def _merge_state(view: dict) -> tuple[str, str]: + """`(mergeable, mergeStateStatus)` from a `gh pr view` payload, checked. + + `gh pr view --json mergeable` prints `MERGEABLE` / `CONFLICTING` / `UNKNOWN`. + The fields are required rather than defaulted: a payload that lost them (a + projection that did not apply, the failure mode this file already hit once with + `at`) must fail loud, because the absent value would read as "not blocking". + """ + mergeable = str(view.get("mergeable") or "") + state = str(view.get("mergeStateStatus") or "") + if not mergeable or not state: + raise RuntimeError( + f"PR payload has no mergeability (mergeable={mergeable!r}, " + f"mergeStateStatus={state!r}) - refusing to report a verdict, since an " + "absent conflict is indistinguishable from no conflict" + ) + return mergeable, state + + def check_pr(number: int, needed: int) -> Verdict: view = _gh_json( - ["pr", "view", str(number), "-R", REPO, "--json", "number,title,headRefOid"] + [ + "pr", + "view", + str(number), + "-R", + REPO, + "--json", + "number,title,headRefOid,mergeable,mergeStateStatus", + ] ) assert isinstance(view, dict) head = str(view["headRefOid"]) + mergeable, merge_state = _merge_state(view) + # GitHub computes mergeability lazily, so a freshly pushed head reports UNKNOWN + # for a short while. That is a question not yet answered, and the failure to + # avoid is answering it anyway - either direction would be a guess about whether + # the text merges. Same for a value this version does not know. + if mergeable not in {_MERGEABLE, _CONFLICTING}: + raise RuntimeError( + f"#{number}: mergeable={mergeable!r} (mergeStateStatus={merge_state!r}) is " + "not a computed mergeability - GitHub reports UNKNOWN until it finishes " + "computing, and this check will not guess a verdict from it" + ) + push_time, exact = _head_push_time(head) # Every page, not the first 30: a truncated list drops the newest reviews, @@ -459,6 +592,8 @@ def check_pr(number: int, needed: int) -> Verdict: head_sha=head, push_time=push_time, push_time_exact=exact, + mergeable=mergeable, + merge_state=merge_state, votes=votes, valid_count=run, needed=needed, @@ -492,7 +627,11 @@ def main(argv: list[str] | None = None) -> int: "push_time_exact": v.push_time_exact, "valid_votes": v.valid_count, "needed": v.needed, - "ready": not v.short, + "mergeable": v.mergeable, + "merge_state": v.merge_state, + "verdict": v.mark, + "enough_votes": not v.short, + "ready": v.ok, } for v in verdicts ], @@ -501,12 +640,17 @@ def main(argv: list[str] | None = None) -> int: ) else: for v in verdicts: - mark = "READY" if not v.short else "SHORT" + mark = v.mark 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}" ) + # The merge state is printed on its own line and always, so a reader + # never has to infer it. `READY` with `CONFLICTING` underneath is the + # contradiction this was fixed for, and it is worth being unable to + # produce: `mark` already refuses to say READY in that case. + print(f" merge state: {v.mergeable}/{v.merge_state}") for vote in v.votes: # The mark column answers *counting*, except that a veto is never # rendered "OK": a veto submitted at the current head is `valid` @@ -529,14 +673,32 @@ def main(argv: list[str] | None = None) -> int: 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): + # Two ways to fail, with different cures, so they are reported separately rather + # than as one "not ready": a SHORT PR needs more review; a BLOCKED one needs the + # conflict resolved, and no amount of further review will change that. A PR that + # is both is listed under BLOCKED only - see `mark` for why that is the useful + # classification rather than the pessimistic one. + blocked = [v for v in verdicts if v.blocked] + short = [v for v in verdicts if v.short and not v.blocked] + + if blocked: + also_short = " (their votes are short too, but resolving the conflict " \ + "replaces the head and voids them - review after the rebase, not before)" + print( + f"\n#{', #'.join(str(v.pr) for v in blocked)}: Git cannot merge the text " + f"({blocked[0].mergeable}). More review does not fix this - the branch has " + "to be brought up to date and the conflict resolved." + + (also_short if any(v.short for v in blocked) else ""), + file=sys.stderr, + ) + if short: 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 + + return 1 if (short or blocked) else 0 if __name__ == "__main__": diff --git a/tests/test_check_vote_count.py b/tests/test_check_vote_count.py index b8bfa489..0676c429 100644 --- a/tests/test_check_vote_count.py +++ b/tests/test_check_vote_count.py @@ -58,17 +58,32 @@ def mod(): class FakeGh: """Routes gh calls to canned answers and records them.""" - def __init__(self, reviews: list[dict], push_time: str = T0, exact: bool = True): + def __init__( + self, + reviews: list[dict], + push_time: str = T0, + exact: bool = True, + mergeable: str = "MERGEABLE", + merge_state: str = "CLEAN", + ): self.reviews = reviews self.push_time = push_time self.exact = exact + self.mergeable = mergeable + self.merge_state = merge_state 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} + return { + "number": 1, + "title": "t", + "headRefOid": HEAD, + "mergeable": self.mergeable, + "mergeStateStatus": self.merge_state, + } if args[0] == "api": joined = " ".join(args) if "actions/runs" in joined: @@ -277,6 +292,214 @@ def test_a_body_that_does_not_open_with_a_mark_but_claims_lgtm_counts(mod): assert mod._classify("## Review\nThis needs work") == "comment" +# --- the mergeable clause: enough votes is not the same as mergeable ------- + + +def _three_votes() -> list[dict]: + return [ + _approve("cyc20260911-010000", "2026-09-11T01:00:00Z"), + _approve("cyc20260911-020000", "2026-09-11T02:00:00Z"), + _approve("cyc20260911-030000", "2026-09-11T03:00:00Z"), + ] + + +def test_a_conflicting_pr_with_three_votes_is_blocked_not_ready(mod, monkeypatch, capsys): + """The bug: six PRs printed READY 3/3 while CONFLICTING and unmergeable. + + Measured 2026-09-12 on the live repo - #1152/#1151/#1145/#1142/#1141/#1136 each + reported `READY 3/3` with `mergeable=CONFLICTING`, `mergeStateStatus=DIRTY`. + Every one of them was a merge `gh pr merge` refuses, so the queue read as six PRs + waiting on a formality when it was a deadlock. + + Both halves are asserted: the verdict must not be READY, and the *count* must + still be reported. Suppressing the count would hide the review work that was + actually done, and this is a tool whose whole point is an honest count. + """ + fake = FakeGh(_three_votes(), mergeable="CONFLICTING", merge_state="DIRTY") + rc = _run(mod, monkeypatch, fake) + out = capsys.readouterr().out + assert rc == 1, "a PR that cannot be merged must not exit 0" + assert "READY" not in out, "CONFLICTING must never render as READY" + assert "BLOCKED 3/3" in out + assert "merge state: CONFLICTING/DIRTY" in out + + +def test_the_blocked_reason_names_the_conflict_and_not_more_review(mod, monkeypatch, capsys): + """The cure differs from SHORT's, so the message must not be the SHORT one. + + A reviewer told "not enough votes" goes and reviews; on a conflicting branch that + work is thrown away by the rebase that resolving the conflict requires, which + pushes a new head and voids every vote. The stderr line has to send them to the + conflict instead. + """ + fake = FakeGh(_three_votes(), mergeable="CONFLICTING", merge_state="DIRTY") + _run(mod, monkeypatch, fake) + err = capsys.readouterr().err + assert "cannot merge the text" in err + assert "Not enough votes yet" not in err, ( + "three votes are present, so the shortfall message would be false" + ) + + +def test_a_pr_that_is_both_short_and_conflicting_is_reported_as_blocked(mod, monkeypatch, capsys): + """Blocked wins the headline, and the output says why the votes do not matter yet. + + Reporting `SHORT 1/3` here would be the original bug one layer down: it names the + wrong cure. The conflict has to be resolved first, and doing that voids the vote + anyway - so the shortfall is real but not the thing to act on. + """ + fake = FakeGh( + [_approve("cyc20260911-010000", "2026-09-11T01:00:00Z")], + mergeable="CONFLICTING", + merge_state="DIRTY", + ) + rc = _run(mod, monkeypatch, fake) + # One read for both streams: `readouterr()` drains, so a second call returns "". + captured = capsys.readouterr() + assert rc == 1 + assert "BLOCKED 1/3" in captured.out, "the conflict is the blocking fact, not the vote deficit" + assert "voids them" in captured.err + + +def test_a_mergeable_pr_with_three_votes_is_ready(mod, monkeypatch, capsys): + """The approving direction of the new clause (#455: pin both states). + + Without this, a predicate that blocked *everything* would pass every other test + in this section - the failure would be in the safe direction, and safe-direction + failures are exactly the ones nobody investigates. + """ + fake = FakeGh(_three_votes(), mergeable="MERGEABLE", merge_state="CLEAN") + rc = _run(mod, monkeypatch, fake) + out = capsys.readouterr().out + assert rc == 0 + assert "READY 3/3" in out + assert "merge state: MERGEABLE/CLEAN" in out + + +def test_a_mergeable_pr_with_too_few_votes_is_still_short(mod, monkeypatch, capsys): + """The mergeable clause can only downgrade; it must not upgrade a vote deficit.""" + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z")], + mergeable="MERGEABLE", merge_state="CLEAN") + rc = _run(mod, monkeypatch, fake) + assert rc == 1 + assert "SHORT 1/3" in capsys.readouterr().out + + +def test_an_uncomputed_mergeability_fails_loud(mod, monkeypatch, capsys): + """`UNKNOWN` is a question not yet answered, not an answer of "mergeable". + + GitHub computes mergeability lazily, so a head pushed seconds ago reports + UNKNOWN. Reading that as permission would be the same class of mistake as + defaulting a missing field to "no conflict" - and the missing-field case is + already pinned below. Asserts the count is *not* printed: a verdict that was not + computed must not be shipped alongside a number that looks like one. + """ + fake = FakeGh(_three_votes(), mergeable="UNKNOWN", merge_state="UNKNOWN") + rc = _run(mod, monkeypatch, fake) + assert rc == 2 + err = capsys.readouterr().err + assert "not a computed mergeability" in err + assert "3/3" not in err + + +def test_an_unrecognised_mergeability_value_fails_loud(mod, monkeypatch, capsys): + """A value this version does not know must not fall through to "not blocking". + + Same reasoning as the `_FRESH_STATUSES` naming in check-merge-freshness.py: a new + GitHub state silently read as permission is a gate that rots without changing. + """ + fake = FakeGh(_three_votes(), mergeable="SOMETHING_NEW", merge_state="CLEAN") + assert _run(mod, monkeypatch, fake) == 2 + assert "not a computed mergeability" in capsys.readouterr().err + + +def test_a_payload_without_mergeability_fails_loud(mod, monkeypatch, capsys): + """An absent field must fail loud, for the same reason the `at` check exists. + + A dropped projection would leave `mergeable` empty, and empty is not CONFLICTING - + so a defaulting implementation would report READY for a PR nobody checked. That is + the exact shape of the `--jq` bug this file already hit once. + """ + class NoMergeFields(FakeGh): + def __call__(self, args): + payload = super().__call__(args) + if isinstance(payload, dict) and "mergeable" in payload: + payload = dict(payload) + payload.pop("mergeable") + payload.pop("mergeStateStatus") + return payload + + fake = NoMergeFields(_three_votes()) + rc = _run(mod, monkeypatch, fake) + assert rc == 2 + assert "no mergeability" in capsys.readouterr().err + + +def test_json_mode_carries_the_merge_state_and_the_verdict(mod, monkeypatch, capsys): + """Machine callers need both facts separately: the count is still the count.""" + fake = FakeGh(_three_votes(), mergeable="CONFLICTING", merge_state="DIRTY") + rc = _run(mod, monkeypatch, fake, ["1", "--json"]) + payload = json.loads(capsys.readouterr().out) + assert rc == 1 + assert payload[0]["valid_votes"] == 3 + assert payload[0]["enough_votes"] is True, "the votes are enough; the merge is not" + assert payload[0]["mergeable"] == "CONFLICTING" + assert payload[0]["merge_state"] == "DIRTY" + assert payload[0]["verdict"] == "BLOCKED" + assert payload[0]["ready"] is False + + +def test_the_mergeable_clause_never_raises_the_exit_code_above_one(mod, monkeypatch, capsys): + """Blocked is a finding (exit 1), not a could-not-check (exit 2). + + The distinction matters because exit 2 means "ask again"; a conflict is a fact + about the branch, and reporting it as a transient error would have a caller + retry forever instead of resolving it. Pinned separately from the verdict test + because conflating the two exit codes is how a CI gate becomes a no-op. + """ + for mergeable, state in [("CONFLICTING", "DIRTY"), ("CONFLICTING", "BEHIND")]: + fake = FakeGh(_three_votes(), mergeable=mergeable, merge_state=state) + assert _run(mod, monkeypatch, fake) == 1, (mergeable, state) + + +def test_the_mergeable_query_is_asked_of_the_pr_it_reports_on(mod): + """The states must come from the same `gh pr view` as the head they describe. + + A second call keyed on a different PR (or a list endpoint's cached state) would + let the merge state describe a different PR than the votes - the two facts would + be about different things, with nothing in the output to show it. + """ + fake = FakeGh(_three_votes()) + mod._gh_json = fake + view = fake(["pr", "view", "1", "-R", mod.REPO, "--json", + "number,title,headRefOid,mergeable,mergeStateStatus"]) + mergeable, state = mod._merge_state(view) + assert (mergeable, state) == ("MERGEABLE", "CLEAN") + + +def test_the_pr_view_actually_requests_the_merge_fields(mod, monkeypatch, capsys): + """A fixture that answers from a keyed dict cannot catch a missing field. + + `FakeGh.__call__` returns its canned payload whatever fields were asked for, so + the tests above would pass even if the real `--json` list never asked for + mergeability - the tool would then fail loud on every real PR while the suite + stayed green. So the requested field list itself is asserted. + """ + seen: list[list[str]] = [] + + class Recording(FakeGh): + def __call__(self, args): + if args[:2] == ["pr", "view"]: + seen.append(list(args)) + return super().__call__(args) + + fake = Recording(_three_votes()) + assert _run(mod, monkeypatch, fake) == 0 + assert seen, "the tool must ask gh for the PR" + fields = " ".join(seen[0]) + assert "mergeable" in fields and "mergeStateStatus" in fields, seen[0] + + # --- the run rule ---------------------------------------------------------- From e8b1a5d7fc86b5d43c0756066c8c7c691af07371 Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Sat, 12 Sep 2026 19:15:01 +0800 Subject: [PATCH 2/4] emrg: read the whole merge gate, so a draft PR is not reported ready check-vote-count.py branched only on `mergeable` and ignored `mergeStateStatus`, so with three valid votes it printed `READY` and exited 0 for MERGEABLE/UNSTABLE (CI not green), /BEHIND, /BLOCKED and /DRAFT - and a draft pull request cannot be merged by anyone. Its own docstring spelled the gate as the pair MERGEABLE/CLEAN, and its own tests could not catch the gap: every one of them used CLEAN or DIRTY, i.e. only the two states the code happened to branch on. The known states are now enumerated (each with the reader's reason) and an unrecognised one fails loud, which buys the rot-resistance the old "never branch on the field" comment was reaching for without also passing every state it covered. Both mutants die: disabling the unknown-state refusal and restoring the pre-fix predicate each leave the new tests red. --- Agent.md | 2 +- scripts/check-vote-count.py | 119 ++++++++++++++++++++++++++------- tests/test_check_vote_count.py | 89 ++++++++++++++++++++++++ 3 files changed, 184 insertions(+), 26 deletions(-) diff --git a/Agent.md b/Agent.md index f91a8d1c..3994665f 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` (1542) — import check: `uv run python -c "from emrg.client.app import run_client" +Python: `uv run pytest tests/ -v` (1546) — 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 上下文) diff --git a/scripts/check-vote-count.py b/scripts/check-vote-count.py index 8fdf3c5c..df57f4f8 100644 --- a/scripts/check-vote-count.py +++ b/scripts/check-vote-count.py @@ -69,20 +69,29 @@ votes. Reported as `BLOCKED`, which is a *different* state from `SHORT`: short means "come back after more review", blocked means "review is done and this still cannot land" - the state that sat invisible behind six `READY` lines. +* `MERGEABLE`, but any `mergeStateStatus` other than `CLEAN` - the gate is the + *pair*, so a `MERGEABLE` PR that is `UNSTABLE` (checks failing or unfinished), + `BEHIND`, `BLOCKED` or `DRAFT` is also blocked. Reading only `mergeable` is what + let a **draft** pull request - which no vote can merge - print `READY`. * `UNKNOWN` - GitHub has not computed mergeability yet (usual right after a push). Not a yes and not a no, so this fails loud (exit 2) rather than printing either. - An unrecognised value fails loud for the same reason. -* `MERGEABLE` - the text merges. This is deliberately **not** taken as evidence that - merging is safe: as `check-merge-freshness.py` documents, GitHub answers "does this - textually merge", and a clean auto-merge of two same-valued count lines is the - *dangerous* case, not the safe one. The clause is used only in the direction where - it is decisive (a conflict is a hard no), and the states are printed so a reader + A `mergeStateStatus` this version does not recognise fails loud for the same + reason: an unknown state is not evidence of cleanliness. (Enumerating the states + and refusing the rest buys the rot-resistance that "never branch on the field" was + reaching for, without also passing every state that enumeration covers.) +* `MERGEABLE`/`CLEAN` - the text merges and GitHub is not withholding the merge. + This is deliberately **not** taken as evidence that merging is safe: as + `check-merge-freshness.py` documents, GitHub answers "does this textually merge", + and a clean auto-merge of two same-valued count lines is the *dangerous* case, not + the safe one. The clause is used only in the direction where it is decisive (a + conflict or a withheld merge is a hard no), and the states are printed so a reader sees them without being told they are fine. -Two sibling questions stay with their own tools, and are named here so this one does -not quietly pretend to answer them: `check-merge-freshness.py` asks whether the CI -verdict is still about the tree that would merge (so this tool does not re-check CI), -and `check-merge-tree-health.py` (PR #1155) asks whether the merged tree passes the +One sibling question stays with its own tool, named here so this one does not +quietly pretend to answer it: `check-merge-freshness.py` asks whether the CI verdict +is still about the tree that would merge (a question about *which* tree ran CI, not +about whether checks pass - `UNSTABLE` answers that one, and is read above). Likewise +`check-merge-tree-health.py` (PR #1155) asks whether the merged tree passes the repo's own guards. Push time, and the honest bound @@ -104,11 +113,12 @@ Exit codes ---------- - 0 every PR has >= --min-votes (default 3) valid votes **and** is mergeable + 0 every PR has >= --min-votes (default 3) valid votes **and** is + `MERGEABLE`/`CLEAN` 1 at least one PR is SHORT (too few votes) or BLOCKED (cannot be merged) 2 the check could not be made (gh failed, unparseable response, mergeability - not computed yet) - fail loud; never report a count for a question that was - not answered + not computed yet, merge state not recognised) - fail loud; never report a + count for a question that was not answered The count is printed either way, so a PR that is BLOCKED still reports its votes; `--json` carries `valid_votes` and the merge states as separate fields for a @@ -141,12 +151,44 @@ _CONFLICTING = "CONFLICTING" _UNKNOWN_MERGEABILITY = "UNKNOWN" -# The merge-state states of the vet, in the sense of #455: the blocking state must -# block, and the permissive state must not be read as a *health* claim. -# `mergeStateStatus` is displayed but never *branched on*: it is a finer-grained view -# of the same fact (DIRTY/BEHIND/BLOCKED/UNSTABLE/...), and pinning behaviour to a -# value GitHub keeps adding states to is how a gate rots. `mergeable` is the decisive -# field because it answers the one question `gh pr merge` itself refuses on. +# The merge gate is spelled **`MERGEABLE`/`CLEAN`** - two fields - and an earlier +# version of this tool read only the first, on the theory that `mergeStateStatus` is +# "a finer-grained view of the same fact" and branching on it would rot as GitHub +# adds states. That theory was wrong twice over, and the second time was measured +# (cyc20260912-190602): with three valid votes the tool printed +# **`READY` and exited 0** for `MERGEABLE`/`UNSTABLE` (checks failing or still +# running), `/BEHIND`, `/BLOCKED` and `/DRAFT`. A **draft** pull request cannot be +# merged by anyone, and `UNSTABLE` is precisely "CI is not green" - the third +# conjunct this docstring claims only a sibling tool checks. So the second field is +# read, and the spelling is the gate's own. +# +# Rot-resistance is bought the other way round from before: rather than *ignoring* +# the field, the known states are enumerated and anything unrecognised **fails loud** +# (exit 2). A state GitHub adds later is then reported as "could not check" instead +# of silently passing as permission - which is what "never branch on it" was +# actually trying to buy, at the cost of the false READY above. +_CLEAN = "CLEAN" + +# The states in which the merge cannot proceed right now, each with the reason the +# reader needs. GitHub's `MergeStateStatus` vocabulary: +# +# DIRTY the merge conflicts +# UNSTABLE mergeable, but commit status is not passing <- the CI conjunct +# BEHIND the head is out of date with the base branch +# BLOCKED GitHub blocks the merge (protection rules / required reviews) +# DRAFT the pull request is a draft +_NON_CLEAN_STATES = { + "DIRTY": "the merge conflicts", + "UNSTABLE": "checks are failing or have not finished", + "BEHIND": "the head is behind the base branch", + "BLOCKED": "GitHub reports the merge blocked (protection rules or required reviews)", + "DRAFT": "the pull request is a draft", +} + +# `HAS_HOOKS` ("merge commits are conditioned on hooks") is deliberately NOT in the +# map: whether it permits a merge is not something this tool can establish, and +# guessing either way would put an unverified verdict behind a gate. It falls to the +# fail-loud branch with every other unrecognised value. # 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 @@ -423,14 +465,29 @@ def short(self) -> bool: @property def blocked(self) -> bool: - """Enough votes, but Git cannot merge the text - so `gh pr merge` refuses. + """The merge cannot proceed: the gate's `MERGEABLE`/`CLEAN` is not satisfied. Kept distinct from `short` because the two call for opposite responses: short means "come back after more review", blocked means "review is done and this still cannot land". Collapsing them is what let six CONFLICTING PRs sit behind six `READY` lines, each looking like it was waiting on a formality. + + Covers every non-clean state, not only a conflict. An earlier version tested + `mergeable == CONFLICTING` and so reported `READY` for `MERGEABLE`/`UNSTABLE` + (CI not green), `/BEHIND`, `/BLOCKED` and `/DRAFT` - a draft PR, which nobody + can merge at all. The gate's spelling is the pair, so the pair is tested. """ - return self.mergeable == _CONFLICTING + return self.mergeable == _CONFLICTING or self.merge_state in _NON_CLEAN_STATES + + @property + def block_reason(self) -> str: + """Why this PR cannot merge, in the reader's terms.""" + if self.mergeable == _CONFLICTING: + return "Git cannot merge the text (CONFLICTING)" + reason = _NON_CLEAN_STATES.get(self.merge_state) + if reason: + return f"merge state is {self.merge_state} - {reason}" + return "" @property def ok(self) -> bool: @@ -527,6 +584,16 @@ def check_pr(number: int, needed: int) -> Verdict: "not a computed mergeability - GitHub reports UNKNOWN until it finishes " "computing, and this check will not guess a verdict from it" ) + # The second half of the gate's spelling. `MERGEABLE` alone is not `MERGEABLE`/ + # `CLEAN`: every other state either blocks the merge or says the CI conjunct is + # unmet, and a state this version does not know is not evidence of cleanliness. + if mergeable == _MERGEABLE and merge_state not in {_CLEAN, *_NON_CLEAN_STATES}: + raise RuntimeError( + f"#{number}: mergeStateStatus={merge_state!r} is not a state this check " + f"knows (known: CLEAN, {', '.join(sorted(_NON_CLEAN_STATES))}). It is not " + "read as permission - an unrecognised state may well block the merge, and " + "reporting READY from it would be a verdict this tool has not verified" + ) push_time, exact = _head_push_time(head) @@ -682,12 +749,14 @@ def main(argv: list[str] | None = None) -> int: short = [v for v in verdicts if v.short and not v.blocked] if blocked: - also_short = " (their votes are short too, but resolving the conflict " \ + also_short = " (their votes are short too, but resolving the block " \ "replaces the head and voids them - review after the rebase, not before)" + reasons = "; ".join( + f"#{v.pr}: {v.block_reason}" for v in blocked + ) print( - f"\n#{', #'.join(str(v.pr) for v in blocked)}: Git cannot merge the text " - f"({blocked[0].mergeable}). More review does not fix this - the branch has " - "to be brought up to date and the conflict resolved." + f"\n{reasons}. More review does not fix this - the branch or the pull " + "request has to be made mergeable first." + (also_short if any(v.short for v in blocked) else ""), file=sys.stderr, ) diff --git a/tests/test_check_vote_count.py b/tests/test_check_vote_count.py index 0676c429..68b5bc70 100644 --- a/tests/test_check_vote_count.py +++ b/tests/test_check_vote_count.py @@ -462,6 +462,95 @@ def test_the_mergeable_clause_never_raises_the_exit_code_above_one(mod, monkeypa assert _run(mod, monkeypatch, fake) == 1, (mergeable, state) +# --- the gate is TWO fields, and the second one was ignored ----------------- +# +# `MERGEABLE` alone is not the gate's spelling: evolution_prompt Step 5 requires +# `MERGEABLE`/`CLEAN`. An earlier version of this tool read only `mergeable` and +# documented the omission as deliberate ("`mergeStateStatus` is a finer-grained view +# of the same fact ... never branched on"). Measured (cyc20260912-190602): with +# three valid votes it printed `READY` and exited 0 for all four short cases below - +# including `DRAFT`, which cannot be merged by anyone, and `UNSTABLE`, which *is* +# the CI conjunct this tool's docstring says it leaves to a sibling. The existing +# tests could not catch it: every one of them used `merge_state="CLEAN"` or +# `"DIRTY"`, i.e. only the two states the code happened to branch on. + + +def test_every_non_clean_merge_state_blocks_with_enough_votes(mod, monkeypatch, capsys): + """One case per state, because the bug was an unhandled *state*, not a state. + + A single test asserting that one non-clean state blocks would have passed on the + old code (it handled `DIRTY`) while the other four stayed broken - which is + exactly how this shipped. + """ + for state, why in [ + ("DIRTY", "conflicts"), + ("UNSTABLE", "checks are failing"), + ("BEHIND", "behind the base"), + ("BLOCKED", "protected"), + ("DRAFT", "a draft"), + ]: + fake = FakeGh(_three_votes(), mergeable="MERGEABLE", merge_state=state) + rc = _run(mod, monkeypatch, fake) + captured = capsys.readouterr() + assert rc == 1, f"MERGEABLE/{state} must block, not exit 0" + assert "READY" not in captured.out, f"MERGEABLE/{state} rendered READY" + assert "BLOCKED" in captured.out, f"MERGEABLE/{state} should read BLOCKED" + assert state in captured.err, f"the reason must name {state}" + + +def test_a_draft_pull_request_is_never_reported_as_ready(mod, monkeypatch, capsys): + """The clearest case, pinned on its own so it cannot be lost in a loop. + + `DRAFT` is not a mergeability question at all - no reviewer vote can merge a + draft. Reporting it `READY` is not a near-miss reading; it is a false statement + about a PR that cannot land. + """ + fake = FakeGh(_three_votes(), mergeable="MERGEABLE", merge_state="DRAFT") + rc = _run(mod, monkeypatch, fake) + out = capsys.readouterr().out + assert rc == 1 + assert "READY" not in out + assert "DRAFT" in out + + +def test_unstable_is_named_as_the_ci_conjunct(mod, monkeypatch, capsys): + """`UNSTABLE` is "checks failing or unfinished" - i.e. CI is not green. + + The tool's own docstring says the CI conjunct is a sibling's question; that is + true of *whether the verdict is stale*, but not of *whether checks pass*, and + GitHub already answers the latter here. So this is pinned as a blocked state, + with the reason saying checks, not "conflict" - the old single-reason message + would have told the reader to resolve a conflict that does not exist. + """ + fake = FakeGh(_three_votes(), mergeable="MERGEABLE", merge_state="UNSTABLE") + _run(mod, monkeypatch, fake) + err = capsys.readouterr().err + assert "checks" in err + assert "conflict" not in err.lower(), ( + "UNSTABLE is a CI problem; telling the reader to resolve a conflict sends " + "them after something that is not there" + ) + + +def test_an_unknown_merge_state_fails_loud_rather_than_passing(mod, monkeypatch, capsys): + """A state GitHub adds later must not be read as permission. + + This is the rot-resistance the old "never branch on it" comment was reaching + for. Enumerating the *known* states and rejecting the rest buys the same + property without also passing every state the enumeration covers, which is what + the ignore-the-field version got wrong. + """ + fake = FakeGh(_three_votes(), mergeable="MERGEABLE", merge_state="HAS_HOOKS") + rc = _run(mod, monkeypatch, fake) + assert rc == 2, "an unrecognised merge state is a could-not-check, not a pass" + captured = capsys.readouterr() + assert "not a state this check knows" in captured.err + # Assert on the verdict stream, not on the error prose: the refusal message + # itself says "reporting READY from it would be ..." - a substring check on + # stderr would fail on the very sentence whose absence it means to prove. + assert "READY" not in captured.out + + def test_the_mergeable_query_is_asked_of_the_pr_it_reports_on(mod): """The states must come from the same `gh pr view` as the head they describe. From 81ea176629039937b456a647f9be6629324aa774 Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Sat, 12 Sep 2026 20:00:42 +0800 Subject: [PATCH 3/4] emrg: a head with no CI run is not CLEAN, it is the CI conjunct unverified MergeStateStatus counts *required* checks, and this repo has no branch protection and no rulesets, so a head with zero check runs reports MERGEABLE/CLEAN - the same value a double-green head reports. The docstring claimed such a PR 'is not mergeable anyway' and used the missing run only to approximate the push time. Measured 2026-09-12: three historical heads (c0860a35 / af2e0efd / 5358d294) have zero workflow runs each and all three report MERGEABLE/CLEAN; a probe with that payload reaches READY and exit 0 on three valid votes - a PR no CI has ever judged, reported mergeable. A probe PR (#1171) also confirms the other direction: a *failed* check does yield UNSTABLE here, so the state is trustworthy when there is a run and silent when there is none. So the missing run is treated as blocking, with a reason that names it (both merge fields are clean, so 'blocked' alone gives a reader nothing to act on). Two mutants killed: removing the clause -> 4 red; making it unconditional -> 15 red. --- Agent.md | 2 +- scripts/check-vote-count.py | 51 ++++++++++++++++++++++--- tests/test_check_vote_count.py | 68 +++++++++++++++++++++++++++++++++- 3 files changed, 112 insertions(+), 9 deletions(-) diff --git a/Agent.md b/Agent.md index 3994665f..7c198e2e 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` (1546) — import check: `uv run python -c "from emrg.client.app import run_client" +Python: `uv run pytest tests/ -v` (1550) — 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 上下文) diff --git a/scripts/check-vote-count.py b/scripts/check-vote-count.py index df57f4f8..964f2d80 100644 --- a/scripts/check-vote-count.py +++ b/scripts/check-vote-count.py @@ -102,8 +102,19 @@ 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. +silently trusted. The fallback is also the case where the PR has no CI at all - and +that is the third conjunct failing, so it **blocks** rather than just being disclosed. + +An earlier version of this note claimed such a PR "is not mergeable anyway" and used +the fallback for the count alone. Measured 2026-09-12: the claim is false. +`MergeStateStatus` is computed from *required* checks, and this repo has no branch +protection and no rulesets, so a head with **zero** checks is not `PENDING` or +`UNSTABLE` - it is `CLEAN`, indistinguishable in the merge state from a double-green +head. Three historical PRs have exactly that shape (heads `c0860a35`, `af2e0efd`, +`5358d294`; zero workflow runs each, all three reported `MERGEABLE`/`CLEAN`), and a +probe with that payload reached `READY`/exit 0 on three votes. So a missing run is not +a nuance about the count - it is the CI conjunct unverified, and it is treated as +blocking. Usage ----- @@ -114,8 +125,9 @@ Exit codes ---------- 0 every PR has >= --min-votes (default 3) valid votes **and** is - `MERGEABLE`/`CLEAN` - 1 at least one PR is SHORT (too few votes) or BLOCKED (cannot be merged) + `MERGEABLE`/`CLEAN` **and** has a CI run for its head commit + 1 at least one PR is SHORT (too few votes) or BLOCKED (cannot be merged: + conflicting, a non-clean merge state, or no CI run for the head) 2 the check could not be made (gh failed, unparseable response, mergeability not computed yet, merge state not recognised) - fail loud; never report a count for a question that was not answered @@ -476,8 +488,20 @@ def blocked(self) -> bool: `mergeable == CONFLICTING` and so reported `READY` for `MERGEABLE`/`UNSTABLE` (CI not green), `/BEHIND`, `/BLOCKED` and `/DRAFT` - a draft PR, which nobody can merge at all. The gate's spelling is the pair, so the pair is tested. + + A head with **no CI run** is blocking too, and it is the case the merge state + cannot express: `MergeStateStatus` counts *required* checks, and with no + branch protection a head that ran nothing is not `PENDING` or `UNSTABLE` but + `CLEAN` - the same value a double-green head reports. Measured 2026-09-12 on + three historical PRs (`c0860a35`, `af2e0efd`, `5358d294`: zero runs, all + `MERGEABLE`/`CLEAN`). So the third conjunct is checked against the run + itself, not against the state that is documented not to carry it. """ - return self.mergeable == _CONFLICTING or self.merge_state in _NON_CLEAN_STATES + return ( + self.mergeable == _CONFLICTING + or self.merge_state in _NON_CLEAN_STATES + or not self.push_time_exact + ) @property def block_reason(self) -> str: @@ -487,6 +511,12 @@ def block_reason(self) -> str: reason = _NON_CLEAN_STATES.get(self.merge_state) if reason: return f"merge state is {self.merge_state} - {reason}" + if not self.push_time_exact: + return ( + "no CI run exists for the head commit, so the CI conjunct is not " + "verified (the merge state cannot show this: with no required checks " + "a head that ran nothing still reads CLEAN)" + ) return "" @property @@ -696,6 +726,8 @@ def main(argv: list[str] | None = None) -> int: "needed": v.needed, "mergeable": v.mergeable, "merge_state": v.merge_state, + "ci_ran": v.push_time_exact, + "blocked": v.blocked, "verdict": v.mark, "enough_votes": not v.short, "ready": v.ok, @@ -708,7 +740,14 @@ def main(argv: list[str] | None = None) -> int: else: for v in verdicts: mark = v.mark - src = "" if v.push_time_exact else " (no CI run: push time approximated by commit date)" + # A missing run is both a caveat about the count *and* the CI conjunct + # unverified, so the line says which: `mark` is already BLOCKED here, + # and a reader should not have to infer why from a parenthetical. + src = ( + "" + if v.push_time_exact + else " (no CI run: push time approximated by commit date; blocked)" + ) print( f"#{v.pr} {mark} {v.valid_count}/{v.needed} valid votes " f"(head {v.head_sha[:8]}, pushed {v.push_time}){src}" diff --git a/tests/test_check_vote_count.py b/tests/test_check_vote_count.py index 68b5bc70..83352fe6 100644 --- a/tests/test_check_vote_count.py +++ b/tests/test_check_vote_count.py @@ -683,13 +683,77 @@ def test_the_push_time_fallback_is_disclosed_not_silently_used(mod, monkeypatch, """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. + direction: it can let a vote count that should not. It is also the case where + the PR has no CI run at all, and that is the third conjunct unverified, so the + same input now *blocks* as well as being disclosed. Both are asserted here: + the disclosure is still what tells a reader the count is approximate, and the + block is what keeps `READY` off a head nothing ever ran on. """ 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 + assert "BLOCKED" in out + + +def test_a_head_with_no_ci_run_is_blocked_even_with_three_votes(mod, monkeypatch, capsys): + """The regression this pins: `CLEAN` does not mean checks ran. + + `MergeStateStatus` counts *required* checks, and this repo has no branch + protection and no rulesets, so a head with zero check runs reports `CLEAN` - + the same value a double-green head reports. Measured 2026-09-12 on three + historical PRs (heads c0860a35 / af2e0efd / 5358d294: zero workflow runs each, + all three `MERGEABLE`/`CLEAN`). An earlier version read the merge state alone + and therefore printed READY, exit 0, for a head no CI had ever judged. + """ + fake = FakeGh( + [_approve(f"cyc2026091{i}-010000", f"2026-09-1{i}T01:00:00Z") for i in (1, 2, 3)], + exact=False, + mergeable="MERGEABLE", + merge_state="CLEAN", + ) + rc = _run(mod, monkeypatch, fake) + assert rc == 1, "a head with no CI run must not exit 0" + out = capsys.readouterr().out + assert "BLOCKED" in out + assert "READY" not in out + + +def test_the_no_ci_block_names_the_missing_run(mod, monkeypatch, capsys): + """The reason has to say what is missing, not just that something is. + + `mergeable`/`mergeStateStatus` are both clean here, so a reader who is told + only "blocked" has nothing to act on - the state looks perfect. + """ + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z")], exact=False) + _run(mod, monkeypatch, fake) + err = capsys.readouterr().err + assert "no CI run" in err + + +def test_a_head_with_a_ci_run_is_not_blocked_for_that_reason(mod, monkeypatch, capsys): + """The other arm of the same predicate: with a run, nothing here blocks. + + Without this, a `blocked` that returned True unconditionally would pass every + test above.""" + fake = FakeGh( + [_approve(f"cyc2026091{i}-010000", f"2026-09-1{i}T01:00:00Z") for i in (1, 2, 3)], + exact=True, + ) + rc = _run(mod, monkeypatch, fake) + assert rc == 0 + assert "READY" in capsys.readouterr().out + + +def test_json_mode_carries_the_ci_conjunct(mod, monkeypatch, capsys): + """`ci_ran` is reported separately, so a caller does not have to infer it from + the verdict (the same reason the merge fields are exposed).""" + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z")], exact=False) + _run(mod, monkeypatch, fake, ["1", "--json"]) + payload = json.loads(capsys.readouterr().out) + assert payload[0]["ci_ran"] is False + assert payload[0]["blocked"] is True + assert payload[0]["ready"] is False def test_an_exact_push_time_is_not_flagged(mod, monkeypatch, capsys): From 7f8fb5a669ca76f20178b6d07f2a35ad410b7ef9 Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Sun, 13 Sep 2026 11:54:48 +0800 Subject: [PATCH 4/4] emrg: a vote that does not count must not be labelled counts --- scripts/check-vote-count.py | 31 ++++++++++++++++++++++++++++--- tests/test_check_vote_count.py | 26 ++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/scripts/check-vote-count.py b/scripts/check-vote-count.py index 964f2d80..75cdd478 100644 --- a/scripts/check-vote-count.py +++ b/scripts/check-vote-count.py @@ -467,6 +467,10 @@ class Verdict: mergeable: str = "" merge_state: str = "" votes: list[Vote] = field(default_factory=list) + # Parallel to `votes`: whether each one contributes to `valid_count`. A valid + # approval whose cycle already appears earlier in the run does not, and the + # label column must say so rather than calling it "counts". + counted: list[bool] = field(default_factory=list) valid_count: int = 0 needed: int = 3 @@ -674,14 +678,23 @@ def check_pr(number: int, needed: int) -> Verdict: # cycle at most once inside the trailing run. run = 0 seen: set[str] = set() + # Which votes `run` is actually made of. The label column says "counts" only for + # these, because a valid approval can still fail to count: counting is per + # *cycle*, so a second vote from a cycle already in the run is valid (it is + # about this head, and it carries a cycle id) but contributes nothing. + counted: list[bool] = [] for v in votes: if v.kind == "veto": run = 0 seen.clear() + counted.append(True) 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 + counted.append(True) + else: + # invalid approvals and repeat-cycle approvals leave the run untouched + counted.append(False) return Verdict( pr=number, @@ -692,6 +705,7 @@ def check_pr(number: int, needed: int) -> Verdict: mergeable=mergeable, merge_state=merge_state, votes=votes, + counted=counted, valid_count=run, needed=needed, ) @@ -757,7 +771,13 @@ def main(argv: list[str] | None = None) -> int: # contradiction this was fixed for, and it is worth being unable to # produce: `mark` already refuses to say READY in that case. print(f" merge state: {v.mergeable}/{v.merge_state}") - for vote in v.votes: + for index, vote in enumerate(v.votes): + # Whether *this* vote is part of the count `run` -- not whether it + # could be: `valid` only says it is about this head. The two come + # apart for a cycle's second vote, which counts once. + contributes = ( + v.counted[index] if index < len(v.counted) else vote.valid + ) # The mark column answers *counting*, except that a veto is never # rendered "OK": a veto submitted at the current head is `valid` # (it is about this head, and it carries a cycle id) while meaning @@ -770,7 +790,12 @@ def main(argv: list[str] | None = None) -> int: mark = "NO " note = "counts - resets the run" if vote.valid else vote.why elif vote.valid: - mark, note = "OK ", "counts" + mark = "OK " + note = ( + "counts" + if contributes + else f"valid, but cycle {vote.cycle} already counted" + ) else: # An approval that predates the head push is a real ✅ and still # does not count; rendering it "OK ... VOID" would contradict diff --git a/tests/test_check_vote_count.py b/tests/test_check_vote_count.py index 83352fe6..83f5100b 100644 --- a/tests/test_check_vote_count.py +++ b/tests/test_check_vote_count.py @@ -650,6 +650,32 @@ def test_a_repeat_cycle_inside_the_run_counts_once(mod, monkeypatch, capsys): assert "SHORT 1/3" in capsys.readouterr().out +def test_a_repeat_cycle_vote_is_not_labelled_counts(mod, monkeypatch, capsys): + """The label column must agree with the count it summarises. + + Counting is per cycle, so a cycle's second approval inside the run is valid but + contributes nothing. It used to be printed `OK ... counts` all the same, so the + per-vote lines disagreed with the `N/3` on the line above them - measured + 2026-09-13 (`cyc20260913-114142`) on the live queue, where 5 of 14 open PRs + printed more `counts` lines than votes: #1151 showed five, then reported `3/3`. + + What is pinned: exactly one line per *counted* vote says "counts", the redundant + ones name the repeat, and the number of "counts" lines equals `valid_count`. The + third assertion is the one that would have caught it - it compares the summary + against the detail instead of checking either against a literal. + """ + fake = FakeGh([_approve("cyc20260911-010000", "2026-09-11T01:00:00Z"), + _approve("cyc20260911-020000", "2026-09-11T02:00:00Z"), + _approve("cyc20260911-020000", "2026-09-11T03:00:00Z"), + _approve("cyc20260911-030000", "2026-09-11T04:00:00Z")]) + assert _run(mod, monkeypatch, fake) == 0 + out = capsys.readouterr().out + + assert "READY 3/3" in out, out + assert out.count("- counts") == 3, out + assert "valid, but cycle cyc20260911-020000 already counted" in out, 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.