From 4e9b0764085436414e6f9882791929dc85d749fb Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 28 Oct 2026 10:43:35 +0100 Subject: [PATCH] Checker: say loudly that a pull request cannot be judged when its repository has no trunk head (issue 1422) A repository never merged through the gate has no entry in source.main, so its pull requests were dropped without a log line, a verdict or a status, and checker_status counted them as new. Now the verdict says it cannot be judged and why, the forge status is error with those words, checker_status names it under cannot_be_judged, the log says it once per head, and the heads are judged whole once the trunk head is known. Quick fix: this. Thorough fix (the gate seeding source.main from the pull message's base head) is still to do: it needs mesh-sdk, the forge bridge and the gate. --- .../internal/check/case.go | 38 ++++++++- .../internal/check/checker.go | 85 ++++++++++++++++++- .../internal/check/checker_test.go | 74 +++++++++++++++- .../internal/check/read.go | 22 ++++- 4 files changed, 209 insertions(+), 10 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/case.go b/modules/mesh-delivery-checker/internal/check/case.go index 7252046..2f2d0ff 100644 --- a/modules/mesh-delivery-checker/internal/check/case.go +++ b/modules/mesh-delivery-checker/internal/check/case.go @@ -115,7 +115,7 @@ func CaseOf(c delivery.Candidate, trunk map[string]string) *Case { for _, p := range c.Pulls { switch head, known := trunk[p.Repository]; { case !known || head == "": - cs.Left = append(cs.Left, fmt.Sprintf("%s#%d: the trunk head of %s is not known", p.Repository, p.Number, p.Repository)) + cs.Left = append(cs.Left, fmt.Sprintf("%s#%d: %s", p.Repository, p.Number, UntrunkedWordsOf([]string{p.Repository}))) default: cs.Pulls = append(cs.Pulls, Pull{Repository: p.Repository, Number: p.Number, Head: p.Head.Head, Trunk: head, Modules: p.Modules, Fps: p.Fps}) cs.Trunk[p.Repository] = head @@ -133,6 +133,42 @@ func CaseOf(c delivery.Candidate, trunk map[string]string) *Case { return cs } +// UntrunkedWords open the words of a verdict, and of a status, of a pull request that cannot be judged because the gate's +// source.main names no trunk head of its repository (issue 1422): never a failure of the change, and judged whole once the +// trunk head is known. It is said loudly, never left silently "new". +const UntrunkedWords = "cannot be judged: the trunk head of " + +// UntrunkedWordsOf are those words for the repositories whose trunk head is not known, in order, each named once. +func UntrunkedWordsOf(repos []string) string { + rs := slices.Clone(repos) + slices.Sort(rs) + rs = slices.Compact(rs) + return UntrunkedWords + strings.Join(rs, ", ") + " is not known: the gate's source.main names none, so there is no commit to merge it onto" +} + +// IsUntrunked says whether a verdict says its heads cannot be judged for want of a trunk head (UntrunkedWords). +func IsUntrunked(v delivery.Verdict) bool { + return v.State == delivery.VerdictFail && v.Failed.Machine == "checker" && strings.HasPrefix(v.Failed.Words, UntrunkedWords) +} + +// leftOf are the pull requests of a candidate whose repository has no trunk head known, with Trunk empty, in repository and +// number order; nil when every repository's is known. +func leftOf(c delivery.Candidate, trunk map[string]string) []Pull { + var out []Pull + for _, p := range c.Pulls { + if trunk[p.Repository] == "" { + out = append(out, Pull{Repository: p.Repository, Number: p.Number, Head: p.Head.Head, Modules: p.Modules, Fps: p.Fps}) + } + } + sort.SliceStable(out, func(i, j int) bool { + if out[i].Repository != out[j].Repository { + return out[i].Repository < out[j].Repository + } + return out[i].Number < out[j].Number + }) + return out +} + // GateHeads is the hash the gate matches the members an Approve covers by (mesh-catalog mesh-delivery-gate onestatus.go // headsOf): delivery.HeadsHash over every pull request of the candidate, hq's included, as // `#=`. A lone pull request is a group of one. Empty for a candidate diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 940f434..3e5b1da 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -295,9 +295,13 @@ func (ch *Checker) heads() map[string]string { } func (ch *Checker) consider(ctx context.Context, key string, c delivery.Candidate) { - cs := CaseOf(c, ch.heads()) + heads := ch.heads() + cs := CaseOf(c, heads) + if left := leftOf(c, heads); len(left) > 0 { + ch.untrunked(ctx, c, left, cs == nil) + } if cs == nil { - return // nothing to judge: no trunk known + return // nothing to judge: no trunk known, which untrunked said } cs.Queued = ch.now() ch.mu.Lock() @@ -351,6 +355,79 @@ func (ch *Checker) consider(ctx context.Context, key string, c delivery.Candidat }() } +// untrunked says, loudly, that pull requests of a candidate cannot be judged because the gate's source.main names no trunk +// head of their repository (issue 1422): each gets the status error with UntrunkedWords, and when none of the candidate is +// judged (whole) its verdict key holds a verdict that says so, which checker_verdicts and checker_status show. It is logged +// once per head: when a status or the verdict is written, never at a look that finds them already said. A verdict of these +// heads that says anything else is left. Once the trunk head is known the verdict is stale (it names no trunk) and the heads +// are judged whole (prepare, claim). +func (ch *Checker) untrunked(ctx context.Context, c delivery.Candidate, left []Pull, whole bool) { + key, err := delivery.CheckerVerdictKey(c.Group, c.HeadsHash) + if err != nil { + return + } + repos := make([]string, 0, len(left)) + for _, p := range left { + repos = append(repos, p.Repository) + } + words := UntrunkedWordsOf(repos) + said := false + if whole { + v := delivery.Verdict{Group: c.Group, Heads: GateHeads(c), Trunk: map[string]string{}, State: delivery.VerdictFail, + Failed: delivery.FailedFirst{Machine: "checker", Words: words}} + data, err := delivery.EncodeVerdict(v) + if err != nil { + ch.logf("%s: %s, and the verdict cannot say so: %v", key, words, err) + return + } + wrote, err := putOnce(ctx, ch.Verdicts, key, data, func(cur []byte) bool { + held, err := delivery.DecodeVerdict(cur) + // The same words stand; a verdict that says anything else (a check that ran, one running) is not this one's to replace. + return err != nil || !IsUntrunked(held) || held.Failed.Words == words + }) + if err != nil { + ch.logf("%s: %s, and the verdict cannot say so: %v", key, words, err) + } + said = said || wrote + } + for _, p := range left { + okey, err := delivery.CheckerOwedKey(p.Repository, p.Number, p.Head) + if err != nil { + ch.logf("%s: %v", key, err) + continue + } + o := delivery.Owed{Act: delivery.OwedStatus, Context: delivery.StatusContextCheck, State: "error", + Target: strings.TrimSuffix(ch.Link, "/") + "/" + key, Text: clipLine(words, MaxLine)} + data, err := delivery.EncodeCheckerOwed(o) + if err != nil { + ch.logf("%s: the status of %s cannot be written: %v", key, okey, err) + continue + } + wrote, err := putOnce(ctx, ch.Owed, okey, data, func(cur []byte) bool { + held, err := delivery.DecodeCheckerOwed(cur) + o.V = delivery.Version + return err == nil && delivery.SameOwed(held, o) + }) + if err != nil { + ch.logf("%s: the status of %s cannot be written: %v", key, okey, err) + continue + } + if wrote { + said = true + if ch.Announce != nil { + ch.Announce(ctx, EventOwedWritten, okey) + } + } + } + if said { + names := make([]string, 0, len(left)) + for _, p := range left { + names = append(names, fmt.Sprintf("%s#%d@%s", p.Repository, p.Number, short12(p.Head))) + } + ch.logf("%s: %s %s; the status says so (error), and they are judged once the trunk head is known", key, strings.Join(names, ", "), words) + } +} + // The kinds of a check (ADR 0536 §4): a head never judged and a head asked again after a failure of the nodes are asked on // the build seats' check stream; a carry, a verdict moved to a newer trunk or sibling commit by its package digests (ADR 0548 // §5), on the recheck stream, pulled after. @@ -380,8 +457,8 @@ func (ch *Checker) prepare(ctx context.Context, cs *Case) { return case IsConflictFailure(v): return // a head git refused: its next check is whole (ADR 0548 §7) - case IsEnded(v): - return // ended unjudged when its heads went (issue 1890): held again, they are checked whole + case IsEnded(v), IsUntrunked(v): + return // ended unjudged when its heads went (issue 1890), or never judged for want of a trunk head (issue 1422): checked whole } cs.Kind = KindTrunk if v.State == delivery.VerdictPass && ch.stale(v, cs) { diff --git a/modules/mesh-delivery-checker/internal/check/checker_test.go b/modules/mesh-delivery-checker/internal/check/checker_test.go index bc5820b..dd89894 100644 --- a/modules/mesh-delivery-checker/internal/check/checker_test.go +++ b/modules/mesh-delivery-checker/internal/check/checker_test.go @@ -239,14 +239,80 @@ func TestAnUnheardForgeFailsWithTheReasonAndIsJudgedAgainWhenTheNodesChange(t *t } } -func TestARepositoryWithNoTrunkGetsNoVerdict(t *testing.T) { // N6/AC6, ADR 0484 clause 11 +// Issue 1422: a pull request of a repository the gate's source.main names no trunk head of (one never merged through the +// gate) is never dropped silently. It is not judged, its verdict says it cannot be judged and why, its status is error with +// those words, checker_status names it, and the log says it once per head. Once its trunk head is known it is judged whole; +// in a group, the members whose trunk is known are judged and the others' status says why they are not. +func TestARepositoryWithNoTrunkCannotBeJudgedLoudlyUntilItsTrunkIsKnown(t *testing.T) { // N6/AC6, ADR 0484 clause 11, 1422 ctx, j := context.Background(), passJudge() r := newRig(t, j) - r.ch.Candidate(ctx, "feat/y", candidateOf(t, "feat/y", pullOf("novox/unknown", 4, head1))) + var logged []string + r.ch.Logf = func(f string, a ...any) { logged = append(logged, fmt.Sprintf(f, a...)) } + const unknown = "novox/unknown" + cand := candidateOf(t, "feat/y", pullOf(unknown, 4, head1)) + key := keyOf(t, "feat/y", cand) + r.ch.Candidate(ctx, "feat/y", cand) r.ch.Wait() - if j.count() != 0 || len(r.verdicts.vals) != 0 || len(r.owed.vals) != 0 { - t.Errorf("judged %d times, %d verdicts, %d statuses, want none", j.count(), len(r.verdicts.vals), len(r.owed.vals)) + r.ch.Reconsider(ctx) // looked at again: nothing is written or said twice + r.ch.NodesChanged(ctx) + r.ch.Wait() + v := r.verdict(t, key) + if j.count() != 0 || !IsUntrunked(v) || !strings.Contains(v.Failed.Words, "the trunk head of "+unknown+" is not known") { + t.Fatalf("judged %d times, verdict %+v, want none judged and a verdict that says it cannot be judged", j.count(), v) } + if s := owedState(t, r, unknown, 4, head1); s != "error" { + t.Errorf("%s#4 owes %q, want error", unknown, s) + } + if n := countWith(logged, "cannot be judged"); n != 1 { + t.Errorf("said %d times in the log, want once for the head: %q", n, logged) + } + reader := func() Reader { + ckey, _ := delivery.CandidatesKey("feat/y") + return Reader{CandidateState: fakeState{ckey: cand}.read(), VerdictState: fakeState(r.verdicts.vals).read(), + Trunks: func() (map[string]string, error) { return r.trunks.Heads(), nil }} + } + st, err := reader().Status() + if err != nil { + t.Fatal(err) + } + if why := st.CannotBeJudged["feat/y"]; !strings.Contains(why, unknown) || len(st.Waiting[KindNew]) != 0 || len(st.Unjudged) != 0 { + t.Errorf("cannot_be_judged %v, waiting %v, unjudged %v: want feat/y named as cannot be judged, not as new", st.CannotBeJudged, st.Waiting, st.Unjudged) + } + if rows, _, err := reader().Verdicts("feat/y"); err != nil || len(rows) != 1 || !strings.Contains(rows[0].Reports[unknown+"#4"], "cannot be judged") { + t.Errorf("checker_verdicts %+v (%v), want the report of %s#4 to say it cannot be judged", rows, err, unknown) + } + + // Its trunk head becomes known: the heads are judged whole, the same key rewritten, the status success. + r.setTrunks(t, map[string]string{repoA: trunkA, repoSDK: trunkS, unknown: trunkA2}) + r.ch.TrunkMoved(ctx) + r.ch.Wait() + if v := r.verdict(t, key); j.count() != 1 || v.State != delivery.VerdictPass || v.Trunk[unknown] != trunkA2 { + t.Errorf("judged %d times, verdict %+v, want one pass on %s", j.count(), v, trunkA2) + } + if s := owedState(t, r, unknown, 4, head1); s != "success" { + t.Errorf("%s#4 owes %q once its trunk is known, want success", unknown, s) + } + + // A group with one member of a repository whose trunk is not known: the others are judged, its status says why it is not. + group := candidateOf(t, "feat/z", pullOf(repoA, 9, head2), pullOf("novox/other", 2, head2)) + r.ch.Candidate(ctx, "feat/z", group) + r.ch.Wait() + if s := owedState(t, r, repoA, 9, head2); s != "success" { + t.Errorf("%s#9 owes %q, want success", repoA, s) + } + if s := owedState(t, r, "novox/other", 2, head2); s != "error" { + t.Errorf("novox/other#2 owes %q, want error", s) + } +} + +func countWith(lines []string, words string) int { + n := 0 + for _, l := range lines { + if strings.Contains(l, words) { + n++ + } + } + return n } // 1887/A10, ADR 0527 §2: hq is judged as any repository, its checks Go tests, and its pull request gets a verdict and a diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index 28ed623..cbe4581 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -86,6 +86,11 @@ func (r Reader) read(key string, raw json.RawMessage) (VerdictRead, error) { cs := CaseOf(c, v.Trunk) out.Reports = map[string]string{} if cs == nil { + if IsUntrunked(v) { + for _, p := range leftOf(c, v.Trunk) { + out.Reports[pullKey(p)] = v.Failed.Words + } + } return out, nil } for _, p := range cs.Pulls { @@ -323,6 +328,9 @@ type Status struct { // HeldByTurn are the candidates whose check an approved merge's turn holds off (ADR 0548 §3), by group: the repository and // the group of the merge that waits. Their check is claimed when that turn goes. HeldByTurn map[string]string `json:"held_by_turn"` + // CannotBeJudged are the candidates no pull request of which can be judged, by group, with why: the gate's source.main + // names no trunk head of their repositories (issue 1422). They are judged once it does; until then their status is error. + CannotBeJudged map[string]string `json:"cannot_be_judged"` // DeadLetters says whether the checker hears the asks the build seats give up on (issue 734): deaf, a try or check // whose ask is given up waits until its bound. DeadLetters DeadLettersHeard `json:"dead_letters"` @@ -452,7 +460,7 @@ func (r Reader) Status() (Status, error) { } st.Unjudged = []string{} st.Waiting = map[string][]string{KindNew: {}, KindNodes: {}, KindTrunk: {}} - st.HeldByTurn = map[string]string{} + st.HeldByTurn, st.CannotBeJudged = map[string]string{}, map[string]string{} turns := r.turns(&st) var heads map[string]string if r.Trunks == nil { @@ -503,6 +511,15 @@ func (r Reader) Status() (Status, error) { continue } c, raw := row.c, row.raw + if heads != nil && CaseOf(c, heads) == nil { + // Nothing of it can be judged now, whatever its verdict says (issue 1422): never counted as waiting. + var repos []string + for _, p := range leftOf(c, heads) { + repos = append(repos, p.Repository) + } + st.CannotBeJudged[c.Group] = UntrunkedWordsOf(repos) + continue + } if len(raw) == 0 { st.Unjudged = append(st.Unjudged, c.Group) st.Waiting[KindNew] = append(st.Waiting[KindNew], c.Group) @@ -516,6 +533,9 @@ func (r Reader) Status() (Status, error) { case err != nil || v.State == delivery.VerdictRunning: case IsNodesFailure(v): st.Waiting[KindNodes] = append(st.Waiting[KindNodes], c.Group) + case IsUntrunked(v): + // Its trunk head is known now: it waits for its whole check. + st.Waiting[KindNew] = append(st.Waiting[KindNew], c.Group) case heads != nil: if cs := CaseOf(c, heads); cs != nil && (!sameTrunk(v.Trunk, cs.Trunk) || besideMoved(v.Beside, besideAt(heads, r.Runs))) { st.Waiting[KindTrunk] = append(st.Waiting[KindTrunk], c.Group)