Skip to content

emrg: check-pr-base names the repository it measures #2630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
argszero merged 1 commit into from
Oct 29, 2026
Merged

emrg: check-pr-base names the repository it measures #2630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 121 additions & 20 deletions scripts/check-pr-base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,28 @@
0 no open PR's base is a dead end (bases either reach master, or are an open
PR's head - the latter reported as LIVE)
1 at least one open PR is based on a dead end - retarget it before merging
2 the question could not be answered (gh failed, response unparseable, or a
requested PR number is not among the open PRs)
2 the question could not be answered (gh failed, response unparseable, a
requested PR number is not among the open PRs, or no repository could be named
to measure - see "Which repository answered" below)

Which repository answered
-------------------------
Every verdict here is about exactly one repository, and the first line of every report
names it. `master` and the branch list are that repository's, so a reader who cannot see
which one was read cannot tell whose bases were classified.

`--repo` names it. Without `--repo` the repository used to be left to gh's own
`{owner}/{repo}` placeholder, which gh resolves from the remotes of the directory this
runs in - correct on an ordinary clone, and wrong on a checkout whose only GitHub remote
is a fork. A fork holds a **copy** of `master` and the branches it had when it was forked,
so the gate answered about the fork and said nothing about it. Measured 2026-10-29
(`cyc20261029-082021`) on master `7e76668`: the placeholder resolved to `pm25coder/emrg`
with 50 branches against the upstream's 1159, so 1158 upstream bases were reported as "no
longer exists on the remote"; and the upstream master head `7e766683` - the commit this
very file is committed in - read as *not on master*, flipping `_ref_is_on_master` from
True to False. Same commit, same question, two answers, and no line of output naming the
repository that gave either. So the placeholder never reaches gh as a path now: a fork is
refused by name, and any other resolution is reported before the verdicts.

What the count covers
---------------------
Expand All @@ -86,6 +106,12 @@

MASTER = "master"

#: gh's placeholder for "the repository of the directory this runs in". Spelled once,
#: because it is the one token that must never again reach a `repos/...` path: gh resolves
#: it from the cwd's remotes, and on a checkout whose only GitHub remote is a fork that
#: resolution *is* the fork. `_repo_for_the_question` is where it is resolved and refused.
PLACEHOLDER = "{owner}/{repo}"


def _gh(*args: str) -> str:
"""Run `gh`, failing loud rather than guessing.
Expand Down Expand Up @@ -123,8 +149,15 @@ def _gh_json(*args: str) -> object:
return json.loads(_gh(*args))


def _open_prs(repo: str | None) -> list[dict]:
"""Every open PR with the fields this check needs."""
def _open_prs(repo: str) -> list[dict]:
"""Every open PR **in `repo`**, with the fields this check needs.

`repo` is always a concrete `owner/name` - the caller named it, or
`_repo_for_the_question` resolved it. The `if repo:` that used to be here was the
second half of the old defect: omitting `-R` is what made gh answer about whatever
repository the checkout resolved to, and leaving the omissible branch in place would
keep that answer one forgotten argument away.
"""
args = [
"pr",
"list",
Expand All @@ -134,9 +167,9 @@ def _open_prs(repo: str | None) -> list[dict]:
"100",
"--json",
"number,baseRefName,headRefName,state",
"-R",
repo,
]
if repo:
args += ["-R", repo]
payload = _gh_json(*args)
if not isinstance(payload, list):
raise RuntimeError(f"expected a list of PRs, got {type(payload).__name__}")
Expand All @@ -147,16 +180,63 @@ def _open_prs(repo: str | None) -> list[dict]:
return payload


def _repo_path(repo: str | None) -> str:
"""`owner/name` - the caller's, or gh's placeholder for the current repo."""
return repo if repo else "{owner}/{repo}"
class Unmeasurable(RuntimeError):
"""The question cannot be answered, and refusing is the answer.

The same exit path as any other `RuntimeError` here (rc 2), and named apart so a
reader of the refusal can see which of the two it was: this one means the repository
to measure could not be named, not that gh failed.
"""


def _repo_for_the_question(repo: str | None) -> str:
"""The concrete `owner/name` this run reads - resolved, never a placeholder.

`--repo` is the caller naming a repository, which is not a guess. Without it, the
repository used to be gh's placeholder and gh resolved it from the remotes of the
checkout - on which a fork is the answer, and a fork is the wrong repository for this
question for two measured reasons: its `master` is a copy that lags, and its branch
list is a snapshot taken at fork time. Measured 2026-10-29 (`cyc20261029-082021`) on
master `7e76668`: the placeholder resolved to `pm25coder/emrg` (50 branches against
the upstream's 1159), and the upstream master head `7e766683` itself read as *not on
master* - the same commit answering False here and True under `--repo argszero/emrg`.

def _branch_heads(repo: str | None) -> dict[str, str]:
"""branch name -> head sha, for every branch on the remote."""
So a fork is refused **by name**, together with the repository it forks, and the
remedy is spelled out; a non-fork resolution is returned so `main` can print it before
any verdict. The non-fork half is load-bearing: an ordinary clone, whose remote is the
repository itself, must keep working without new flags.
"""
if repo:
return repo
meta = _gh_json("api", f"repos/{PLACEHOLDER}")
if not isinstance(meta, dict):
raise Unmeasurable(
f"gh answered {type(meta).__name__} for {PLACEHOLDER}, not a repository - "
f"there is no repository to measure"
)
resolved = meta.get("full_name")
if not resolved:
raise Unmeasurable(
f"gh did not name the repository it resolved from {PLACEHOLDER}: {meta!r}"
)
parent = (meta.get("parent") or {}).get("full_name")
if parent:
raise Unmeasurable(
f"refusing to measure: with no --repo, gh resolved this checkout to "
f"{resolved}, which is a fork of {parent}. The base a PR declares and the "
f"master it has to reach both belong to one repository, and a fork holds a "
f"copy of master and the branches it had when it was forked, so a verdict "
f"read here is a verdict about {resolved}, not about {parent}. Name the "
f"repository to measure: --repo {parent}"
)
return resolved


def _branch_heads(repo: str) -> dict[str, str]:
"""branch name -> head sha, for every branch of `repo`."""
# `_gh` (not a bare subprocess.run) so a missing `gh` raises the same
# RuntimeError as a failing one and takes the rc-2 path.
out = _gh("api", f"repos/{_repo_path(repo)}/branches", "--paginate")
out = _gh("api", f"repos/{repo}/branches", "--paginate")
# --paginate concatenates page arrays, which is not valid JSON as a whole;
# the endpoint returns an array per page, so parse per document if it parses,
# else fall back to line-delimited objects.
Expand All @@ -175,9 +255,9 @@ def _branch_heads(repo: str | None) -> dict[str, str]:
return heads


def _ref_is_on_master(sha: str, repo: str | None) -> bool:
"""True when `sha` is an ancestor of (or equal to) master's head."""
data = _gh_json("api", f"repos/{_repo_path(repo)}/compare/{MASTER}...{sha}")
def _ref_is_on_master(sha: str, repo: str) -> bool:
"""True when `sha` is an ancestor of (or equal to) `repo`'s master head."""
data = _gh_json("api", f"repos/{repo}/compare/{MASTER}...{sha}")
status = data.get("status")
if status is None:
raise RuntimeError(f"compare response has no status: {data}")
Expand All @@ -187,7 +267,7 @@ def _ref_is_on_master(sha: str, repo: str | None) -> bool:


def classify_base(
base: str, branches: dict[str, str], repo: str | None, open_heads: set[str] = frozenset()
base: str, branches: dict[str, str], repo: str, open_heads: set[str] = frozenset()
) -> tuple[str, str]:
"""`(verdict, why)` for one PR's base branch.

Expand Down Expand Up @@ -228,15 +308,36 @@ def main(argv: list[str] | None = None) -> int:
parser = argparse.ArgumentParser(
description="Report PRs whose base branch cannot reach master."
)
parser.add_argument("--repo", help="owner/name (default: the current repo)")
parser.add_argument(
"--repo",
help=(
"owner/name to measure (default: the repository gh resolves for this "
"checkout, refused when that repository is a fork)"
),
)
parser.add_argument(
"prs", nargs="*", type=int, help="only these PR numbers (default: all open)"
)
args = parser.parse_args(argv)

# Resolved before anything is measured: a refusal that arrived after a verdict would
# still be a verdict, and the shape this file used to print was a verdict about an
# unnamed repository.
try:
repo = _repo_for_the_question(args.repo)
except (RuntimeError, json.JSONDecodeError) as exc:
print(f"cannot determine PR bases: {exc}", file=sys.stderr)
return 2

# Named before any verdict, on every run: `master` and the branch list are this
# repository's, so the reader has to be able to see whose they were. The line carries
# no other word on purpose - the tests below assert what the *verdict* lines say, and
# this one must not fall inside those assertions.
print(f"repo: {repo}")

try:
prs = _open_prs(args.repo)
branches = _branch_heads(args.repo)
prs = _open_prs(repo)
branches = _branch_heads(repo)
except (RuntimeError, json.JSONDecodeError) as exc:
print(f"cannot determine PR bases: {exc}", file=sys.stderr)
return 2
Expand Down Expand Up @@ -274,7 +375,7 @@ def main(argv: list[str] | None = None) -> int:
for pr in sorted(prs, key=lambda p: p["number"]):
try:
verdict, why = classify_base(
pr["baseRefName"], branches, args.repo, open_heads
pr["baseRefName"], branches, repo, open_heads
)
except (RuntimeError, json.JSONDecodeError) as exc:
print(f"#{pr['number']} UNKNOWN base={pr['baseRefName']!r}: {exc}")
Expand Down
173 changes: 173 additions & 0 deletions tests/test_check_pr_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,3 +434,176 @@ def test_the_two_halves_of_the_sentence_are_different_numbers(

assert rc == 1, out
assert "2 of the 3 named PR(s)" in out, out


class TestTheRepositoryItReadIsNamed:
"""A verdict here is about exactly one repository, and this gate has to say which.

Measured 2026-10-29 (`cyc20261029-082021`) on master `7e76668`, in a checkout whose
`origin` is a local directory and whose only GitHub remote is `fork = pm25coder/emrg`.
With no `--repo`, gh resolved the tool's `{owner}/{repo}` placeholder to the fork, and
the same commit asked the same question answered twice:

_ref_is_on_master("7e766683...", None) -> False
_ref_is_on_master("7e766683...", "argszero/emrg") -> True

because `master` was read in two repositories. The branch sets the verdicts are
classified against differ the same way - 50 against 1159 - so 1158 upstream bases came
back as "no longer exists on the remote", and the upstream master head itself read as
not on master. No line of that output named the repository that had answered, and with
an empty queue the exit code was a clean 0.

Three directions are pinned, and each rules out one bad fix: a fork is refused **and
named** (so restoring the silent fallback reds), an ordinary checkout is still measured
**and named** (so "refuse whenever `--repo` is absent" cannot pass), and `--repo`
performs no resolution read at all (so a fix that keeps asking gh and only prints the
answer afterwards is not this fix).
"""

FORK = {
"full_name": "pm25coder/emrg",
"fork": True,
"parent": {"full_name": "argszero/emrg"},
}
ORDINARY = {"full_name": "someone/emrg", "fork": False, "parent": None}

@staticmethod
def _dead_pr() -> list[dict]:
return [{"number": 1148, "baseRefName": "feature/dead", "headRefName": "h1"}]

def _wire_resolution(self, mod, monkeypatch, resolution, calls=None):
def fake(*args):
if calls is not None:
calls.append(args)
return resolution

monkeypatch.setattr(mod, "_gh_json", fake)

def _wire_measurement(self, mod, monkeypatch, seen, prs=None):
def fake_open(repo):
seen.append(repo)
return self._dead_pr() if prs is None else prs

monkeypatch.setattr(mod, "_open_prs", fake_open)
monkeypatch.setattr(mod, "_branch_heads", lambda repo: {"feature/dead": "s1"})
monkeypatch.setattr(mod, "_ref_is_on_master", lambda sha, repo: False)

def test_a_fork_is_refused_by_name(self, mod, monkeypatch, capsys) -> None:
"""The measured shape: the checkout resolves to a fork, so nothing is measured."""
seen: list[str] = []
self._wire_resolution(mod, monkeypatch, self.FORK)
self._wire_measurement(mod, monkeypatch, seen)

rc = mod.main([])
err = capsys.readouterr().err

assert rc == 2, "a fork is not a repository this question can be asked of"
assert seen == [], "the repository must be settled before anything is measured"
assert "pm25coder/emrg" in err, err
assert "argszero/emrg" in err, "the refusal must name the repository it is a fork of"
assert "--repo argszero/emrg" in err, "and it must name the remedy"

def test_an_ordinary_checkout_is_measured_and_named(
self, mod, monkeypatch, capsys
) -> None:
"""The control: a non-fork resolution keeps the tool's ordinary invocation alive.

Without this, a fix that refused whenever `--repo` is absent would pass the test
above while taking away the way every normal clone runs this gate.
"""
seen: list[str] = []
self._wire_resolution(mod, monkeypatch, self.ORDINARY)
self._wire_measurement(mod, monkeypatch, seen)

rc = mod.main([])
out = capsys.readouterr().out

assert rc == 1, out
assert seen == ["someone/emrg"], "the measurement must use the resolved repository"
assert out.splitlines()[0] == "repo: someone/emrg", out

def test_a_named_repo_is_printed_before_any_verdict(
self, mod, monkeypatch, capsys
) -> None:
"""`--repo` is the caller naming the repository, so gh is never asked to guess."""
seen: list[str] = []
calls: list[tuple] = []
self._wire_resolution(mod, monkeypatch, self.FORK, calls)
self._wire_measurement(mod, monkeypatch, seen)

rc = mod.main(["--repo", "owner/repo"])
out = capsys.readouterr().out

assert rc == 1, out
assert calls == [], "--repo means no resolution read at all"
assert seen == ["owner/repo"], seen
assert out.splitlines()[0] == "repo: owner/repo", out

def test_the_placeholder_is_a_lookup_and_never_a_path(
self, mod, monkeypatch, capsys
) -> None:
"""The defect itself, read off every argument the tool hands `gh`.

The token `{owner}/{repo}` is what gh resolves from the cwd's remotes, so the fix
is not "print the repository afterwards" but "never let gh pick": the token may
appear exactly once, as the lookup this module performs itself, and never inside a
`repos/...` path a verdict then rests on.
"""
calls: list[tuple] = []

def fake_gh_json(*args):
calls.append(args)
if " ".join(args) == f"api repos/{mod.PLACEHOLDER}":
return self.ORDINARY
return {"status": "ahead"} # not on master: the DEAD verdict

monkeypatch.setattr(mod, "_gh_json", fake_gh_json)
monkeypatch.setattr(
mod,
"_gh",
lambda *args: json.dumps([{"name": "feature/dead", "commit": {"sha": "s1"}}]),
)
monkeypatch.setattr(mod, "_open_prs", lambda repo: self._dead_pr())

rc = mod.main([])
assert rc == 1, capsys.readouterr()

with_token = [a for a in calls if any(mod.PLACEHOLDER in part for part in a)]
assert with_token == [("api", f"repos/{mod.PLACEHOLDER}")], with_token
assert any(
"repos/someone/emrg/compare/master..." in part for a in calls for part in a
), calls

def test_a_resolution_that_names_no_repository_is_unmeasurable(
self, mod, monkeypatch, capsys
) -> None:
"""A resolution that answers nothing is not a repository to measure."""
seen: list[str] = []
self._wire_resolution(mod, monkeypatch, {"full_name": ""})
self._wire_measurement(mod, monkeypatch, seen)

rc = mod.main([])
err = capsys.readouterr().err

assert rc == 2, err
assert seen == [], seen
assert "did not name the repository" in err, err

def test_an_empty_queue_still_names_the_repository(
self, mod, monkeypatch, capsys
) -> None:
"""Named even when there is nothing to classify.

The repository line is not a header of the verdict table - it is the answer to
"whose queue was read", and an empty queue is exactly the case where a caller has
nothing else to go on. What the queue being empty *means* (this gate answers 0
where its siblings answer 2) is a separate question and not pinned here.
"""
seen: list[str] = []
self._wire_resolution(mod, monkeypatch, self.ORDINARY)
self._wire_measurement(mod, monkeypatch, seen, prs=[])

mod.main([])
out = capsys.readouterr().out

assert out.splitlines()[0] == "repo: someone/emrg", out
Loading