Skip to content

Commit a2a9800

Browse files
Read every open pull request when selecting
Selection listed at most 100 pull requests. The bot's updates accumulate during a freeze, and the ones past the cap are the oldest, which are exactly the ones that need closing: one left open becomes the newest open pull request for its title once the merged one closes, and merging it would put an earlier description back on the default branch. There are already more than twenty open per version. Page through them all instead. The endpoint that does this cannot filter by author, so the author rule moves next to the title rule in the selection script, where it is tested, and an entry whose author cannot be read is skipped rather than assumed to match. Also keep the whole change summary rather than its first sixty lines, since these updates routinely touch more files than that and the total line was being cut, and skip the pull request comment on a dry run so rollout validation stays read-only and does not leave behind a marker that would suppress the real comment later. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6b022cc6-cf92-4a1a-a283-cc4bcd70288c
1 parent 107496b commit a2a9800

3 files changed

Lines changed: 170 additions & 12 deletions

File tree

‎.github/scripts/select_openapi_prs.py‎

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11
#!/usr/bin/env python3
22
"""Select the OpenAPI description update PRs to merge, and the superseded ones.
33
4-
Reads the JSON array produced by
4+
Reads the pages produced by
55
6-
gh pr list --state open --author <bot> \
7-
--json number,title,headRefOid,createdAt
6+
gh api --paginate --slurp "repos/{repo}/pulls?state=open&per_page=100"
87
98
on stdin and writes `key=value` lines suitable for `$GITHUB_OUTPUT`.
109
10+
Every open pull request is read, following pagination to the end, because a
11+
capped listing would silently drop the oldest matches. Those are exactly the
12+
superseded pull requests that must be closed: one left open becomes the
13+
newest open pull request for its title once the merged one closes, and
14+
merging it would put an earlier description back on the default branch.
15+
1116
Two properties matter for safety and are the reason this is a script rather
1217
than an inline `jq` filter:
1318
@@ -23,6 +28,10 @@
2328
* The head commit SHA of each selected pull request is emitted alongside its
2429
number, so every later step can pin to the exact commit that was inspected
2530
instead of resolving a branch name again.
31+
* The author is matched here rather than by a listing flag, because the
32+
endpoint that pages over every open pull request cannot filter by author.
33+
Keeping the author and title rules together means one tested place decides
34+
what is eligible.
2635
"""
2736

2837
import argparse
@@ -37,6 +46,44 @@
3746
GROUPS = (('30', TITLE_30), ('31', TITLE_31))
3847

3948

49+
def normalise(payload, author):
50+
"""Flatten paginated pages and map API fields to the names used below.
51+
52+
`--slurp` yields one list per page, so the payload is a list of lists.
53+
A single flat list is accepted too, which keeps the parsing rules the
54+
same whichever way the input was produced.
55+
"""
56+
flat = []
57+
for entry in payload:
58+
if isinstance(entry, list):
59+
flat.extend(entry)
60+
else:
61+
flat.append(entry)
62+
63+
normalised = []
64+
for pr in flat:
65+
if not isinstance(pr, dict):
66+
continue
67+
# An entry whose author cannot be read is skipped rather than
68+
# assumed to match: it must never become a merge candidate, and it
69+
# must never be closed as superseded.
70+
user = pr.get('user')
71+
login = user.get('login') if isinstance(user, dict) else None
72+
if login != author:
73+
continue
74+
head = pr.get('head')
75+
normalised.append(
76+
{
77+
'number': pr.get('number'),
78+
'title': pr.get('title'),
79+
'headRefOid': head.get('sha') if isinstance(head, dict) else None,
80+
'createdAt': pr.get('created_at'),
81+
}
82+
)
83+
84+
return normalised
85+
86+
4087
def _sort_key(pr):
4188
# `createdAt` is RFC 3339 in UTC, so lexical order is chronological.
4289
# The number breaks ties deterministically.
@@ -94,6 +141,10 @@ def outputs(pull_requests):
94141

95142
def main(argv=None):
96143
parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
144+
parser.add_argument(
145+
'--author',
146+
help='only consider pull requests opened by this login',
147+
)
97148
parser.add_argument(
98149
'--title',
99150
choices=[suffix for suffix, _ in GROUPS],
@@ -105,12 +156,16 @@ def main(argv=None):
105156
print(dict(GROUPS)[args.title])
106157
return 0
107158

159+
if not args.author:
160+
print('--author is required when selecting', file=sys.stderr)
161+
return 2
162+
108163
payload = json.load(sys.stdin)
109164
if not isinstance(payload, list):
110165
print('expected a JSON array of pull requests', file=sys.stderr)
111166
return 2
112167

113-
for line in outputs(payload):
168+
for line in outputs(normalise(payload, args.author)):
114169
print(line)
115170
return 0
116171

‎.github/scripts/test_select_openapi_prs.py‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,78 @@ def test_malformed_entries_are_ignored(self):
155155
self.assertEqual(out['superseded_30'], '')
156156

157157

158+
def api_pr(number, title, sha='sha', created='2026-01-01T00:00:00Z', login='bot'):
159+
"""One entry shaped like the REST pulls endpoint returns it."""
160+
return {
161+
'number': number,
162+
'title': title,
163+
'head': {'sha': sha},
164+
'created_at': created,
165+
'user': {'login': login},
166+
}
167+
168+
169+
class NormaliseTest(unittest.TestCase):
170+
"""Reading every page matters: a dropped page leaves superseded PRs open."""
171+
172+
def test_pages_are_flattened_and_fields_mapped(self):
173+
pages = [
174+
[api_pr(1, sel.TITLE_30, sha='a', created='2026-01-01T00:00:00Z')],
175+
[api_pr(2, sel.TITLE_30, sha='b', created='2026-02-01T00:00:00Z')],
176+
]
177+
self.assertEqual(
178+
sel.normalise(pages, 'bot'),
179+
[
180+
{
181+
'number': 1,
182+
'title': sel.TITLE_30,
183+
'headRefOid': 'a',
184+
'createdAt': '2026-01-01T00:00:00Z',
185+
},
186+
{
187+
'number': 2,
188+
'title': sel.TITLE_30,
189+
'headRefOid': 'b',
190+
'createdAt': '2026-02-01T00:00:00Z',
191+
},
192+
],
193+
)
194+
195+
def test_a_flat_list_is_accepted_too(self):
196+
self.assertEqual(len(sel.normalise([api_pr(1, sel.TITLE_30)], 'bot')), 1)
197+
198+
def test_another_author_is_never_eligible(self):
199+
pages = [[api_pr(1, sel.TITLE_30, login='someone-else')]]
200+
self.assertEqual(sel.normalise(pages, 'bot'), [])
201+
202+
def test_an_unreadable_author_is_skipped_rather_than_assumed(self):
203+
pages = [[{'number': 1, 'title': sel.TITLE_30, 'head': {'sha': 'a'}}]]
204+
self.assertEqual(sel.normalise(pages, 'bot'), [])
205+
206+
def test_a_malformed_entry_does_not_abort_the_page(self):
207+
pages = [['not a dict', api_pr(2, sel.TITLE_30), None]]
208+
self.assertEqual([pr['number'] for pr in sel.normalise(pages, 'bot')], [2])
209+
210+
def test_a_missing_head_survives_normalisation_and_blocks_pinning(self):
211+
pages = [[{'number': 1, 'title': sel.TITLE_30, 'user': {'login': 'bot'}}]]
212+
normalised = sel.normalise(pages, 'bot')
213+
self.assertIsNone(normalised[0]['headRefOid'])
214+
self.assertEqual(sel.outputs(normalised)[0], 'found=false')
215+
216+
def test_an_older_pull_request_on_a_later_page_is_still_superseded(self):
217+
# The regression this guards: the oldest matches arrive last, and
218+
# dropping them would leave a stale pull request open to be selected
219+
# by a later run once the merged one closes.
220+
pages = [
221+
[api_pr(9, sel.TITLE_30, sha='new', created='2026-05-01T00:00:00Z')],
222+
[api_pr(1, sel.TITLE_30, sha='old', created='2026-01-01T00:00:00Z')],
223+
]
224+
out = dict(line.split('=', 1) for line in sel.outputs(sel.normalise(pages, 'bot')))
225+
self.assertEqual(out['pr_30'], '9')
226+
self.assertEqual(out['sha_30'], 'new')
227+
self.assertEqual(out['superseded_30'], '1')
228+
229+
158230
class CliTest(unittest.TestCase):
159231
def _run(self, argv):
160232
import io
@@ -171,6 +243,15 @@ def test_title_flag_prints_the_exact_title_for_a_group(self):
171243
self.assertEqual(self._run(['--title', '30']), (0, [sel.TITLE_30]))
172244
self.assertEqual(self._run(['--title', '31']), (0, [sel.TITLE_31]))
173245

246+
def test_selecting_without_an_author_is_refused(self):
247+
import io
248+
249+
captured, sys.stderr = sys.stderr, io.StringIO()
250+
try:
251+
self.assertEqual(sel.main([]), 2)
252+
finally:
253+
sys.stderr = captured
254+
174255
def test_title_flag_rejects_an_unknown_group(self):
175256
import io
176257

‎.github/workflows/auto-merge-openapi-updates.yml‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,16 @@ jobs:
9898
run: |
9999
set -euo pipefail
100100
101-
gh pr list \
102-
--state open \
103-
--author "$BOT_LOGIN" \
104-
--limit 100 \
105-
--json number,title,headRefOid,createdAt \
106-
| python3 tools/.github/scripts/select_openapi_prs.py >selection.txt
101+
# Every open pull request by the bot, following pagination rather
102+
# than stopping at a fixed count. A cap that silently drops the
103+
# oldest matches would leave superseded pull requests open, and a
104+
# later run would then treat one of them as the newest open
105+
# candidate and put an earlier description back on the default
106+
# branch. During a long freeze these accumulate well past one page.
107+
gh api --paginate --slurp \
108+
"repos/$GH_REPO/pulls?state=open&per_page=100" \
109+
| python3 tools/.github/scripts/select_openapi_prs.py \
110+
--author "$BOT_LOGIN" >selection.txt
107111
108112
cat selection.txt
109113
cat selection.txt >>"$GITHUB_OUTPUT"
@@ -308,13 +312,31 @@ jobs:
308312
exit 0
309313
fi
310314
311-
git diff --stat "$base_commit" "$sha" | sed -n '1,60p' >"summary-$pr.txt"
315+
# Kept whole: a truncated inventory would omit files and the
316+
# total line, which is the part worth reading. These updates
317+
# routinely touch dozens of files but only a few kilobytes of
318+
# stat, so this stays far inside the comment size limit. The
319+
# guard below is for the pathological case only.
320+
git diff --stat "$base_commit" "$sha" >"summary-$pr.txt"
321+
if [ "$(wc -c <"summary-$pr.txt")" -gt 60000 ]; then
322+
echo "::notice::#$pr has an unusually large change summary; truncating."
323+
# `sed -n` rather than `head`: it consumes all input, so the
324+
# producer is never killed by SIGPIPE under `pipefail`.
325+
sed -n '1,500p' "summary-$pr.txt" >"summary-$pr.trimmed"
326+
printf '\n... truncated; see the pull request files tab.\n' \
327+
>>"summary-$pr.trimmed"
328+
mv "summary-$pr.trimmed" "summary-$pr.txt"
329+
fi
312330
done
313331
314332
echo "status=pinned" >>"$GITHUB_OUTPUT"
315333
316334
- name: Post change summary to pull requests
317-
if: steps.pin.outputs.status == 'pinned'
335+
# A dry run is for confirming selection, so it stays read-only apart
336+
# from the run summary. Posting here would also leave a comment
337+
# carrying this commit's marker, which the later real run would treat
338+
# as "already summarised" and skip.
339+
if: steps.pin.outputs.status == 'pinned' && inputs.dry_run != true
318340
working-directory: repo
319341
env:
320342
GH_TOKEN: ${{ github.token }}

0 commit comments

Comments
 (0)