Skip to content

Commit 2dd2296

Browse files
myasnikovdaniilmyasnikovdaniil
committed
fix(release): fail the audit on a truncated listing instead of shortening it
Each of the three `gh` listings the audit makes is a completeness claim -- these are all the labelled PRs, this is every release, this is every PR on the branch -- and gh stops at --limit in silence. A saturated query therefore does not make the audit partial, it makes its verdicts wrong: the PRs past the cut are reported nowhere and the exit code says clean. That is the same silent-green failure this tool exists to catch, reached from the inside. None of the three was near its cap when they were chosen and two are now inside 30% of it: 290 labelled PRs against a 400 default, 217 releases against 300, and at most 54 PRs on a release branch against 500. The labelled-PR count is the one that grows with every backport. Check each listing against its cap and exit 2 naming the cap to raise, rather than returning a short list. Raise the caps well clear of the current populations while at it: gh stops as soon as the result set is exhausted, so a higher cap costs nothing when it is not reached -- 12.2s against 13.2s for the same two lines at 400 and at 1000 -- and the reason to bound them at all is only to stop a runaway query paginating forever. Output over release-1.6, release-1.5 and release-1.4 is unchanged. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 2b53cf9 commit 2dd2296

2 files changed

Lines changed: 48 additions & 6 deletions

File tree

‎cmd/backport-audit/README.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ Requires `git` and an authenticated `gh` on PATH. The audit is read-only: it fet
3030
| Flag | Meaning |
3131
|------|---------|
3232
| `--remote NAME` | Git remote holding the release branches (default `origin`) |
33-
| `--limit N` | Max merged PRs scanned per label (default 400) |
33+
| `--limit N` | Max merged PRs scanned per label spelling (default 1000) |
3434
| `--no-fetch` | Trust the local refs as-is, skipping `git fetch` |
3535
| `--json` | Machine-readable output |
3636
| `--no-color` | Disable color (auto-disabled on a non-TTY and under `--json`) |
@@ -39,14 +39,16 @@ Arguments and flags may be given in any order.
3939

4040
## Exit code
4141

42-
`0` when nothing is outstanding, `1` when something is. That is the point of the tool, so it holds in `--json` mode too:
42+
`0` when nothing is outstanding, `1` when something is, `2` when the audit could not be completed. That is the point of the tool, so it holds in `--json` mode too:
4343

4444
```bash
4545
go run ./cmd/backport-audit release-1.5 || echo "do not cut yet"
4646
```
4747

4848
`go run` forwards the exit code but also prints its own `exit status 1` line to stderr. Build the binary first (`go build ./cmd/backport-audit`) where that noise is unwelcome.
4949

50+
Exit `2` covers the cases where an answer cannot be trusted rather than merely being bad news, and a saturated listing is one of them: `gh` truncates at `--limit` in silence, and a truncated list does not make the audit partial, it makes it wrong — the PRs past the cut are reported nowhere and the exit code says clean. Each of the three listings is checked against its cap and fails instead, naming the cap to raise.
51+
5052
## Statuses
5153

5254
| Status | Meaning | Action |

‎cmd/backport-audit/main.go‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ Arguments:
6969
7070
Options:
7171
--remote NAME Git remote holding the release branches (default: origin)
72-
--limit N Max merged PRs to scan per label (default: 400)
72+
--limit N Max merged PRs to scan per label (default: 1000)
7373
--no-fetch Trust the local refs as-is, skipping git fetch
7474
--json Machine-readable output
7575
--no-color Disable color output (auto-disabled on non-TTY)
@@ -88,6 +88,18 @@ var (
8888
freezeTagRE = regexp.MustCompile(`^v(\d+)\.(\d+)\.0-rc\.\d+$`)
8989
)
9090

91+
// Caps on the three listings the audit makes. gh truncates at --limit in
92+
// silence, so each one is checked against its cap by truncated() and the
93+
// numbers are set well clear of the current populations rather than close to
94+
// them: at the time of writing, 290 labelled PRs, 217 releases, and at most 54
95+
// PRs on any one release branch. Raising a cap costs nothing when it is not
96+
// reached -- gh stops as soon as the result set is exhausted -- so the reason
97+
// they are bounded at all is to keep a runaway query from paginating forever.
98+
const (
99+
releaseListCap = 1000
100+
branchPRCap = 1000
101+
)
102+
91103
// freezeContractLandedAt is when cutting an rc started creating release-X.Y,
92104
// and with it when backport.yaml stopped resolving its targets from the last
93105
// published stable.
@@ -320,7 +332,7 @@ func run() int {
320332
}
321333

322334
func parseArgs(args []string) (*config, int, bool) {
323-
cfg := &config{remote: "origin", limit: 400, fetch: true, useColor: true}
335+
cfg := &config{remote: "origin", limit: 1000, fetch: true, useColor: true}
324336
for i := 0; i < len(args); i++ {
325337
a := args[i]
326338
next := func() (string, bool) {
@@ -424,6 +436,22 @@ func gitOK(args ...string) bool {
424436
return exec.Command("git", args...).Run() == nil
425437
}
426438

439+
// truncated turns a saturated `gh ... --limit N` query into an error instead of
440+
// a quietly shorter answer.
441+
//
442+
// Every list this tool makes is a completeness claim -- these are all the
443+
// candidates, this is every backport PR on the branch -- and gh stops at
444+
// --limit without saying so. A saturated query therefore does not make the
445+
// audit partial, it makes its verdicts wrong: the PRs past the cut are
446+
// reported nowhere, and the exit code says clean. That is the same failure
447+
// class the tool exists to catch, so it fails loudly instead.
448+
func truncated(got, limit int, what, remedy string) error {
449+
if got < limit {
450+
return nil
451+
}
452+
return fmt.Errorf("%s hit its cap of %d results, so the list is truncated and the audit would be unsound: %s", what, limit, remedy)
453+
}
454+
427455
func ghJSON(target any, args ...string) error {
428456
out, err := capture("gh", args...)
429457
if err != nil {
@@ -489,10 +517,14 @@ func lineOpenDates() ([]lineOpen, error) {
489517
IsPrerelease bool `json:"isPrerelease"`
490518
IsDraft bool `json:"isDraft"`
491519
}
492-
if err := ghJSON(&releases, "release", "list", "--limit", "300",
520+
if err := ghJSON(&releases, "release", "list", "--limit", strconv.Itoa(releaseListCap),
493521
"--json", "tagName,publishedAt,isPrerelease,isDraft"); err != nil {
494522
return nil, err
495523
}
524+
if err := truncated(len(releases), releaseListCap, "the release listing",
525+
"raise releaseListCap in cmd/backport-audit"); err != nil {
526+
return nil, err
527+
}
496528

497529
earliest := map[line]time.Time{}
498530
for _, rel := range releases {
@@ -624,6 +656,10 @@ func candidates(limit int) (map[int]*mainPR, error) {
624656
"--json", "number,title,url,mergedAt,mergeCommit,labels,author"); err != nil {
625657
return nil, err
626658
}
659+
if err := truncated(len(prs), limit, "the merged-PR query for label "+spelling,
660+
"re-run with a higher --limit"); err != nil {
661+
return nil, err
662+
}
627663
for i := range prs {
628664
pr := prs[i]
629665
existing, ok := found[pr.Number]
@@ -706,10 +742,14 @@ func (h *branchHistory) hasCherryPickOf(oids []string) bool {
706742
// hand-written backport carries.
707743
func backportPRsFor(branch string) (map[int][]backportPR, error) {
708744
var prs []backportPR
709-
if err := ghJSON(&prs, "pr", "list", "--base", branch, "--state", "all", "--limit", "500",
745+
if err := ghJSON(&prs, "pr", "list", "--base", branch, "--state", "all", "--limit", strconv.Itoa(branchPRCap),
710746
"--json", "number,title,url,state,headRefName,body,isDraft,comments"); err != nil {
711747
return nil, err
712748
}
749+
if err := truncated(len(prs), branchPRCap, "the PR listing for "+branch,
750+
"raise branchPRCap in cmd/backport-audit"); err != nil {
751+
return nil, err
752+
}
713753

714754
index := map[int][]backportPR{}
715755
for _, pr := range prs {

0 commit comments

Comments
 (0)