diff --git a/Agent.md b/Agent.md index bce0e6fe..066d7d5f 100644 --- a/Agent.md +++ b/Agent.md @@ -119,11 +119,12 @@ 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` (1494) — import check: `uv run python -c "from emrg.client.app import run_client" +Python: `uv run pytest tests/ -v` (1508) — 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 上下文) Re-trigger: `scripts/re-trigger-ci.sh [branch]` (workflow_dispatch, #527 — 替代空 commit 重触发：Actions outage 会整段丢弃 push 事件，dispatch 走 API 路径不受影响) +Merge freshness: `uv run --no-sync python3 scripts/check-merge-freshness.py ...` — 合并前问一句「这条绿色 CI 说的还是**将要合并的那棵树**吗」。`pull_request` 事件下 GitHub 构建的是 `Merge into `（合到**分叉点**，不是当前 master）：分叉点就是 master 时两者同一棵树，master 一移动就不是了，而 master 移动**不触发** `synchronize`（只有 push 分支才触发），于是绿灯永远保持绿灯却已经过期。#1137 实测：`MERGEABLE/CLEAN` + 双 job 全绿，两侧计数行都写成同一个 1397 ⇒ git **无冲突**自动合并、保留 1397，而合并树实收 1401，两个守卫在 master 上才变红——**干净合并才是危险的那一种**（冲突时人被迫看一眼，反而安全）。判定不用时间戳比较（时钟/秒级竞态会骗人）而用**图结构**：master 的 tip 是否是 head 的祖先（`compare/master...` 的 `identical`/`ahead`），是则 merge base 就是 master 本身、判决可平移。**祖先性只是一半**：head 含 master 但**根本没有 CI 运行**（push 事件丢失 ⇒ `no checks reported`）同样不算新鲜，故第二个条件是「该 SHA 上存在一个**通过**的运行」。按 SHA 而非分支取运行（分支推两次会有两个运行）。exit 0 全部新鲜 / 1 至少一条过期 / 2 问不出来（坏 PR、gh 失败、状态不认识）——不认识的状态一律 fail-loud，绝不默认新鲜 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 即报该原因，绝不报假数 diff --git a/scripts/check-merge-freshness.py b/scripts/check-merge-freshness.py new file mode 100644 index 00000000..dc79d01f --- /dev/null +++ b/scripts/check-merge-freshness.py @@ -0,0 +1,343 @@ +#!/usr/bin/env python3 +"""Check that a pull request's green CI still describes the tree that would merge. + +The class this exists for +------------------------ +On 2026-09-11 PR #1137 was `MERGEABLE/CLEAN` with both CI jobs green. The merge +was nevertheless unsafe, and the way it was unsafe is reproducible: + + merge base cb651a4 : Agent.md count line 1393 own collection 1393 + ours ea0a06a : Agent.md count line 1397 own collection 1397 (+4 tests) + master 64bab52 : Agent.md count line 1397 own collection 1397 (+4 other tests) + +Both sides set the count line to the same number, so git merged it without a +conflict, kept 1397, and the merged tree collected 1401. Two guards +(`test_doc_counts.py::test_python_count_matches_docs` and +`test_check_doc_count.py::test_real_tree_is_consistent`) went red - *after* the +merge, on master, where nobody was looking. + +The CI verdict was not wrong. It was about a different tree. On `pull_request` +GitHub builds `Merge into ` - the head merged onto the branch +point, **not** onto current master. While the branch point is master's tip those +are the same tree; once master moves they are not, and nothing re-runs the check: +the `synchronize` event fires on a branch push, and master moving is not a branch +push. + +The question, made structural +----------------------------- +"Is this verdict current?" reduces exactly to: **is master's tip an ancestor of +the head?** If it is, the merge base *is* master's tip, so the tree CI built and +the tree that would merge are the same commit and the verdict transfers. If it is +not, CI judged a merge onto an older master and the verdict is about a tree that +can no longer be merged. + +That is a graph property, so this tool asks the graph instead of comparing +timestamps. Deliberately so: two timestamps are a proxy that can be wrong (clock +skew, a run created a second before the merge commit), whereas ancestry is the +thing itself. It also gets the "master has not moved" case right for free - the +verdict is simply fresh, and no re-run is needed. + +Ancestry alone is only half the question, so a second condition is checked +---------------------------------------------------------------------------- +Ancestry answers "would a verdict transfer". It does not answer "is there a +verdict". A head can contain master's tip and still have **no CI run at all** - a +dropped push event leaves the branch with zero checks, which reads as `no checks +reported` and is not evidence of anything. So a head only counts as FRESH if it +is an ancestor-descendant of master **and** a run exists for that exact SHA. + +Keyed on the SHA, not the branch: a branch pushed twice has two runs, and reading +the older one as the current verdict is the same class of mistake in miniature. +The query asks GitHub for that SHA's runs directly, so there is no window to fall +out of either. It is also pinned to the workflow whose verdict is being claimed - +`test.yml` - because "some passing run" is only the test verdict while nothing +else happens to run on a PR head. + +The run is also required to have *passed* - a failing or cancelled run is not a +stale verdict, it is a verdict the committer has to deal with on its own terms, +and this tool says so rather than calling it fresh. + +Why the obvious shortcut is wrong +--------------------------------- +`gh pr view --json mergeable` returns `CLEAN` here and is actively misleading: +GitHub computes mergeability as "does this textually merge", which is exactly the +property that failed. A cleanly auto-merged line is the *dangerous* case - when +the count line conflicts, a human is forced to look at it. + +Usage +----- + uv run --no-sync python3 scripts/check-merge-freshness.py [ ...] + uv run --no-sync python3 scripts/check-merge-freshness.py --json + +Exit codes +---------- + 0 every head contains master's tip - CI's merge base is master itself + 1 at least one head does NOT contain master's tip - the verdict is stale + 2 the check could not be made (bad PR, gh failed, unreadable response) - + fail loud; never report "fresh" for a question that was not answered + +`gh` is required, and so is network access to GitHub. There is no offline mode: +the whole question is about a remote verdict, and a local guess would be the +failure mode this tool exists to prevent. +""" + +from __future__ import annotations + +import argparse +import json +import subprocess +import sys +from dataclasses import dataclass + +REPO = "argszero/emrg" + +# GitHub `compare` statuses, split by the one property that decides freshness: +# is master's tip an ancestor of the head? +# +# identical / ahead -> master is an ancestor -> merge base IS master -> fresh +# behind / diverged -> master is not an ancestor -> stale +# +# Both sets are named rather than expressed as `status == "ahead"`, so an +# unrecognised status (a new GitHub value) falls through to the fail-loud branch +# instead of being silently treated as fresh. +_FRESH_STATUSES = frozenset({"identical", "ahead"}) +_STALE_STATUSES = frozenset({"behind", "diverged"}) + +# A run that has not concluded yet has judged nothing, so it is not a verdict to +# expire - it is a verdict still being formed. Reported as such, never as fresh. +_UNFINISHED = frozenset({"", "pending", "queued", "in_progress", "requested", "waiting"}) + +# The workflow whose verdict this tool speaks about, by the name GitHub reports. +# `test.yml` ("Test") is the only workflow triggered by `pull_request` in this +# repo, so it is the one whose greenness a merge rests on. Pinned rather than +# "any passing run": the claim "the verdict is about the tests" was otherwise +# carried by coincidence (today nothing else runs on a PR head), and the first +# workflow added on a branch would silently become the verdict instead. A PR head +# that has runs *but none from this workflow* is reported distinctly, so a rename +# here reads as "the verdict workflow did not run", not as "the branch has no CI". +_VERDICT_WORKFLOW = "Test" + + +def _gh_json(args: list[str]) -> object: + """Run `gh` and parse JSON, failing loud rather than guessing. + + `args` are gh's arguments *without* the program name; it is prepended here so + every call site cannot forget it. Measured 2026-09-11: a call site that passed + `["pr", "view", ...]` to a helper that also omitted the program name ran the + POSIX `pr` utility instead - which took `view` and the PR number as filenames + and failed with `pr: cannot open view`, a message that names neither gh nor + the real 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) + + +@dataclass +class Verdict: + pr: int + title: str + head_sha: str + merge_base: str + ahead_by: int + behind_by: int + run_created_at: str | None + run_conclusion: str | None + stale: bool + reason: str + + +def _latest_run_for_head(head: str) -> dict | None: + """The newest `_VERDICT_WORKFLOW` run for this exact commit, or None. + + Asked by `head_sha`, not by branch + a window: the run set wanted is directly + addressable, and the branch form has two failure modes with one cause. It + carried `--limit 30`, so a branch pushed more than 30 times would report + "no CI run" for a head that has one - fail-loud, but with the wrong reason + (measured 2026-09-11 by pm25coder on #1138, who also confirmed the SHA form + returns the identical answer). It also assumed the run is reachable under the + head *branch* name, which a fork PR or a renamed branch breaks. + + Filtering to `_VERDICT_WORKFLOW` is the second half of the same point: a + passing run from *any* workflow is not a test verdict. + """ + payload = _gh_json( + [ + "api", + f"repos/{REPO}/actions/runs?head_sha={head}&per_page=100", + "--jq", + "{runs: [.workflow_runs[] | {headSha: .head_sha, name, " + "createdAt: .created_at, conclusion}]}", + ] + ) + assert isinstance(payload, dict) + runs_raw = payload.get("runs") + assert isinstance(runs_raw, list) + matching = [ + r + for r in runs_raw + if isinstance(r, dict) + and r.get("headSha") == head + and r.get("name") == _VERDICT_WORKFLOW + ] + if not matching: + return None + return max(matching, key=lambda r: str(r.get("createdAt") or "")) + + +def check_pr(number: int) -> Verdict: + view = _gh_json( + [ + "pr", + "view", + str(number), + "-R", + REPO, + "--json", + "number,title,headRefOid", + ] + ) + assert isinstance(view, dict) + head_sha = str(view["headRefOid"]) + + cmp_raw = _gh_json( + [ + "api", + f"repos/{REPO}/compare/master...{head_sha}", + "--jq", + "{status, ahead_by, behind_by, merge_base: .merge_base_commit.sha}", + ] + ) + assert isinstance(cmp_raw, dict) + status = str(cmp_raw["status"]) + ahead_by = int(cmp_raw["ahead_by"]) + behind_by = int(cmp_raw["behind_by"]) + merge_base = str(cmp_raw["merge_base"]) + + run = _latest_run_for_head(head_sha) + created = str(run.get("createdAt") or "") if run else None + conclusion = str(run.get("conclusion") or "") if run else None + + common = dict( + pr=number, + title=str(view["title"]), + head_sha=head_sha, + merge_base=merge_base, + ahead_by=ahead_by, + behind_by=behind_by, + run_created_at=created, + run_conclusion=conclusion, + ) + + if status not in _FRESH_STATUSES and status not in _STALE_STATUSES: + raise RuntimeError( + f"unrecognised compare status {status!r} for #{number}; refusing to call it fresh" + ) + + if status in _STALE_STATUSES: + return Verdict( + **common, + stale=True, + reason=( + f"head does not contain master (status={status}, behind_by={behind_by}) " + f"- CI's merge base was {merge_base[:8]}, so the verdict is about a tree " + "that can no longer be merged" + ), + ) + + # Master is an ancestor. That answers "would a verdict transfer"; now answer + # "is there one". + if run is None: + return Verdict( + **common, + stale=True, + reason=( + f"master is an ancestor (status={status}) but there is NO {_VERDICT_WORKFLOW} " + f"run for head {head_sha[:8]} - an unjudged head, which `gh pr checks` reports " + "as 'no checks reported'" + ), + ) + if conclusion in _UNFINISHED: + return Verdict( + **common, + stale=True, + reason=f"CI is still {conclusion or 'pending'} on head {head_sha[:8]} - no verdict yet", + ) + if conclusion != "success": + return Verdict( + **common, + stale=True, + reason=( + f"CI concluded {conclusion!r} on head {head_sha[:8]} - a failing verdict, " + "not a stale one; re-running will not make it fresh" + ), + ) + return Verdict( + **common, + stale=False, + reason=( + f"master is an ancestor (status={status}, behind_by={behind_by}) and head " + f"{head_sha[:8]} has a passing run - merge base {merge_base[:8]} IS master's tip" + ), + ) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + prog="check-merge-freshness.py", + description="Is each PR's green CI still about the tree that would merge?", + ) + parser.add_argument("prs", nargs="+", type=int, help="pull request number(s)") + parser.add_argument("--json", action="store_true", help="emit JSON instead of prose") + args = parser.parse_args(argv) + + try: + verdicts = [check_pr(n) 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, + "merge_base": v.merge_base, + "ahead_by": v.ahead_by, + "behind_by": v.behind_by, + "stale": v.stale, + "reason": v.reason, + } + for v in verdicts + ], + indent=2, + ) + ) + else: + for v in verdicts: + mark = "STALE" if v.stale else "FRESH" + print(f"#{v.pr} {mark} (head {v.head_sha[:8]}, base {v.merge_base[:8]}) - {v.reason}") + + if any(v.stale for v in verdicts): + print( + "\nRe-merge master into each stale branch, then let CI run. On `pull_request` " + "GitHub builds Merge into ; merging current master in is what " + "moves the merge base to master, so CI then judges the real merged tree.", + file=sys.stderr, + ) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_check_merge_freshness.py b/tests/test_check_merge_freshness.py new file mode 100644 index 00000000..099f2132 --- /dev/null +++ b/tests/test_check_merge_freshness.py @@ -0,0 +1,336 @@ +"""Tests for scripts/check-merge-freshness.py - is a PR's CI verdict still current? + +Background (cycle cyc20260911-083721) +------------------------------------- +On 2026-09-11 PR #1137 was `MERGEABLE/CLEAN` with both CI jobs green, and the +merge still produced a tree that failed two guards. The cause is worth pinning in +tests because the failure is silent and the obvious signals are all green: + +* GitHub builds `Merge into ` for `pull_request`, so the verdict + is about the head merged onto the *branch point*. When master moves, the branch + point is no longer master, and the green run describes a tree that cannot be + merged - yet it stays green, because master moving is not a branch push and + fires no `synchronize` event. +* `gh pr view --json mergeable` says `CLEAN` throughout, because it answers "does + this textually merge", which is the property that failed. + +Both states are pinned here, never inferred from the failure case alone (#455): + +* **fresh** - the head contains master's tip and has a passing run for that exact + SHA; rc 0, and the tool is safe to gate a merge on. +* **stale** - four distinguishable ways (diverged, no run at all, still running, + concluded non-success), each with its own reason, so a caller can tell "rebase + it" apart from "fix it". + +The fourth case is the one a naive implementation gets wrong: a head that contains +master but has *zero* CI runs is `no checks reported`, not a pass. Ancestry alone +is not enough to call a verdict current, so it is tested as its own state. + +Nothing here touches the network: `_gh_json` is replaced, and the replacement is +asserted to receive the arguments the real helper would (so a test cannot pass by +never calling it). +""" + +from __future__ import annotations + +import importlib.util +import json +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +SCRIPT = REPO_ROOT / "scripts" / "check-merge-freshness.py" + +HEAD = "a" * 40 +OTHER = "b" * 40 +BASE = "c" * 40 + + +def _load_module(): + spec = importlib.util.spec_from_file_location("check_merge_freshness", SCRIPT) + mod = importlib.util.module_from_spec(spec) + # Register before exec: the module declares a dataclass, and dataclasses + # resolves annotations through sys.modules[cls.__module__] at class-creation + # time. A module that is not registered there raises AttributeError inside + # dataclasses itself - an error that names neither this test nor the cause. + import sys + + sys.modules[spec.name] = mod + spec.loader.exec_module(mod) + return mod + + +@pytest.fixture +def mod(): + return _load_module() + + +class FakeGh: + """A stand-in for `_gh_json` that answers from a routing table. + + Records every call so a test can assert the query was actually made - a test + whose fake is never called would pass while the code under test queried + nothing at all. + """ + + def __init__(self, pr_view: dict, compare: dict, runs: list[dict] | None): + self.pr_view = pr_view + self.compare = compare + self.runs = runs + self.calls: list[list[str]] = [] + + def __call__(self, args: list[str]) -> object: + self.calls.append(list(args)) + # Every gh invocation must name its program at the call site (the helper + # prepends it); assert the shape rather than trusting it. + assert args, "gh was called with no arguments" + if args[:2] == ["pr", "view"]: + return self.pr_view + if args[0] == "api": + if any("actions/runs" in a for a in args): + return {"runs": self.runs if self.runs is not None else []} + assert any(a.startswith("repos/") and "/compare/" in a for a in args), args + return self.compare + raise AssertionError(f"unexpected gh call: {args}") + + +def _compare(status: str, ahead: int, behind: int, base: str = BASE) -> dict: + return {"status": status, "ahead_by": ahead, "behind_by": behind, "merge_base": base} + + +def _view(sha: str = HEAD, branch: str = "feature/x") -> dict: + return {"number": 1, "title": "t", "headRefOid": sha, "headRefName": branch} + + +def _run_(sha: str = HEAD, conclusion: str = "success", at: str = "2026-09-11T00:00:00Z") -> dict: + """A workflow run, as the actions/runs payload reports it. + + `name` is not decoration: the tool pins the verdict to the `Test` workflow, so + a fixture without it is a run the tool is right to ignore. + """ + return {"headSha": sha, "name": "Test", "conclusion": conclusion, "createdAt": at} + + +def _install(mod, monkeypatch, fake: FakeGh) -> None: + monkeypatch.setattr(mod, "_gh_json", fake) + + +def _run(mod, monkeypatch, fake: FakeGh, argv: list[str] | None = None) -> int: + _install(mod, monkeypatch, fake) + return mod.main(argv if argv is not None else ["1"]) + + +# --- fresh ----------------------------------------------------------------- + + +def test_fresh_when_head_contains_master_and_has_a_passing_run(mod, monkeypatch, capsys): + fake = FakeGh(_view(), _compare("ahead", 4, 0), [_run_()]) + rc = _run(mod, monkeypatch, fake) + out = capsys.readouterr().out + assert rc == 0 + assert "FRESH" in out + assert "STALE" not in out + assert len(fake.calls) == 3, "expected exactly the three queries, not a short-circuit" + + +def test_fresh_when_master_has_not_moved_at_all(mod, monkeypatch, capsys): + """`identical` means master's tip *is* the head - trivially current.""" + fake = FakeGh(_view(), _compare("identical", 0, 0), [_run_()]) + assert _run(mod, monkeypatch, fake) == 0 + assert "FRESH" in capsys.readouterr().out + + +# --- stale: the four distinguishable ways ---------------------------------- + + +def test_stale_when_the_head_does_not_contain_master(mod, monkeypatch, capsys): + """The #1137 case: the verdict was green but about an older master.""" + fake = FakeGh( + _view(), _compare("diverged", 2, 1, base="cb651a4"), [_run_()] + ) + rc = _run(mod, monkeypatch, fake) + out = capsys.readouterr().out + assert rc == 1 + assert "STALE" in out + assert "no longer be merged" in out + # The reason must name the branch point, or a reader cannot tell which master + # the verdict was actually about. + assert "cb651a4" in out + + +def test_stale_when_there_is_no_ci_run_for_this_head(mod, monkeypatch, capsys): + """Ancestry is not enough: `no checks reported` is not a pass. + + This is the state a dropped push event leaves behind, and it is the one an + ancestry-only implementation would wrongly call fresh. + """ + fake = FakeGh(_view(), _compare("ahead", 1, 0), []) + rc = _run(mod, monkeypatch, fake) + out = capsys.readouterr() + assert rc == 1 + assert "NO Test run" in out.out + assert "no checks reported" in out.out + + +def test_stale_when_a_run_exists_only_for_a_different_sha(mod, monkeypatch, capsys): + """Keyed on the SHA, not the branch. + + A branch pushed twice has runs for both heads; reading the older one as the + current verdict is the same mistake one step smaller. + """ + fake = FakeGh(_view(), _compare("ahead", 1, 0), [_run_(OTHER)]) + rc = _run(mod, monkeypatch, fake) + assert rc == 1 + assert "NO Test run" in capsys.readouterr().out + + +def test_a_passing_run_from_another_workflow_is_not_the_verdict(mod, monkeypatch, capsys): + """The claim is "the *tests* passed", so the workflow is part of the query. + + Today `test.yml` is the only workflow `pull_request` triggers, so accepting + any passing run happens to give the right answer - which is exactly the kind + of coincidence that stops being true silently, the first time a second + workflow is added to a branch. Pinned on a fixture whose only difference from + the fresh case is the workflow name. + """ + other_workflow = _run_() + other_workflow["name"] = "Build Release" + fake = FakeGh(_view(), _compare("ahead", 4, 0), [other_workflow]) + rc = _run(mod, monkeypatch, fake) + out = capsys.readouterr().out + assert rc == 1 + assert "NO Test run" in out, out + + +def test_stale_and_distinguished_when_ci_is_still_running(mod, monkeypatch, capsys): + fake = FakeGh(_view(), _compare("ahead", 1, 0), [_run_(conclusion="pending")]) + rc = _run(mod, monkeypatch, fake) + out = capsys.readouterr().out + assert rc == 1 + assert "still pending" in out + + +def test_a_failing_verdict_is_reported_as_failing_not_as_stale(mod, monkeypatch, capsys): + """Re-running CI will not help, so the wording must not suggest a rebase does.""" + fake = FakeGh(_view(), _compare("ahead", 1, 0), [_run_(conclusion="failure")]) + rc = _run(mod, monkeypatch, fake) + out = capsys.readouterr().out + assert rc == 1 + assert "failing verdict, not a stale one" in out + + +def test_the_newest_run_for_the_head_wins(mod, monkeypatch, capsys): + """Two runs on one SHA: the freshest conclusion decides, not the first seen.""" + fake = FakeGh( + _view(), + _compare("ahead", 1, 0), + [ + _run_(conclusion="failure", at="2026-09-11T00:00:00Z"), + _run_(at="2026-09-11T01:00:00Z"), + ], + ) + rc = _run(mod, monkeypatch, fake) + assert rc == 0 + assert "FRESH" in capsys.readouterr().out + + +# --- fail loud, never guess ------------------------------------------------ + + +def test_an_unrecognised_compare_status_is_refused_not_called_fresh(mod, monkeypatch, capsys): + """A new GitHub status must not silently read as fresh. + + The freshness sets are named rather than written as `status == "ahead"` for + exactly this case: anything unrecognised has to fail loud. + """ + fake = FakeGh(_view(), _compare("some_new_status", 1, 0), [_run_()]) + rc = _run(mod, monkeypatch, fake) + err = capsys.readouterr().err + assert rc == 2 + assert "unrecognised compare status" in err + + +def test_a_gh_failure_exits_2_with_the_reason(mod, monkeypatch, capsys): + def boom(args): + raise RuntimeError("gh failed (rc=1): gh api repos/x/compare/...\nsome stderr") + + monkeypatch.setattr(mod, "_gh_json", boom) + rc = mod.main(["1"]) + err = capsys.readouterr().err + assert rc == 2 + assert "gh failed" in err + + +def test_the_helper_invokes_the_gh_program_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"] + + +def test_json_mode_is_machine_readable(mod, monkeypatch, capsys): + fake = FakeGh(_view(), _compare("ahead", 4, 0), [_run_()]) + rc = _run(mod, monkeypatch, fake, ["1", "--json"]) + payload = json.loads(capsys.readouterr().out) + assert rc == 0 + assert payload[0]["stale"] is False + assert payload[0]["head"] == HEAD + assert payload[0]["merge_base"] == BASE + + +def test_no_function_has_an_unused_parameter() -> None: + """Every declared parameter must be read somewhere in its own body. + + Added because this script shipped one: `_latest_run_for_head(head, branch, + number)` never read `number`, left over from a draft that used it to build the + error message. A dead parameter is not cosmetic here - it tells the next reader + the function needs the PR number to do its job, which is exactly the kind of + false signal about a *merge-gate* helper that this repo treats as a defect. + + Parsed with `ast` rather than a regex, and checked against the whole function + body including nested scopes, so a parameter read only inside a closure still + counts as used. `self` is excluded (methods), as are names prefixed with `_`. + """ + import ast + + tree = ast.parse(SCRIPT.read_text(encoding="utf-8")) + offenders: list[str] = [] + for node in ast.walk(tree): + if not isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + continue + args = node.args + declared = [a.arg for a in (*args.posonlyargs, *args.args, *args.kwonlyargs)] + if args.vararg: + declared.append(args.vararg.arg) + if args.kwarg: + declared.append(args.kwarg.arg) + named = set(declared) - {"self"} + used = {n.id for n in ast.walk(node) if isinstance(n, ast.Name)} + used |= {n.attr for n in ast.walk(node) if isinstance(n, ast.Attribute)} + for param in sorted(named): + if param not in used: + offenders.append(f"{node.name}({param})") + assert not offenders, ( + "unused parameter(s) - each declares a dependency the body does not have: " + f"{offenders}" + )