Skip to content

Commit 820bab3

Browse files
wolfgang-auraclaude
andcommitted
feat: read a filed pull request's issue for a competing pull request
`mailman contributions --refresh` read only our own pull request's state. python/mypy#21967 was opened against the issue python/mypy#21961 fixed and the ledger said nothing for a day; it was found by hand in the PR thread. While a pull request is open, the refresh now reads its issue's timeline, where GitHub records every cross-reference, and keeps the other pull requests from the same repository that are open or merged. The listing names each one, says "no competing pull request" when the read was clean, and says why when it could not be read, so an unchecked row never reads like an unchallenged one. The command exits non-zero while a competitor exists. The parser is tested against the recorded timeline of python/mypy#21960, which holds ours, the competitor and a cross-repository reference to filter out. Closes #86. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 893d35d commit 820bab3

4 files changed

Lines changed: 549 additions & 2 deletions

File tree

mailman/cli.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
from mailman.prompts import load_recorded_verification, write_task_prompts
7272
from mailman.provenance import (
7373
collect_contributions,
74+
competitors,
7475
deletion_is_safe,
7576
record_provenance,
7677
refresh_contributions,
@@ -2092,7 +2093,31 @@ def _contributions(arguments: argparse.Namespace) -> int:
20922093
"the states above are the ones already on disk, not fresh readings",
20932094
file=sys.stderr,
20942095
)
2095-
return 1 if (unrecorded or failures) else 0
2096+
challenged = [entry for entry in found if competitors(entry)]
2097+
if challenged:
2098+
# Someone else's pull request on the same issue changes what to do
2099+
# with ours, and the maintainer's review time is spent either way.
2100+
# https://github.com/wolfgang-aura/Mailman/issues/86
2101+
print(
2102+
"\n".join(
2103+
[
2104+
"",
2105+
f"{len(challenged)} open pull request(s) have a competing pull "
2106+
"request on the same issue. Read it and decide whether ours "
2107+
"still stands:",
2108+
*(
2109+
f" {entry.repository}#{entry.pull_request}: "
2110+
+ ", ".join(
2111+
f"#{item.get('number')} ({item.get('state')})"
2112+
for item in competitors(entry)
2113+
)
2114+
for entry in challenged
2115+
),
2116+
]
2117+
),
2118+
file=sys.stderr,
2119+
)
2120+
return 1 if (unrecorded or failures or challenged) else 0
20962121

20972122

20982123
def _identity(arguments: argparse.Namespace) -> int:

mailman/provenance.py

Lines changed: 214 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ class Contribution:
5555
superseded_by: int | None = None
5656
patch_path: str | None = None
5757
checked_at: str | None = None
58+
competition: dict[str, Any] | None = None
5859

5960
def permalinks(self) -> tuple[str, ...]:
6061
return tuple(
@@ -73,6 +74,7 @@ def to_dict(self) -> dict[str, Any]:
7374
"superseded_by": self.superseded_by,
7475
"patch_path": self.patch_path,
7576
"checked_at": self.checked_at,
77+
"competition": self.competition,
7678
}
7779

7880

@@ -208,6 +210,123 @@ def pull_request_state(repository: str, number: int) -> dict[str, Any]:
208210
}
209211

210212

213+
_ISSUE_URL = re.compile(
214+
r"^https://github\.com/([A-Za-z0-9._-]+/[A-Za-z0-9._-]+)/issues/(\d+)/?$"
215+
)
216+
217+
218+
def upstream_issue_number(run_directory: Path, repository: str) -> int | None:
219+
"""The issue this run set out to fix, if the run names one in `repository`.
220+
221+
`run.json` carries the issue URL from the start of the run; the staged
222+
`submission.json` carries the number and target again. Either will do, and
223+
an issue in some other repository says nothing about this one.
224+
"""
225+
slug = repository_slug(repository).lower()
226+
run_record = _read_json(run_directory / "run.json")
227+
match = _ISSUE_URL.match(str(run_record.get("issue") or "").strip())
228+
if match is not None and match.group(1).lower() == slug:
229+
return int(match.group(2))
230+
submission = _read_json(run_directory / SUBMISSION_DIRECTORY / "submission.json")
231+
number = submission.get("issue_number")
232+
target = str(submission.get("target") or "").lower()
233+
if isinstance(number, int) and number > 0 and target == slug:
234+
return number
235+
return None
236+
237+
238+
def _read_json(path: Path) -> dict[str, Any]:
239+
try:
240+
payload = json.loads(path.read_text(encoding="utf-8"))
241+
except (OSError, ValueError):
242+
return {}
243+
return payload if isinstance(payload, dict) else {}
244+
245+
246+
def competing_pull_requests(
247+
repository: str, issue_number: int, *, own_number: int
248+
) -> dict[str, Any]:
249+
"""Every other pull request in `repository` that references the issue.
250+
251+
Read from the issue's timeline, where GitHub records each cross-reference
252+
as it is made. That catches `Fixes #N` in a body, a mention in a comment and
253+
a manual link, which a text search for `#N` does not. Pull requests from
254+
other repositories reference issues too and are left out; so are ones that
255+
closed without merging, which stopped competing.
256+
See https://github.com/wolfgang-aura/Mailman/issues/86.
257+
"""
258+
slug = repository_slug(repository)
259+
if shutil.which("gh") is None:
260+
return {"available": False, "detail": "gh is not installed"}
261+
try:
262+
completed = subprocess.run(
263+
[
264+
"gh",
265+
"api",
266+
f"repos/{slug}/issues/{issue_number}/timeline",
267+
"--paginate",
268+
"--slurp",
269+
],
270+
capture_output=True,
271+
text=True,
272+
encoding="utf-8",
273+
errors="replace",
274+
timeout=clamp_timeout_seconds(60),
275+
check=False,
276+
shell=False,
277+
)
278+
except (OSError, subprocess.TimeoutExpired) as error:
279+
return {"available": False, "detail": str(error)}
280+
if completed.returncode != 0:
281+
detail = completed.stderr.strip() or completed.stdout.strip()
282+
return {"available": False, "detail": detail}
283+
try:
284+
pages = json.loads(completed.stdout)
285+
except json.JSONDecodeError as error:
286+
return {"available": False, "detail": f"unreadable response ({error})"}
287+
events: list[Any] = []
288+
for page in pages if isinstance(pages, list) else []:
289+
events.extend(page if isinstance(page, list) else [])
290+
return {
291+
"available": True,
292+
"pull_requests": competitors_from_timeline(events, slug, own_number=own_number),
293+
}
294+
295+
296+
def competitors_from_timeline(
297+
events: list[Any], repository: str, *, own_number: int
298+
) -> list[dict[str, Any]]:
299+
"""The competing pull requests in a GitHub issue timeline, oldest first."""
300+
found: dict[int, dict[str, Any]] = {}
301+
repository_url = f"https://api.github.com/repos/{repository}".lower()
302+
for event in events:
303+
if not isinstance(event, dict) or event.get("event") != "cross-referenced":
304+
continue
305+
source = event.get("source") or {}
306+
issue = source.get("issue") if isinstance(source, dict) else None
307+
if not isinstance(issue, dict) or not issue.get("pull_request"):
308+
continue
309+
if str(issue.get("repository_url") or "").lower() != repository_url:
310+
continue
311+
number = issue.get("number")
312+
if not isinstance(number, int) or number == own_number:
313+
continue
314+
pull_request = issue.get("pull_request")
315+
merged = isinstance(pull_request, dict) and bool(pull_request.get("merged_at"))
316+
state = "merged" if merged else str(issue.get("state") or "").lower()
317+
if state not in {"open", "merged"}:
318+
continue
319+
user = issue.get("user") or {}
320+
found[number] = {
321+
"number": number,
322+
"state": state,
323+
"author": user.get("login") if isinstance(user, dict) else None,
324+
"url": issue.get("html_url"),
325+
"created_at": issue.get("created_at"),
326+
}
327+
return [found[number] for number in sorted(found)]
328+
329+
211330
def submission_directory(run_directory: Path) -> Path:
212331
return run_directory / SUBMISSION_DIRECTORY
213332

@@ -325,6 +444,9 @@ def contribution_from_record(record: dict[str, Any]) -> Contribution:
325444
superseded_by=record.get("superseded_by"),
326445
patch_path=record.get("patch_path"),
327446
checked_at=record.get("checked_at"),
447+
competition=record.get("competition")
448+
if isinstance(record.get("competition"), dict)
449+
else None,
328450
)
329451

330452

@@ -355,6 +477,7 @@ def refresh_state(
355477
run_directory: Path,
356478
*,
357479
state_lookup: Any = pull_request_state,
480+
competitor_lookup: Any = competing_pull_requests,
358481
now: datetime | None = None,
359482
) -> tuple[dict[str, Any] | None, str | None]:
360483
"""Re-read one run's pull request state from GitHub and store the answer.
@@ -363,6 +486,11 @@ def refresh_state(
363486
so it still works after the clone is gone. A lookup that fails leaves every
364487
stored field alone and returns why: a state written on filing day beats one
365488
invented now.
489+
490+
While the pull request is open, the issue it fixes is read too, for a pull
491+
request someone else filed against it. python/mypy#21967 was opened against
492+
the issue python/mypy#21961 fixed and nothing here noticed for a day.
493+
See https://github.com/wolfgang-aura/Mailman/issues/86.
366494
"""
367495
record = load_provenance(run_directory)
368496
if record is None:
@@ -381,15 +509,60 @@ def refresh_state(
381509
record["merge_commit"] = lookup.get("merge_commit")
382510
if lookup.get("url"):
383511
record["url"] = lookup.get("url")
512+
record["competition"] = _read_competition(
513+
run_directory, slug, int(number), record["state"], competitor_lookup, now=now
514+
)
384515
path = provenance_path(run_directory)
385516
path.write_text(json.dumps(record, indent=2) + "\n", encoding="utf-8", newline="\n")
517+
competition = record["competition"]
518+
if competition.get("detail"):
519+
detail = competition["detail"]
520+
return record, f"{slug}#{number}: competitors unchecked: {detail}"
386521
return record, None
387522

388523

524+
def _read_competition(
525+
run_directory: Path,
526+
slug: str,
527+
number: int,
528+
state: str | None,
529+
competitor_lookup: Any,
530+
*,
531+
now: datetime | None,
532+
) -> dict[str, Any]:
533+
"""What was read about other pull requests on the run's issue, or why not.
534+
535+
Only an open pull request can be overtaken, so a closed or merged one is
536+
not looked up; its record says why the list is empty.
537+
"""
538+
checked_at = (now or datetime.now(UTC)).isoformat()
539+
if (state or "").upper() != "OPEN":
540+
return {"checked_at": checked_at, "skipped": f"the pull request is {state}"}
541+
issue_number = upstream_issue_number(run_directory, slug)
542+
if issue_number is None:
543+
return {
544+
"checked_at": checked_at,
545+
"detail": "run.json names no issue in this repository",
546+
}
547+
lookup = competitor_lookup(slug, issue_number, own_number=number)
548+
if not lookup.get("available"):
549+
return {
550+
"checked_at": checked_at,
551+
"issue": issue_number,
552+
"detail": lookup.get("detail") or "gh gave no reason",
553+
}
554+
return {
555+
"checked_at": checked_at,
556+
"issue": issue_number,
557+
"pull_requests": list(lookup.get("pull_requests") or []),
558+
}
559+
560+
389561
def refresh_contributions(
390562
data_root: Path,
391563
*,
392564
state_lookup: Any = pull_request_state,
565+
competitor_lookup: Any = competing_pull_requests,
393566
now: datetime | None = None,
394567
) -> tuple[list[Contribution], list[str]]:
395568
"""Every recorded run, re-read from GitHub, with whatever could not be."""
@@ -398,7 +571,12 @@ def refresh_contributions(
398571
if not data_root.is_dir():
399572
return found, failures
400573
for directory in sorted(path for path in data_root.glob("*") if path.is_dir()):
401-
record, failure = refresh_state(directory, state_lookup=state_lookup, now=now)
574+
record, failure = refresh_state(
575+
directory,
576+
state_lookup=state_lookup,
577+
competitor_lookup=competitor_lookup,
578+
now=now,
579+
)
402580
if record is None:
403581
continue
404582
if failure:
@@ -464,6 +642,7 @@ def render_contributions(
464642
lines.append(f"{entry.run_id} {entry.repository} {pull_request} {state}")
465643
if entry.pull_request:
466644
lines.append(f" {_reading_age(entry, now=now)}")
645+
lines.extend(f" {line}" for line in _competition_lines(entry))
467646
for link in entry.permalinks():
468647
lines.append(f" {link}")
469648
if entry.merge_commit:
@@ -476,6 +655,40 @@ def render_contributions(
476655
return "\n".join(lines)
477656

478657

658+
def competitors(entry: Contribution) -> list[dict[str, Any]]:
659+
"""The other pull requests read against this run's issue, if any were read."""
660+
competition = entry.competition or {}
661+
found = competition.get("pull_requests")
662+
if not isinstance(found, list):
663+
return []
664+
return [item for item in found if isinstance(item, dict)]
665+
666+
667+
def _competition_lines(entry: Contribution) -> list[str]:
668+
"""What is known about other pull requests on the issue.
669+
670+
Silence is not an option for an open pull request: an unchecked one would
671+
read exactly like an unchallenged one.
672+
"""
673+
if (entry.state or "").upper() != "OPEN":
674+
return []
675+
competition = entry.competition
676+
if competition is None:
677+
return [
678+
"competing pull requests never read -- run `mailman contributions --refresh`"
679+
]
680+
if competition.get("detail"):
681+
return [f"competing pull requests unchecked: {competition['detail']}"]
682+
found = competitors(entry)
683+
if not found:
684+
return [f"no competing pull request on issue #{competition.get('issue')}"]
685+
return [
686+
f"COMPETING: #{item.get('number')} {item.get('state')} by {item.get('author')}, "
687+
f"opened {item.get('created_at')} {item.get('url')}"
688+
for item in found
689+
]
690+
691+
479692
def _reading_age(entry: Contribution, *, now: datetime | None = None) -> str:
480693
if not entry.checked_at:
481694
return "state never read from GitHub -- run `mailman contributions --refresh`"

tests/test_cli.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
from mailman.artifacts import create_run
1818
from mailman.cli import _command_hunt, _emit, main
19+
from mailman.provenance import contribution_from_record
1920

2021

2122
class ContributionsCliTests(unittest.TestCase):
@@ -75,6 +76,41 @@ def test_a_refresh_that_could_not_read_github_exits_non_zero(self) -> None:
7576
self.assertIn(failure, err.getvalue())
7677
self.assertIn("not fresh readings", err.getvalue())
7778

79+
def test_a_competing_pull_request_exits_non_zero_and_names_it(self) -> None:
80+
"""https://github.com/wolfgang-aura/Mailman/issues/86"""
81+
with tempfile.TemporaryDirectory() as temporary_directory:
82+
data_root = Path(temporary_directory) / "runs"
83+
data_root.mkdir()
84+
entry = contribution_from_record(
85+
{
86+
"run_id": "20260908T204404Z-1e0aa3",
87+
"repository": "python/mypy",
88+
"pull_request": 21961,
89+
"state": "OPEN",
90+
"competition": {
91+
"issue": 21960,
92+
"pull_requests": [
93+
{
94+
"number": 21967,
95+
"state": "open",
96+
"author": "EmmanuelNiyonshuti",
97+
"url": "https://github.com/python/mypy/pull/21967",
98+
"created_at": "2026-09-10T20:17:05Z",
99+
}
100+
],
101+
},
102+
}
103+
)
104+
with patch("mailman.cli.refresh_contributions", return_value=([entry], [])):
105+
out, err = StringIO(), StringIO()
106+
with redirect_stdout(out), redirect_stderr(err):
107+
code = main(
108+
["contributions", "--refresh", "--data-root", str(data_root)]
109+
)
110+
self.assertEqual(code, 1)
111+
self.assertIn("COMPETING: #21967", out.getvalue())
112+
self.assertIn("python/mypy#21961: #21967 (open)", err.getvalue())
113+
78114
def test_without_refresh_nothing_calls_github(self) -> None:
79115
with tempfile.TemporaryDirectory() as temporary_directory:
80116
data_root = Path(temporary_directory) / "runs"

0 commit comments

Comments
 (0)