Release notes & QA status: replace (#N) squash-subject regex with commits→PRs API

[sfreudenthaler]

Why

.github/scripts/gather-release-data/src/github.ts and .github/scripts/release-qa-status/src/github.ts both extracted PR numbers from commit subjects with the same \(#(\d+)\)\s*$ regex. That only holds under squash merging.

Squash merging is now disabled on this repo (allow_squash_merge: false), so PRs land as merge commits and the compare range between two release tags contains merge commits and every feature-branch commit underneath them. Two things go wrong:

1. Merged PRs silently disappear. A merge commit subject (Merge pull request #N from …, or a custom merge title) doesn't end in (#N), so its PR is never resolved. It drops out of the AI release notes and out of the QA-status section. release-qa-status warned about this at index.ts:272; gather-release-data was silent.
2. False PR numbers. Feature-branch commits land on main verbatim, and their subjects frequently end in an issue number the author typed. The old regex hands that to pulls.get, which 404s.

Measured against a real release range

v26.08.28-01...v26.08.31-01, 118 commits, 24 merge commits:

	result
old (#N)$ regex	13 numbers — 5 are issue numbers that 404 on pulls.get: #36197, #36720, #36845, #36855, #37066
commits/{sha}/pulls API	exactly the 8 real merged PRs, no false positives

A 38% garbage rate on the most recent release cut. Every release since squash was disabled has run through this path.

What changed

extractPRNumbers is replaced in both packages by an async resolvePRNumbers(octokit, owner, repo, commits) that resolves each commit through GET /repos/{owner}/{repo}/commits/{sha}/pulls (octokit.repos.listPullRequestsAssociatedWithCommit):

• Filters to pr.merged_at — for commits not reachable from the default branch the endpoint also returns open PRs.
• Dedupes through a Set. The API maps a merged PR's branch commits and its merge commit to the same PR number, so branch/merge dedup is free.
• Returns nothing for direct pushes (no associated PR), instead of guessing.
• per_page: 100, so the default 30-item page can never truncate. A default-branch commit resolves to the single merged PR that introduced it (measured: 1 for all 40 sampled commits of v26.08.28-01...v26.08.31-01), so page 1 always suffices — no octokit.paginate needed.
• Batched BATCH_SIZE = 15 with Promise.all and sleep(500) between batches, reusing the existing fetchPRDetails concurrency pattern in the same files.
• Per-commit try/catch writes a warning to stderr and returns [], so one unresolvable sha cannot abort the whole range — consistent with how fetchPRDetails already degrades to partial results.

Rate limiting

One request per commit means 118–228 requests per release range (measured: 118 across v26.08.28-01...v26.08.31-01, 228 across v26.08.24-01...v26.08.28-01). That is only 2–5% of the 5000/hr primary budget, but BATCH_SIZE = 15 with sleep(500) between batches puts us at roughly 1200+ req/min against GitHub's ~900/min REST secondary limit. A 403 there gets swallowed by the per-commit catch and degrades into a PR silently missing from the release notes — the exact failure this PR exists to remove.

So @octokit/plugin-throttling (GitHub's own plugin) is wired into the single createOctokit() choke point in each package. It queues requests and honors retry-after for both the primary and secondary limits. Retries are bounded at MAX_RATE_LIMIT_RETRIES = 3 so an exhausted quota cannot park a release job indefinitely.

No regex fast path. A "two-parent commits only" shortcut was considered and rejected. On the range above it happens to return the identical 8 PRs at 1/5 the API cost — but any range spanning the squash→merge transition contains single-parent squash commits, which it would silently drop. That is the same class of silent loss this PR removes.

Supporting cleanups

• CommitInfo.message was read only by the old regex, so the field (types.ts) and the message-building line in fetchCommitRange are both removed in both packages.
• release-qa-status/src/index.ts: the 11-line "has the merge strategy changed? expected squash-merge commit subjects" stderr warning is deleted, not reworded — it existed solely to flag the squash assumption this PR removes, and the line above it already prints Resolved N merged PRs from M commits.
• release-qa-status/src/github.ts: the stale comment in fetchClosingIssueRefs that justified GraphQL alias interpolation by citing "extractPRNumbers' strict regex" now cites the API. The Number.isInteger(n) && n > 0 belt-and-suspenders filter stays.

The two scripts remain independent parallel copies, per the existing Mirrors the patterns in .github/scripts/gather-release-data/src/github.ts header — no shared module was extracted. Both must land together or the documented mirror pair goes out of sync.

No workflow changes needed. cicd_comp_ai-release-notes-phase.yml:41 already grants pull-requests: read, which covers this endpoint, and the report job in cicd_6-release.yml already calls pulls.get with the same scope.

Testing

gather-release-data — npm test → 36/36 passing, npx tsc --noEmit clean, npm run build clean. The three old extractPRNumbers regex tests are replaced by:

1. Release notes & QA status: replace (#N) squash-subject regex with commits→PRs API #37201 regression — three commits in one fixture: aaa (feature-branch commit whose subject ended in an issue number), bbb (its two-parent merge commit), ccc (direct push). Asserts [37196] and 3 API calls, covering merge-commit resolution, branch/merge dedup, and the direct-push case in one shot.
2. Unmerged PR filtered out (merged_at: null).
3. Batch does not abort on one bad sha — one commit's API call rejects; the sibling's PR is still returned and a process.stderr.write spy sees the failing sha. Mutation-checked: deleting the try/catch from resolvePRNumbers fails this test and only this test (1 failed, 34 passed).
4. Retry cap — onThrottle returns true below MAX_RATE_LIMIT_RETRIES and false at it, so the throttling plugin cannot retry forever. Also asserts the log shapes (retry 1/3, retry 3/3, giving up after 3 retries, and never retry 4/), since the give-up line is only ever read during real rate-limit exhaustion. Mutation-checked: restoring the unconditional message fails it.

release-qa-status — npm test → 40/40 passing, npx tsc --noEmit clean, npm run build clean. No resolvePRNumbers test here: it is a documented verbatim mirror of the gather-release-data copy, and duplicating the fixture buys coverage of the same code twice. The existing findPreviousTag drift guard already fails if the two files diverge on release-boundary resolution.

Octokit is stubbed inline with jest.fn in both suites — no network, no new test harness. All fixtures are ≤3 commits so the inter-batch sleep(500) never runs and no fake timers are needed.

One new dependency (@octokit/plugin-throttling, official, in both packages). No behavior change beyond PR-number resolution and rate-limit handling.

Closes: #37201

Part of dotCMS/private-issues#673 — item 3 of 4. Item 1 (#37203) is closed, item 2 (#37200) landed in #37219, item 4 (#37202) is #37342.

🤖 Generated with Claude Code

This PR fixes: #37201

[claude]

Claude finished @sfreudenthaler's task in 2m 0s —— View job

---

Code Review

• Read both github.ts copies, index.ts callers, tests, types.ts, and package.json
• Reviewed throttling wiring, resolvePRNumbers, dedup/filter logic, error paths
• Checked test coverage and the two-copy mirror invariant

New Issues

• 🟡 Medium: .github/scripts/gather-release-data/src/github.ts:202-208 (and mirror release-qa-status/src/github.ts:187-193) — the residual silent-loss path this PR set out to remove still exists, just less likely. When the throttling plugin exhausts MAX_RATE_LIMIT_RETRIES it re-throws the 403/429; the per-commit catch returns [] and writes one Warning: line to stderr. Unlike fetchPRDetails, which accumulates fetchErrors and emits an aggregate summary (could not fetch N PR(s): #…), resolvePRNumbers has no aggregate count and does not surface a non-zero failure signal, so a partially-resolved range still exits 0 and looks complete. Consider collecting failed shas and emitting a single summary line (mirroring fetchPRDetails) so a degraded run is visible in the log tail rather than buried per-commit. Non-blocking — the throttling plugin makes this rare and the behavior is documented as an intentional degrade.

• 🟡 Medium: .github/scripts/release-qa-status/src/github.ts:23,162 — onThrottle and resolvePRNumbers are exported new behavior in this package but have no test here; the only coverage is in gather-release-data's suite. The PR justifies this as "a documented verbatim mirror," which is reasonable, but the mirror is enforced by convention (the header comment) — nothing fails if the two onThrottle/resolvePRNumbers bodies drift. The findPreviousTag drift guard cited in the PR body only covers release-boundary resolution, not these two functions. Non-blocking, but worth noting the coverage claim rests on humans keeping the copies identical.

Notes (not issues)

• Throttling wiring is correct: @octokit/plugin-throttling@^9.6.1 is the right major for @octokit/rest@^21, both onRateLimit and onSecondaryRateLimit are supplied (the plugin warns if either is missing), and the (retryAfter, options, octokit, retryCount) signature matches the plugin contract. Retry cap semantics are right: retryCount 0/1/2 → retry, 3 → give up = original + 3 retries.
• data.filter((pr) => pr.merged_at) correctly drops open PRs returned for commits not on the default branch; the Set gives branch/merge dedup for free. Matches the #37201 regression test.
• per_page: 100 + the merged_at filter closes the >30 truncation class without needing octokit.paginate for these main..main inputs — agreed with the reasoning in the 19:11 comment.
• CommitInfo.message removal is clean — no lingering readers in either package (grep-verified), and fetchCommitRange now pushes { sha } only.
• The two catch degrade paths are consistent with the pre-existing fetchPRDetails pattern in the same files.

No 🔴 Critical or 🟠 High issues. The two 🟡 Medium items are non-blocking. The change is sound and the regression coverage for #37201 (merge-commit resolution, branch/merge dedup, direct-push, unmerged filter, error-branch mutation check, retry cap) is the right set.
· issue-37201-commits-pulls-api

[sfreudenthaler]

Addressing the three findings from the automatic review — all three were fair, and one turned out to be a live problem.

1 & 2 (test coverage vs. the description) — correct, and the cause was a stale PR body rather than missing work. Commit fe05fea9a2 ("apply ponytail review cuts") deliberately deleted the error-branch test and the release-qa-status drift guard; the Testing section was never updated to match. Verified locally: the claimed 35/35 + 41/41 was actually 34/34 + 40/40 with two cases, not three.

Resolved as follows:

• Error-branch test restored (gather-release-data/src/github.test.ts). That catch is the "one bad sha can't abort the whole range" guarantee and it degrades silently, so it earns a test. Mutation-checked rather than asserted — deleting the try/catch from resolvePRNumbers fails this test and only this test (1 failed, 34 passed).
• release-qa-status drift guard left out, deliberately. It is a documented verbatim mirror; duplicating the fixture covers the same logic twice. The existing findPreviousTag drift guard already fails if the two files diverge on release-boundary resolution. Body now says this explicitly instead of claiming a test that isn't there.
• Body rewritten to the real numbers: 36/36 and 40/40.

3 (API-call volume) — measured, and the primary-budget concern is a non-issue while the secondary limit was a real bug:

range	commits	= API calls
v26.08.28-01...v26.08.31-01	118	118
v26.08.24-01...v26.08.28-01	228	228

That is 2–5% of the 5000/hr primary budget — fine. But BATCH_SIZE = 15 with sleep(500) between batches is roughly 1200+ req/min against GitHub's ~900/min REST secondary limit, so a 228-commit range would likely have tripped it. The per-commit catch would then swallow the 403 and drop that PR from the release notes — reintroducing the exact silent loss this PR removes.

Fixed by wiring @octokit/plugin-throttling into the single createOctokit() choke point in each package, bounded at MAX_RATE_LIMIT_RETRIES = 3 so an exhausted quota can't park a release job. Good catch — thanks.

On the rejected merge-commit fast path: you're right that it cuts calls to the number of PRs, not commits. On the range above it returns the identical 8 PRs at 1/5 the cost. Still rejected, but for a better reason than the original body gave: any range spanning the squash→merge transition contains single-parent squash commits, which a two-parent filter drops silently. Same failure class.

---

Also worth noting for reviewers: the premise is no longer hypothetical. allow_squash_merge is already false on this repo, so every release cut has been running through the broken path. On v26.08.28-01...v26.08.31-01 the old regex emits 13 numbers of which 5 are issue numbers that 404 on pulls.get (#36197, #36720, #36845, #36855, #37066); the API returns exactly the 8 real merged PRs. Details in the updated description.

[sfreudenthaler]

Added per_page: 100 to the listPullRequestsAssociatedWithCommit call in both copies (b92fd85) — thanks for the flag.

Worth recording how narrow the exposure actually is, so it doesn't get re-litigated: the endpoint's documented behavior is that for a commit present in the default branch it returns the merged pull request that introduced the commit. Measured across v26.08.28-01...v26.08.31-01, all 40 sampled commits return exactly 1 PR. Since resolvePRNumbers is only ever fed commits from a main..main compare range, page 1 always suffices. The >30 case needs a commit absent from main, where the endpoint switches to returning open PRs — which the merged_at filter already discards.

So this is belt-and-braces rather than a live bug. Took it anyway: one line per copy closes a silent-truncation class for free, and octokit.paginate would be machinery for a case that can't arise from these inputs.

tsc --noEmit clean, npm run build clean, 36/36 and 40/40 still green.

[sfreudenthaler]

Fixed in 0259189 — real bug in code I added in this PR, and the suggested rewrite was right, so I took it as written in both copies.

The reason it's worth more than "cosmetic": that line only ever prints during genuine rate-limit exhaustion, which is exactly when someone is reading stderr to work out what happened. retry 4/3 followed by silence reads like a truncated log or a broken cap, not a clean give-up.

Also tightened the onThrottle test rather than just fixing the string — it previously asserted only the return values plus a substring match on the kind prefix, which is why the off-by-one slipped through. It now pins all three shapes (retry 1/3, retry 3/3, giving up after 3 retries) and asserts retry 4/ never appears. Mutation-checked: restoring the old unconditional message fails that test and only that test (1 failed, 35 passed).

tsc --noEmit and npm run build clean in both packages; 36/36 and 40/40.

[github-actions]

dotbot code review:

• Reviewer: deepseek/deepseek-v4-pro-0813 (medium)
• Overall: patch is incorrect
• New findings this run: 2
• Prior unresolved dotbot findings still relevant: 0
• Active findings total: 2

Core API change (regex to commits->pulls) is sound, tested, and verified against real release ranges. The rate-limit hook retry timing is a P2 behavioral/log-accuracy issue that degrades retry usefulness but is bounded and not an output-corrupting correctness bug, so it does not warrant marking the patch incorrect.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

_{reviewed by dotbot · deepseek/deepseek-v4-pro-0813 · medium}

[github-actions]

dotbot code review:

• Reviewer: google/gemini-3.8-flash (medium)
• Overall: patch is correct
• New findings this run: 0
• Prior unresolved dotbot findings still relevant: 0
• Active findings total: 0

The change correctly replaces the squash-merge commit subject regex with the commits-to-pulls GitHub API and handles deduplication cleanly. The @octokit/plugin-throttling handler correctly returns a boolean indicating whether to retry; the plugin internally schedules the retry after retryAfter seconds.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

_{reviewed by dotbot · google/gemini-3.8-flash · medium}

[github-actions]

dotbot code review:

• Reviewer: ~z-ai/glm-latest (medium)
• Overall: patch is correct
• New findings this run: 0
• Prior unresolved dotbot findings still relevant: 0
• Active findings total: 0

Verified the full diff end-to-end: npm ci resolves @octokit/plugin-throttling@9.6.1 from both updated lockfiles, tsc --noEmit is clean, and both suites pass (36/36 gather-release-data, 40/40 release-qa-status). The two prior dotbot comments on onThrottle returning true are false positives in the pinned plugin version: @octokit/plugin-throttling@9.6.1's own README states "Return true to automatically retry the request after retryAfter seconds", and its source computes the wait itself (if (wantRetry) { ...; return retryAfter * state2.retryAfterBaseValue } with retryAfterBaseValue: 1000), so the callback's return value is only a wantRetry signal and the Retry-After/reset delay is honored. The suggested return willRetry ? retryAfter : false would be a no-op at best and a regression when retryAfter resolves to 0 (falsy → retry cancelled). The merged_at filter, Set dedup, per_page: 100, bounded retry cap, per-commit catch, and CommitInfo.message removal (no remaining readers, grep-verified; pull-requests: read already granted in both consuming workflows) are all sound.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

_{reviewed by dotbot · ~z-ai/glm-latest · medium}