From ce054dc8dd4d99b68685c52e59c49ab0b72bcf09 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 30 Oct 2026 18:12:33 +0100 Subject: [PATCH] Judge nothing and owe no status for a candidate outside the mesh (issue 774) The forge carries the operator's own repositories (jschoubben/hal, jschoubben/novadock) beside the mesh's; the indexer writes their pull requests as candidates, the gate never names their trunk, so the checker said each "cannot be judged" and owed the status error on mesh/check, which the forge module wrote into repositories the mesh's forge account may not write: 403, 119 times each (sweep 2026-10-30 item 7). Quick fix: name the two repositories as not the mesh's in the checker. Thorough fix, chosen because any repository on the forge can meet it: a candidate every pull request of which is of a repository the gate's source.main names no trunk of and touches no module is outside the mesh. The checker writes it no verdict and owes it no status, takes away a status owed before, says it once in the log, and lists it in checker_status as outside_the_mesh. A new mesh repository's first pull request adds a module, so it is still said untrunked and owed error (issue 1422). Nothing is outside before the gate's source is read. --- .../internal/check/case.go | 25 +++++++++ .../internal/check/checker.go | 40 +++++++++++++++ .../internal/check/checker_test.go | 51 ++++++++++++++++++- .../internal/check/read.go | 8 ++- .../internal/check/store.go | 5 ++ 5 files changed, 126 insertions(+), 3 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/case.go b/modules/mesh-delivery-checker/internal/check/case.go index 2f2d0ff..3f80136 100644 --- a/modules/mesh-delivery-checker/internal/check/case.go +++ b/modules/mesh-delivery-checker/internal/check/case.go @@ -151,6 +151,31 @@ func IsUntrunked(v delivery.Verdict) bool { return v.State == delivery.VerdictFail && v.Failed.Machine == "checker" && strings.HasPrefix(v.Failed.Words, UntrunkedWords) } +// **A candidate outside the mesh is not the checker's** (sweep 2026-10-30 item 7): the forge carries repositories the mesh +// does not deliver (an operator's own), whose pull requests the indexer writes as candidates like any. Their trunk the +// gate never names, so each was judged "cannot be judged" and owed the status error on mesh/check, which the forge +// module wrote 119 times per head into a repository the mesh's forge account may not write (403). A candidate every pull +// request of which is of a repository the gate's source.main names no trunk head of, and touches no module, is outside +// the mesh: judged nothing, owed nothing on the forge, said once in the log and in checker_status (outside_the_mesh). A +// new repository of the mesh is not outside: its first pull request adds a module, and is said untrunked (issue 1422). + +// OutsideWords say why a candidate is outside the mesh. +const OutsideWords = "outside the mesh: the gate's source.main names no trunk head of its repositories and it touches no module, so the checker judges nothing and owes no status on the forge" + +// Outside says every pull request of c is of a repository trunk names no head of and touches no module. Never with +// trunk unknown (nil): before the gate's source is read nothing is outside. +func Outside(c delivery.Candidate, trunk map[string]string) bool { + if trunk == nil || len(c.Pulls) == 0 { + return false + } + for _, p := range c.Pulls { + if trunk[p.Repository] != "" || len(p.Modules) > 0 || len(p.Fps) > 0 { + return false + } + } + return true +} + // 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 { diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 2456ab9..3fbffdd 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -104,6 +104,8 @@ type Checker struct { held map[string]string // by verdict key: the turn that holds its check off, as last said live map[string]liveCheck // by verdict key: the check running in this process, ended when its heads go settling map[string]*window // by candidates key: the window its heads settle in + // outsideSaid are the candidates outside the mesh said in the log, by group and heads. + outsideSaid map[string]bool } // liveCheck is a check running in this process: the candidates key of its group and the end of its run. @@ -160,6 +162,7 @@ func (ch *Checker) init() { ch.known, ch.inflight, ch.again, ch.tried = map[string]delivery.Candidate{}, map[string]bool{}, map[string]bool{}, map[string]bool{} ch.nodesFail, ch.held = map[string]uint64{}, map[string]string{} ch.live, ch.settling = map[string]liveCheck{}, map[string]*window{} + ch.outsideSaid = map[string]bool{} }) } @@ -327,6 +330,10 @@ func (ch *Checker) heads() map[string]string { func (ch *Checker) consider(ctx context.Context, key string, c delivery.Candidate) { heads := ch.heads() + if Outside(c, heads) { + ch.outside(ctx, c) + return + } if left := leftOf(c, heads); len(left) > 0 { // A pull request the checker cannot claim (its repository's trunk head is not known) leaves its whole group // unjudged, said in a failure verdict (issue 1422, issue 1733): the gate asks the Approve only on a verdict, and a @@ -464,6 +471,39 @@ func (ch *Checker) untrunked(ctx context.Context, c delivery.Candidate, left []P } } +// outside takes a candidate outside the mesh (case.go Outside): no verdict, and every status owed for its pull requests +// taken away, so the forge module stops writing one into a repository the mesh's forge account may not write. Said once +// per heads in the log. +func (ch *Checker) outside(ctx context.Context, c delivery.Candidate) { + var names []string + for _, p := range c.Pulls { + names = append(names, fmt.Sprintf("%s#%d@%s", p.Repository, p.Number, short12(p.Head.Head))) + key, err := delivery.CheckerOwedKey(p.Repository, p.Number, p.Head.Head) + if err != nil { + continue + } + _, _, found, err := ch.Owed.Get(ctx, key) + if err != nil || !found { + continue + } + d, ok := ch.Owed.(Deleter) + if !ok { + continue + } + if err := d.Delete(ctx, key); err != nil { + ch.logf("%s: the status owed of a pull request outside the mesh could not be taken away: %v", key, err) + } + } + said := c.Group + "@" + c.HeadsHash + ch.mu.Lock() + first := !ch.outsideSaid[said] + ch.outsideSaid[said] = true + ch.mu.Unlock() + if first { + ch.logf("candidates of %s: %s %s", c.Group, strings.Join(names, ", "), OutsideWords) + } +} + // 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. diff --git a/modules/mesh-delivery-checker/internal/check/checker_test.go b/modules/mesh-delivery-checker/internal/check/checker_test.go index 888cf41..20683ca 100644 --- a/modules/mesh-delivery-checker/internal/check/checker_test.go +++ b/modules/mesh-delivery-checker/internal/check/checker_test.go @@ -48,6 +48,13 @@ func pullOf(repo string, number int, head string) delivery.CandidatePull { return delivery.CandidatePull{Head: delivery.Head{Repository: repo, Number: number, Head: head}, Modules: []string{}} } +// newRepoPullOf is a pull request of a new repository of the mesh: its trunk not named yet, its first module added. +func newRepoPullOf(repo string, number int, head string) delivery.CandidatePull { + p := pullOf(repo, number, head) + p.Modules = []string{"mod-u"} + return p +} + // judgeFunc is a Judge written as a function; it counts its calls. type judgeFunc struct { mu sync.Mutex @@ -249,7 +256,7 @@ func TestARepositoryWithNoTrunkCannotBeJudgedLoudlyUntilItsTrunkIsKnown(t *testi 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)) + cand := candidateOf(t, "feat/y", newRepoPullOf(unknown, 4, head1)) key := keyOf(t, "feat/y", cand) r.ch.Candidate(ctx, "feat/y", cand) r.ch.Wait() @@ -305,7 +312,7 @@ func TestARepositoryWithNoTrunkCannotBeJudgedLoudlyUntilItsTrunkIsKnown(t *testi want string // the verdict's state, and for a failure the repository it names }{ {name: "a member the checker cannot claim", pulls: []delivery.CandidatePull{pullOf(repoA, 9, head2), pullOf("novox/other", 2, head2)}, want: "novox/other"}, - {name: "a running verdict left for heads that cannot be judged", pulls: []delivery.CandidatePull{pullOf(unknown, 5, head2)}, running: true, want: unknown}, + {name: "a running verdict left for heads that cannot be judged", pulls: []delivery.CandidatePull{newRepoPullOf(unknown, 5, head2)}, running: true, want: unknown}, {name: "a verdict key deleted", pulls: []delivery.CandidatePull{pullOf(repoA, 5, head2)}, deleted: true, judged: 2, want: delivery.VerdictPass}, } { t.Run(row.name, func(t *testing.T) { @@ -566,3 +573,43 @@ func TestTheForgeBridgeIsToldOnceForEachChangeOfAStatus(t *testing.T) { // 1683 t.Errorf("told %v after a check that changed nothing", told) } } + +// Sweep 2026-10-30 item 7: a pull request of a repository the mesh does not deliver (an operator's own, on the same +// forge) touches no module and its trunk the gate never names. It is outside the mesh: no verdict, no status owed (one +// owed before is taken away, so the forge module stops writing it into a repository the mesh may not write), said once. +func TestACandidateOutsideTheMeshIsJudgedNothingAndOwesNothing(t *testing.T) { + ctx, j := context.Background(), passJudge() + r := newRig(t, j) + var logged []string + r.ch.Logf = func(f string, a ...any) { logged = append(logged, fmt.Sprintf(f, a...)) } + const theirs = "someone/own-app" + okey, _ := delivery.CheckerOwedKey(theirs, 55, head1) + before, _ := delivery.EncodeCheckerOwed(delivery.Owed{Act: delivery.OwedStatus, Context: delivery.StatusContextCheck, State: "error", Text: "cannot be judged"}) + if _, err := r.owed.Create(ctx, okey, before); err != nil { + t.Fatal(err) + } + cand := candidateOf(t, "feat/theirs", pullOf(theirs, 55, head1)) + key := keyOf(t, "feat/theirs", cand) + r.ch.Candidate(ctx, "feat/theirs", cand) + r.ch.Wait() + r.ch.Reconsider(ctx) + r.ch.Wait() + if _, _, found, _ := r.verdicts.Get(ctx, key); found || j.count() != 0 { + t.Errorf("a candidate outside the mesh was judged (%d) or given a verdict (%v)", j.count(), found) + } + if s := owedState(t, r, theirs, 55, head1); s != "" { + t.Errorf("%s#55 still owes %q on the forge", theirs, s) + } + if n := countWith(logged, "outside the mesh"); n != 1 { + t.Errorf("said %d times in the log, want once: %q", n, logged) + } + ckey, _ := delivery.CandidatesKey("feat/theirs") + st, err := Reader{CandidateState: fakeState{ckey: cand}.read(), VerdictState: fakeState(r.verdicts.vals).read(), + Trunks: func() (map[string]string, error) { return r.trunks.Heads(), nil }}.Status() + if err != nil { + t.Fatal(err) + } + if st.Outside["feat/theirs"] == "" || len(st.CannotBeJudged) != 0 || len(st.Unjudged) != 0 { + t.Errorf("status: outside %v, cannot be judged %v, unjudged %v", st.Outside, st.CannotBeJudged, st.Unjudged) + } +} diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index 4d22745..30cb4d5 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -335,6 +335,8 @@ type Status struct { // 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"` + // Outside are the candidates outside the mesh, by group, with why (case.go Outside): judged nothing, owed nothing. + Outside map[string]string `json:"outside_the_mesh"` // Lost are the candidates whose verdict is running while no step of theirs can still run and their newest step ended, by // group, with that step: nothing will judge them (the lost check of 2026-10-28); checker_ask_again asks one again. Lost map[string]string `json:"lost"` @@ -415,7 +417,7 @@ func (r Reader) Status() (Status, error) { } st.Unjudged = []string{} st.Waiting = map[string][]string{KindNew: {}, KindNodes: {}, KindTrunk: {}} - st.HeldByTurn, st.CannotBeJudged = map[string]string{}, map[string]string{} + st.HeldByTurn, st.CannotBeJudged, st.Outside = map[string]string{}, map[string]string{}, map[string]string{} turns := r.turns(&st) var heads map[string]string if r.Trunks == nil { @@ -466,6 +468,10 @@ func (r Reader) Status() (Status, error) { continue } c, raw := row.c, row.raw + if Outside(c, heads) { + st.Outside[c.Group] = OutsideWords + continue + } if heads != nil && len(leftOf(c, heads)) > 0 { // Nothing of it can be judged now, whatever its verdict says (issue 1422): never counted as waiting. var repos []string diff --git a/modules/mesh-delivery-checker/internal/check/store.go b/modules/mesh-delivery-checker/internal/check/store.go index 4df618e..6e7dbe2 100644 --- a/modules/mesh-delivery-checker/internal/check/store.go +++ b/modules/mesh-delivery-checker/internal/check/store.go @@ -17,3 +17,8 @@ type KV interface { // Keys are every key of the bucket. Keys(ctx context.Context) ([]string, error) } + +// Deleter is a KV that can take a key away; a KV that cannot leaves it. +type Deleter interface { + Delete(ctx context.Context, key string) error +}