fix(release): fail check --cut while a workflow original waits alone

check --cut returned 0 whenever nothing was missing and nothing was
only on the batch, so a workflow original waiting for a maintainer
passed the cut once the pick beside it was labelled backport/skip or
reached the line by hand. The waiting original was not even printed:
the case that held it swallowed the warning the plain check gives.

check now sorts every original into one disposition and exits once,
so a wait fails the cut on its own. A backport/skip original the open
batch still carries is named with the run --defer that takes it out.

Assisted-By: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
This commit is contained in:
Myasnikov Daniil committed 2026-11-09 14:52:13 +00:00
1 parent 63abbc80ee
commit 35c47f20c1
7 files changed
+342 -45

No files matched your search

+3 -1
View File
@@ -42,7 +42,9 @@ The pull request body goes to stdout, built from the line's own `.github/PULL_RE
Exits 1 and prints the URL of every pull request the rules take that is neither on the line nor on the open batch pull request, 0 when there is none, and 2 when it cannot tell. A pull request that the open batch lists as deferred, a conflict or one taken out with `--defer`, prints as a warning and does not fail the check: deferring is a maintainer's call. So does one it lists as waiting for a maintainer because it changes workflows.
`--cut` makes it the gate before a patch release or an rc is cut from a line. A cut tags the line, so a pull request that is only on the open batch pull request fails it too, with one line each: `#N is only on open batch PR #M, merge it before the cut`. One that waits there for a maintainer fails it as well, since nobody decided to leave it out: bring it over, or take it out with `run --defer` and a reason. Everything else is as without it. The scheduled run checks without `--cut`, since its batch is open by design.
`--cut` makes it the gate before a patch release or an rc is cut from a line. A cut tags the line, so a pull request that is only on the open batch pull request fails it too, with one line each: `#N is only on open batch PR #M, merge it before the cut`. One that waits there for a maintainer fails it as well, since nobody decided to leave it out, also when it is all the batch has left: bring it over, or take it out with `run --defer` and a reason. Everything else is as without it, and a cut that passes says `no change the policy takes is missing before the cut`. The scheduled run checks without `--cut`, since its batch is open by design.
An original with `backport/skip` that the open batch picked before the label came prints a warning in either mode, naming the batch and `run --defer`, which takes it out; it fails neither.
### fold
+90 -37
View File
@@ -18,6 +18,7 @@ package main
import (
"fmt"
"slices"
"strconv"
)
@@ -46,59 +47,111 @@ func (a *app) check(args []string) int {
if err != nil {
return a.fail(err)
}
deferred := map[int]deferredEntry{}
if b.pr != nil {
if deferred, err = parseDeferred(b.pr.Body, l); err != nil {
return a.fail(fmt.Errorf("batch pull request #%d: %w", b.pr.Number, err))
disp, err := b.dispositions(*cut)
if err != nil {
return a.fail(err)
}
var missing, failing []string
for _, v := range b.verdicts {
d, ok := disp[v]
switch {
case !ok:
case d.kind == outcomeMissing:
missing = append(missing, d.text)
case d.kind == outcomeUnmerged || d.kind == outcomeWaiting:
failing = append(failing, d.text)
case d.kind == outcomeWarning:
a.warn("%s", d.text)
case d.kind == outcomeNote:
fmt.Fprintf(a.stderr, "note: %s\n", d.text)
}
}
var missing, unmerged, waiting []*verdict
for _, v := range b.qualifying() {
e, isDeferred := deferred[v.o.number]
switch {
case v.onBatch != "":
if *cut {
unmerged = append(unmerged, v)
}
case isDeferred && e.Workflows && *cut:
waiting = append(waiting, v)
case isDeferred && e.Workflows:
a.warn("waiting on #%d for a maintainer, since it changes %s: %s %s", b.pr.Number, workflowsDir, v.o.url(), terminalSafe(v.o.title))
case isDeferred && e.Reason != "":
a.warn("deferred on #%d to the next batch: %s %s (%s)", b.pr.Number, v.o.url(), terminalSafe(v.o.title), terminalSafe(e.Reason))
case isDeferred:
a.warn("deferred conflict on #%d: %s %s", b.pr.Number, v.o.url(), terminalSafe(v.o.title))
default:
missing = append(missing, v)
if len(missing)+len(failing) == 0 {
if *cut {
fmt.Fprintf(a.stdout, "%s: no change the policy takes is missing before the cut\n", l.branch())
} else {
fmt.Fprintf(a.stdout, "%s: every change the policy takes is on the line or on the batch\n", l.branch())
}
return 0
}
where := "and no batch pull request is open"
if b.pr != nil {
where = "nor on batch pull request #" + strconv.Itoa(b.pr.Number)
}
switch {
case len(missing) == 0 && len(unmerged) == 0 && len(waiting) == 0 && *cut:
fmt.Fprintf(a.stdout, "%s: every change the policy takes is on the line\n", l.branch())
return 0
case len(missing) == 0 && len(unmerged) == 0:
fmt.Fprintf(a.stdout, "%s: every change the policy takes is on the line or on the batch\n", l.branch())
return 0
}
if len(missing) > 0 {
fmt.Fprintf(a.stdout, "%s: %d changes the policy takes are not on the line, %s:\n", l.branch(), len(missing), where)
for _, v := range missing {
fmt.Fprintf(a.stdout, " %s %s\n", v.o.url(), terminalSafe(v.o.title))
for _, text := range missing {
fmt.Fprintf(a.stdout, " %s\n", text)
}
}
for _, v := range unmerged {
fmt.Fprintf(a.stdout, "%s: #%d is only on open batch PR #%d, merge it before the cut: %s %s\n", l.branch(), v.o.number, b.pr.Number, v.o.url(), terminalSafe(v.o.title))
}
for _, v := range waiting {
fmt.Fprintf(a.stdout, "%s: #%d changes %s and waits on batch PR #%d for a maintainer, bring it over or defer it before the cut: %s %s\n", l.branch(), v.o.number, workflowsDir, b.pr.Number, v.o.url(), terminalSafe(v.o.title))
for _, text := range failing {
fmt.Fprintf(a.stdout, "%s: %s\n", l.branch(), text)
}
return 1
}
// outcome is what check makes of one original.
type outcome int
const (
// outcomeMissing fails check and the cut: nothing accounts for an
// original the line lacks.
outcomeMissing outcome = iota + 1
// outcomeUnmerged and outcomeWaiting fail the cut only: the cut would not
// carry what is only on the open batch, nor what waits there.
outcomeUnmerged
outcomeWaiting
outcomeWarning
outcomeNote
)
type disposition struct {
kind outcome
text string
}
// dispositions says what check reports about each original that needs a line
// of its own; the others pass in silence. The text is what follows the line's
// name, or the original alone for one that is missing.
func (b *batch) dispositions(cut bool) (map[*verdict]disposition, error) {
deferred := map[int]deferredEntry{}
var stale []*verdict
if b.pr != nil {
var err error
if deferred, err = parseDeferred(b.pr.Body, b.line); err != nil {
return nil, fmt.Errorf("batch pull request #%d: %w", b.pr.Number, err)
}
if stale, err = keptOut(b, b.picks); err != nil {
return nil, err
}
}
out := map[*verdict]disposition{}
for _, v := range b.verdicts {
named := v.o.url() + " " + terminalSafe(v.o.title)
e, isDeferred := deferred[v.o.number]
switch {
case slices.Contains(stale, v):
out[v] = disposition{outcomeWarning, fmt.Sprintf("#%d is still on batch #%d; take it out with backport-batch run --defer: %s", v.o.number, b.pr.Number, named)}
case v.rule != 0:
case v.onBatch != "":
if cut {
out[v] = disposition{outcomeUnmerged, fmt.Sprintf("#%d is only on open batch PR #%d, merge it before the cut: %s", v.o.number, b.pr.Number, named)}
}
case isDeferred && e.Workflows && cut:
out[v] = disposition{outcomeWaiting, fmt.Sprintf("#%d changes %s and waits on batch PR #%d for a maintainer, bring it over or defer it before the cut: %s", v.o.number, workflowsDir, b.pr.Number, named)}
case isDeferred && e.Workflows:
out[v] = disposition{outcomeWarning, fmt.Sprintf("waiting on #%d for a maintainer, since it changes %s: %s", b.pr.Number, workflowsDir, named)}
case isDeferred && e.Reason != "":
out[v] = disposition{outcomeWarning, fmt.Sprintf("deferred on #%d to the next batch: %s (%s)", b.pr.Number, named, terminalSafe(e.Reason))}
case isDeferred:
out[v] = disposition{outcomeWarning, fmt.Sprintf("deferred conflict on #%d: %s", b.pr.Number, named)}
default:
out[v] = disposition{outcomeMissing, named}
}
}
return out, nil
}
// terminalSafe keeps a title from carrying escape sequences into a terminal.
func terminalSafe(s string) string { return capped(oneLine(clean(s, false)), titleLimit) }
+173
View File
@@ -186,6 +186,179 @@ func TestCheck(t *testing.T) {
}
}
// Every category an original can be in, through the commands that put it
// there, with what check and check --cut make of it: the exit code, and the
// warning or note on stderr that leaves it 0.
func TestCheckDispositions(t *testing.T) {
plain := func(t *testing.T) *fixture {
f := newFixture(t)
f.lines()
f.pr(5, "fix(app): five", "", map[string]string{"app/five.txt": "5\n"})
f.pr(6, "fix(app): six", "", map[string]string{"app/six.txt": "6\n"})
return f
}
// conflicting has #5 conflict on the line, beside #6, which picks.
conflicting := func(t *testing.T) *fixture {
f := newFixture(t)
f.lines()
f.onLine("release-1.1", "fix(app): a line-only five", map[string]string{"app/five.txt": "line\n"})
f.pr(5, "fix(app): five", "", map[string]string{"app/five.txt": "5\n"})
f.pr(6, "fix(app): six", "", map[string]string{"app/six.txt": "6\n"})
return f
}
must := func(t *testing.T, f *fixture, gh *fakeGH, args ...string) {
t.Helper()
if code, _, stderr := f.exec(gh, args...); code != 0 {
t.Fatalf("%q = %d\n%s", args, code, stderr)
}
f.git("switch", "-q", "main")
if len(gh.bodies) > 0 {
reopen(f, gh)
}
}
sixOnLine := func(f *fixture) { pickOnLine(f, 6, map[string]string{"app/six.txt": "6\n"}) }
cases := []struct {
name string
arrange func(t *testing.T) (*fixture, *fakeGH)
line string
check int
cut int
checkErr string // on stderr from check
cutOut string // on stdout from check --cut
cutErr string // on stderr from check --cut
notCutOut string
}{
{
name: "missing",
arrange: func(t *testing.T) (*fixture, *fakeGH) { return plain(t), &fakeGH{} },
check: 1, cut: 1, cutOut: "/pull/5 ",
},
{
name: "carried",
arrange: func(t *testing.T) (*fixture, *fakeGH) {
f := plain(t)
pickOnLine(f, 5, map[string]string{"app/five.txt": "5\n"})
sixOnLine(f)
return f, &fakeGH{}
},
check: 0, cut: 0, notCutOut: "/pull/5",
},
{
name: "picked, on the open batch",
arrange: func(t *testing.T) (*fixture, *fakeGH) {
f, gh := plain(t), &fakeGH{}
openBatch(t, f, gh)
return f, gh
},
check: 0, cut: 1, cutOut: "#5 is only on open batch PR #90",
},
{
name: "deferred conflict, open record",
arrange: func(t *testing.T) (*fixture, *fakeGH) {
f, gh := conflicting(t), &fakeGH{}
openBatch(t, f, gh)
sixOnLine(f)
return f, gh
},
check: 0, cut: 0, checkErr: "warning: deferred conflict on #90: https://github.com/cozystack/cozystack/pull/5 ", cutErr: "warning: deferred conflict on #90",
},
{
name: "deferred with a reason, open record",
arrange: func(t *testing.T) (*fixture, *fakeGH) {
f, gh := plain(t), &fakeGH{}
openBatch(t, f, gh)
must(t, f, gh, "run", "--line", "release-1.1", "--defer", "5", "--reason", "fails the app e2e", "--push")
sixOnLine(f)
return f, gh
},
check: 0, cut: 0, checkErr: "warning: deferred on #90 to the next batch: https://github.com/cozystack/cozystack/pull/5 ", cutErr: "(fails the app e2e)",
},
{
name: "workflow wait, open record, beside a pick",
arrange: func(t *testing.T) (*fixture, *fakeGH) {
f, gh := workflowWorld(t), &fakeGH{}
must(t, f, gh, "run", "--line", "release-1.1", "--no-workflow-files", "--push")
return f, gh
},
check: 0, cut: 1, checkErr: "warning: waiting on #90 for a maintainer", cutOut: "#5 changes .github/workflows/ and waits on batch PR #90",
},
{
name: "workflow wait, open record, alone",
arrange: func(t *testing.T) (*fixture, *fakeGH) {
f, gh := workflowWorld(t), &fakeGH{}
must(t, f, gh, "run", "--line", "release-1.1", "--no-workflow-files", "--push")
pickOnLine(f, 6, map[string]string{"app/six.txt": "6\n", ".github/renovate.json": "{}\n"})
return f, gh
},
check: 0, cut: 1, checkErr: "warning: waiting on #90 for a maintainer", cutOut: "#5 changes .github/workflows/ and waits on batch PR #90",
},
{
name: "backport/skip, not picked",
arrange: func(t *testing.T) (*fixture, *fakeGH) {
f := plain(t)
sixOnLine(f)
return f, &fakeGH{labels: map[string][]int{labelSkip: {5}}}
},
check: 0, cut: 0, notCutOut: "/pull/5",
},
{
name: "backport/skip after the pick, still on the open batch",
arrange: func(t *testing.T) (*fixture, *fakeGH) {
f, gh := plain(t), &fakeGH{}
openBatch(t, f, gh)
sixOnLine(f)
gh.labels = map[string][]int{labelSkip: {5}}
return f, gh
},
check: 0, cut: 0, checkErr: "warning: #5 is still on batch #90; take it out with backport-batch run --defer", cutErr: "still on batch #90",
},
{
name: "rule 6",
arrange: func(t *testing.T) (*fixture, *fakeGH) {
f := newFixture(t)
f.lines()
f.pr(5, "fix(app): five", "", map[string]string{"app/five.txt": "5\n"})
return f, &fakeGH{}
},
line: "release-1.0",
check: 0, cut: 0, notCutOut: "/pull/5",
},
{
name: "rules 2 to 4",
arrange: func(t *testing.T) (*fixture, *fakeGH) {
f := newFixture(t)
f.lines()
f.pr(5, "feat(app): five", "", map[string]string{"app/five.txt": "5\n"})
return f, &fakeGH{}
},
check: 0, cut: 0, notCutOut: "/pull/5",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
f, gh := tc.arrange(t)
line := tc.line
if line == "" {
line = "release-1.1"
}
code, stdout, stderr := f.exec(gh, "check", "--line", line)
if code != tc.check || !strings.Contains(stderr, tc.checkErr) {
t.Errorf("check = %d, want %d with %q\nstdout:\n%s\nstderr:\n%s", code, tc.check, tc.checkErr, stdout, stderr)
}
code, stdout, stderr = f.exec(gh, "check", "--line", line, "--cut")
if code != tc.cut || !strings.Contains(stdout, tc.cutOut) || !strings.Contains(stderr, tc.cutErr) {
t.Errorf("check --cut = %d, want %d with %q, %q\nstdout:\n%s\nstderr:\n%s", code, tc.cut, tc.cutOut, tc.cutErr, stdout, stderr)
}
if tc.notCutOut != "" && strings.Contains(stdout, tc.notCutOut) {
t.Errorf("check --cut lists %q:\n%s", tc.notCutOut, stdout)
}
if tc.cut == 0 && !strings.Contains(stdout, "no change the policy takes is missing before the cut") {
t.Errorf("check --cut passes without saying so:\n%s", stdout)
}
})
}
}
type errString string
func (e errString) Error() string { return string(e) }
+64
View File
@@ -0,0 +1,64 @@
/*
Copyright 2026 The Cozystack Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package main
import (
"fmt"
"strings"
"testing"
)
// pickOnLine lands original n on release-1.1 as a maintainer's pick of its
// merge commit, with the -x line git appends, so the line carries it.
func pickOnLine(f *fixture, n int, files map[string]string) {
f.t.Helper()
mc, _, _ := strings.Cut(f.git("log", "--first-parent", "--format=%H", "--grep", fmt.Sprintf("(#%d)$", n), "main"), "\n")
f.onLine("release-1.1", fmt.Sprintf("fix: original %d\n\n(cherry picked from commit %s)", n, mc), files)
}
// A workflow original that waits on the open batch fails the cut, also when
// it is all the batch has left: the pick beside it was labelled
// backport/skip after the pick, or reached the line by hand.
func TestCutFailsWhenOnlyAWorkflowWaitIsLeft(t *testing.T) {
for _, tc := range []struct {
name string
leave func(f *fixture, gh *fakeGH)
}{
{"skip after the pick", func(_ *fixture, gh *fakeGH) { gh.labels = map[string][]int{labelSkip: {6}} }},
{"picked onto the line by hand", func(f *fixture, _ *fakeGH) {
pickOnLine(f, 6, map[string]string{"app/six.txt": "6\n", ".github/renovate.json": "{}\n"})
}},
} {
t.Run(tc.name, func(t *testing.T) {
f := workflowWorld(t)
gh := &fakeGH{}
if code, _, stderr := f.exec(gh, "run", "--line", "release-1.1", "--no-workflow-files", "--push"); code != 0 {
t.Fatalf("run --push = %d\n%s", code, stderr)
}
f.git("switch", "-q", "main")
reopen(f, gh)
tc.leave(f, gh)
code, stdout, stderr := f.exec(gh, "check", "--line", "release-1.1", "--cut")
if code != 1 || !strings.Contains(stdout, "#5 changes .github/workflows/ and waits on batch PR #90 for a maintainer") {
t.Errorf("check --cut = %d, want 1 naming #5:\nstdout:\n%s\nstderr:\n%s", code, stdout, stderr)
}
if strings.Contains(stdout, "on the line or on the batch") {
t.Errorf("check --cut passes the line while #5 waits:\n%s", stdout)
}
})
}
}
+5 -4
View File
@@ -203,6 +203,8 @@ type batch struct {
verdicts []*verdict
pr *pullRequest
prHead string
// picks is what the open batch pull request carries beyond the line.
picks *history
// start is when the line started under the policy, zero for its fork point.
start time.Time
// textUntil is the batch-only day: what a commit's text says counts as
@@ -300,13 +302,12 @@ func (a *app) prepare(l line) (*batch, error) {
if err != nil {
return nil, err
}
var prPicks *history
if pr != nil {
b.pr = pr
if b.prHead, err = batchHead(pr); err != nil {
return nil, err
}
if prPicks, err = readPicks(b.prHead, b.lineHead, b.textUntil); err != nil {
if b.picks, err = readPicks(b.prHead, b.lineHead, b.textUntil); err != nil {
return nil, err
}
}
@@ -315,8 +316,8 @@ func (a *app) prepare(l line) (*batch, error) {
if err != nil {
return nil, err
}
if v.rule == 0 && prPicks != nil {
v.onBatch, _ = prPicks.found(v.o, v.commits)
if v.rule == 0 && b.picks != nil {
v.onBatch, _ = b.picks.found(v.o, v.commits)
}
b.verdicts = append(b.verdicts, v)
}
+6 -2
View File
@@ -173,8 +173,12 @@ func TestDeferKeepsWorkflowChangesWaiting(t *testing.T) {
}
f.git("switch", "-q", "main")
reopen(f, gh)
if code, stdout, _ := f.exec(gh, "check", "--line", "release-1.1", "--cut"); strings.Contains(stdout, "#5 changes") {
t.Errorf("check --cut = %d, %q; a deferred #5 still stops the cut as waiting", code, stdout)
// #6 and #7 are still only on the batch, which fails the cut; #5 does not.
if code, stdout, _ := f.exec(gh, "check", "--line", "release-1.1", "--cut"); code != 1 || strings.Contains(stdout, "#5 changes") || !strings.Contains(stdout, "#6 is only on open batch PR #90") {
t.Errorf("check --cut = %d, %q; want 1 for #6 and #7 alone, a deferred #5 no longer waiting", code, stdout)
}
if code, _, stderr := f.exec(gh, "check", "--line", "release-1.1"); code != 0 || !strings.Contains(stderr, "deferred on #90 to the next batch: https://github.com/cozystack/cozystack/pull/5 ") {
t.Errorf("check = %d, want 0 with a warning about #5:\n%s", code, stderr)
}
}
+1 -1
View File
@@ -85,7 +85,7 @@ A run by hand trusts the open batch pull request the scheduled run opened as `co
A deferred conflict waits for the next batch. To land it sooner, pick the original's commits onto the batch head with `git cherry-pick -x`, resolving each conflict, and join them to the batch head with a merge commit that names the original; an original squash-merged into `main` is one pick of its commit there, with no merge. Check that merge or that pick with `./backport-batch fold --line <line> --original <number> --commit <sha>`, and push it onto the batch branch, as [resolving a conflict](./backport-tasks/resolve-conflict.md) describes. CI runs `backport-batch verify` on every push to a batch pull request, which holds every original on it to the same shape. When CI on the batch pull request fails, the original it comes from is taken out with `./backport-batch run --line <line> --defer <number> --reason <why> --push` and goes to the next batch, as [attributing a failed batch CI](./backport-tasks/failed-ci.md) describes; the rest of the batch merges without it.
Before a patch release or an rc is cut from a line, `./backport-batch check --line <line> --cut` has to exit 0. A cut tags the line, so the check fails while a pull request the rules take is not on the line, including one that is only on the open batch pull request: merge the batch pull request first. It prints the URLs, and prints what the batch deferred as warnings, since deferring is a maintainer's call. Without `--cut`, as the scheduled run uses it, a change on the open batch pull request counts as carried.
Before a patch release or an rc is cut from a line, `./backport-batch check --line <line> --cut` has to exit 0. A cut tags the line, so the check fails while a pull request the rules take is not on the line, including one that is only on the open batch pull request: merge the batch pull request first. It prints the URLs, and prints what the batch deferred as warnings, since deferring is a maintainer's call; so is a pull request labelled `backport/skip` that the open batch still carries. An original that waits on the batch for a maintainer fails the cut, also when it is all the batch has left. A cut that passes says so. Without `--cut`, as the scheduled run uses it, a change on the open batch pull request counts as carried.
### Changes to workflows