From c66fe11e1693267570c12982c3e4bcc1db755be7 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 11:29:25 +0100 Subject: [PATCH 1/9] =?UTF-8?q?checker:=20claim=20no=20check=20of=20a=20re?= =?UTF-8?q?pository=20an=20approved=20merge=20holds=20a=20turn=20on,=20and?= =?UTF-8?q?=20claim=20it=20when=20the=20turn=20goes=20(ADR=200548=20=C2=A7?= =?UTF-8?q?3;=20issue=20568=20D1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-delivery-checker-tools/main.go | 7 +- .../cmd/mesh-delivery-checker/main.go | 19 +++ .../internal/check/adr548_test.go | 114 ++++++++++++++++++ .../internal/check/checker.go | 57 ++++++++- .../internal/check/read.go | 53 +++++++- .../internal/check/read_test.go | 12 ++ .../internal/check/turns.go | 78 ++++++++++++ .../mesh-delivery-checker/manifest_test.go | 2 +- modules/mesh-delivery-checker/module.json | 1 + 9 files changed, 336 insertions(+), 7 deletions(-) create mode 100644 modules/mesh-delivery-checker/internal/check/adr548_test.go create mode 100644 modules/mesh-delivery-checker/internal/check/turns.go diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go index da0aabe..4d5aa37 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go @@ -174,7 +174,9 @@ func Tools(r check.Reader) []stdio.Tool { "and the candidates whose check is owed by kind (waiting_by_kind: new, nodes, and trunk, a re-judge pulled after the other two). A key a " + "build module left running (started past the step bound, or its build module not running on its node) is listed apart under " + "left_running with why, and not counted. Nothing holds the checker and it has no capacity of its own: a check is an ask on the " + - "build seats' streams, whose backlog the delivery view shows (novox/hq ADR 0525). Read-only.", + "build seats' streams, whose backlog the delivery view shows (novox/hq ADR 0525). held_by_turn names the candidates whose check an approved " + + "merge's turn holds off (novox/hq ADR 0548 §3), with the repository and the merge that waits; the gate's turns tool reads every turn and its " + + "hand-now tool steers one. Read-only.", Input: map[string]any{"type": "object", "properties": map[string]any{}}, Run: func(map[string]any) (any, error) { return r.Status() }}, } @@ -193,7 +195,8 @@ func main() { r := check.Reader{VerdictState: read(stdio.State("verdicts")), KeptState: read(stdio.State("kept")), CandidateState: read(stdio.State("mesh-delivery-indexer.candidates")), PreviewState: read(stdio.State("previews")), StepStates: map[string]check.StateRead{"mesh-build-go": read(stdio.State("mesh-build-go.steps")), "mesh-build-node": read(stdio.State("mesh-build-node.steps"))}, - HaveState: read(stdio.State("node-engine.have")), StepBound: stepBound()} + HaveState: read(stdio.State("node-engine.have")), StepBound: stepBound(), + TurnState: read(stdio.State("mesh-delivery-gate.turns"))} tries := stdio.State(check.TriesState) source := stdio.State("mesh-delivery-gate.source") trunks := func() (map[string]string, error) { diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go index c46ec4d..a8c0ddc 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go @@ -193,6 +193,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro tools := &buildrun.ToolchainSet{} presence := &buildrun.PresenceSet{} trunks := &check.Trunks{} + turns := &check.Turns{} // A check is one ask per pull request and build module on the build seats' check stream, pulled by a build node with a // free slot (novox/hq ADR 0525 §1, §3): the checker chooses no node and starts no step. asks := buildrun.JetStreamAsks{JS: js} @@ -200,6 +201,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro Verdicts: kvStore{buckets[delivery.CheckerVerdictsBucket]}, Owed: kvStore{buckets[delivery.CheckerOwedBucket]}, Trunks: trunks, + Turns: turns, Bound: checkBound, Link: link, Logf: logger.Printf, @@ -313,6 +315,23 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro }() } } + // The gate's merge turns (novox/hq ADR 0548 §3): no check is claimed of a repository an approved merge waits on, and the + // delete of its turn claims it. Read before the candidates, so no check starts past a turn written before this start. + // Where the gate's bucket is not there yet it is read as empty: no turn holds a check until the checker starts again. + if kv := process.OptionalBucket(ctx, js, delivery.TurnsBucket, "no merge turn holds a check until the checker starts again", logger.Printf); kv != nil { + tw, err := settle(ctx, kv, func(e jetstream.KeyValueEntry) { turns.Put(e.Key(), e.Value(), !opOf(e)) }) + if err != nil { + process.NotWatched(logger.Printf, delivery.TurnsBucket, err, "no merge turn holds a check until the checker starts again") + } else { + go func() { + errs <- follow(ctx, tw, func(e jetstream.KeyValueEntry) { + if turns.Put(e.Key(), e.Value(), !opOf(e)) { + ch.Reconsider(ctx) // a turn went or changed: a check it held is claimed now + } + }) + }() + } + } // The modules say which repository holds a TypeScript module, so the Node build module judges it too. mw, err := settle(ctx, buckets[delivery.ModulesBucket], func(e jetstream.KeyValueEntry) { repos.Put(ctx, e.Key(), e.Value(), !opOf(e)) }) if err != nil { diff --git a/modules/mesh-delivery-checker/internal/check/adr548_test.go b/modules/mesh-delivery-checker/internal/check/adr548_test.go new file mode 100644 index 0000000..8ff5cdf --- /dev/null +++ b/modules/mesh-delivery-checker/internal/check/adr548_test.go @@ -0,0 +1,114 @@ +package check + +import ( + "context" + "strings" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// One check per delivery (novox/hq ADR 0548): the checker's part. A merge turn holds new checks off its repository (§3); +// a trunk or sibling move never ends a running check and carries its verdict once it ended (§1, §5, §6); only a head git +// refused gets a whole check after a move (§7); the line of a carried verdict says so (§8). + +// turnOf is a turn's value on a repository for a delivery of the heads hash. +func turnOf(t testing.TB, repo, group, heads string) []byte { + t.Helper() + at := time.Date(2026, 10, 26, 9, 0, 0, 0, time.UTC) + data, err := delivery.EncodeTurn(delivery.Turn{Repository: repo, Waiting: []delivery.TurnWait{{Group: group, Heads: heads, Approved: at, Since: at}}}) + if err != nil { + t.Fatal(err) + } + return data +} + +func turnKey(t testing.TB, repo string) string { + t.Helper() + k, err := delivery.TurnsKey(repo) + if err != nil { + t.Fatal(err) + } + return k +} + +// waitFor polls a condition the checker's goroutines make true, failing after two seconds. +func waitFor(t testing.TB, what string, ok func() bool) { + t.Helper() + for deadline := time.Now().Add(2 * time.Second); !ok(); time.Sleep(time.Millisecond) { + if time.Now().After(deadline) { + t.Fatalf("%s did not happen", what) + } + } +} + +func TestOneCheckPerDelivery(t *testing.T) { + ctx := context.Background() + for name, run := range map[string]func(t *testing.T){ + // §3: no claim while a turn names one of its repositories; a claim on the turn's delete event. + "a turn on its repository holds a new check off until the turn goes": func(t *testing.T) { + j := passJudge() + r := newRig(t, j) + turns := &Turns{} + r.ch.Turns = turns + turns.Put(turnKey(t, repoA), turnOf(t, repoA, "fix/other", "0123456789ab"), false) + cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1)) + r.ch.Candidate(ctx, "feat%2Fx", cand) + r.ch.Wait() + if j.count() != 0 || len(r.verdicts.vals) != 0 { + t.Fatalf("judged %d times with a turn on %s, verdicts %v", j.count(), repoA, r.verdicts.vals) + } + if held := r.ch.Held()[keyOf(t, "feat/x", cand)]; !strings.Contains(held, repoA) || !strings.Contains(held, "fix/other") { + t.Errorf("the hold is said as %q, want the turn's repository and group", held) + } + if !turns.Put(turnKey(t, repoA), nil, true) { + t.Fatal("the turn's delete changed nothing") + } + r.ch.Reconsider(ctx) // what the watch of turns does on the delete + r.ch.Wait() + if j.count() != 1 || r.verdict(t, keyOf(t, "feat/x", cand)).State != delivery.VerdictPass { + t.Errorf("judged %d times after the turn went, want once", j.count()) + } + }, + "a turn on another repository, or of its own heads, holds nothing": func(t *testing.T) { + j := passJudge() + r := newRig(t, j) + turns := &Turns{} + r.ch.Turns = turns + cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1)) + c, _ := delivery.DecodeCandidate(cand) + turns.Put(turnKey(t, repoSDK), turnOf(t, repoSDK, "fix/other", "0123456789ab"), false) + turns.Put(turnKey(t, repoA), turnOf(t, repoA, "feat/x", GateHeads(c)), false) + r.ch.Candidate(ctx, "feat%2Fx", cand) + r.ch.Wait() + if j.count() != 1 { + t.Errorf("judged %d times, want once: the turns hold another repository and its own merge", j.count()) + } + }, + // §3, the race: a check claimed before the gate's turn is judged on the commit it read, never restarted. + "a check claimed before a turn was written runs to its verdict": func(t *testing.T) { + release, started := make(chan struct{}), make(chan struct{}) + j := &judgeFunc{f: func(context.Context, *Case, int) (Outcome, error) { + close(started) + <-release + return Outcome{Verdict: passing()}, nil + }} + r := newRig(t, j) + turns := &Turns{} + r.ch.Turns = turns + cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1)) + r.ch.Candidate(ctx, "feat%2Fx", cand) + <-started + turns.Put(turnKey(t, repoA), turnOf(t, repoA, "fix/other", "0123456789ab"), false) + r.ch.Reconsider(ctx) + close(release) + r.ch.Wait() + if j.count() != 1 || r.verdict(t, keyOf(t, "feat/x", cand)).State != delivery.VerdictPass { + t.Errorf("judged %d times; the running check is not ended by the turn", j.count()) + } + }, + } { + t.Run(name, run) + } +} diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index d116005..86e55bd 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -65,6 +65,11 @@ type Checker struct { Owed KV Judge Judge Trunks interface{ Heads() map[string]string } + // Turns are the gate's merge turns (ADR 0548 §3): no check is claimed of a repository a turn names, but the check of the + // waiting delivery's own heads; nil holds none. + Turns interface { + Holding(repositories []string, heads string) (string, delivery.TurnWait, bool) + } // Bound is how long one check may run; none when 0 (DefaultBound). Bound time.Duration // Link is the base of the address a status points at; the verdict key follows. @@ -83,12 +88,13 @@ type Checker struct { tried map[string]bool // by verdict key: looked at since this process started epoch uint64 // changes of the nodes seen since this process started nodesFail map[string]uint64 // by verdict key: the epoch a failure of the nodes was written at + held map[string]string // by verdict key: the turn that holds its check off, as last said } func (ch *Checker) init() { ch.once.Do(func() { ch.known, ch.inflight, ch.again, ch.tried = map[string]delivery.Candidate{}, map[string]bool{}, map[string]bool{}, map[string]bool{} - ch.nodesFail = map[string]uint64{} + ch.nodesFail, ch.held = map[string]uint64{}, map[string]string{} }) } @@ -265,6 +271,9 @@ func (ch *Checker) claim(ctx context.Context, cs *Case, first bool, epoch uint64 return nil, fmt.Errorf("%s: %w", cs.Key, err) } if !found { + if ch.heldByTurn(cs) { + return nil, nil + } rev, err := ch.Verdicts.Create(ctx, cs.Key, data) if errors.Is(err, ErrConflict) { return nil, nil // another checker made it first @@ -293,7 +302,7 @@ func (ch *Checker) claim(ctx context.Context, cs *Case, first bool, epoch uint64 ch.mu.Unlock() retake = !ok || epoch > at } - if !retake { + if !retake || ch.heldByTurn(cs) { return nil, nil } rev, err = ch.Verdicts.Update(ctx, cs.Key, data, rev) @@ -306,6 +315,50 @@ func (ch *Checker) claim(ctx context.Context, cs *Case, first bool, epoch uint64 return &claimed{rev}, nil } +// heldByTurn says whether a merge turn holds the case's check off (ADR 0548 §3): an approved merge waits on one of the +// repositories it merges heads into, and it is not the merge of the case's own heads. The check is claimed when the turn's +// key goes (Reconsider on the turns watch). Said once per turn in the log. A check already claimed is never held: one +// claimed in the moment before the gate wrote its turn is judged on the commit it read and carried once after. +func (ch *Checker) heldByTurn(cs *Case) bool { + if ch.Turns == nil { + return false + } + repos := make([]string, 0, len(cs.Trunk)) + for r := range cs.Trunk { + repos = append(repos, r) + } + repo, w, held := ch.Turns.Holding(repos, cs.Heads) + why := "" + if held { + why = repo + " " + w.Group + " " + w.Heads + } + ch.mu.Lock() + said := ch.held[cs.Key] + if why == "" { + delete(ch.held, cs.Key) + } else { + ch.held[cs.Key] = why + } + ch.mu.Unlock() + if held && said != why { + ch.logf("%s: not claimed: the approved merge of %s (heads %s) holds a turn on %s, so its check waits for that merge (ADR 0548 §3)", + cs.Key, w.Group, w.Heads, repo) + } + return held +} + +// Held is every check a merge turn holds off now, by verdict key, with the turn's repository, group and heads. +func (ch *Checker) Held() map[string]string { + ch.init() + ch.mu.Lock() + defer ch.mu.Unlock() + out := make(map[string]string, len(ch.held)) + for k, v := range ch.held { + out[k] = v + } + return out +} + // IsNodesFailure says whether a verdict failed because a part of the mesh the check reads was not heard (UnheardWords): // it is judged again when the nodes change. No check fails for want of a build node any more: its asks wait in the build // seats' streams (ADR 0525 §6, issue 1912). diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index f5953f9..6bfa883 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -39,7 +39,9 @@ type Reader struct { StepBound time.Duration // how long the checker waits for one step; DefaultStepBound when 0 // Trunks are the trunk heads of every repository (the gate's source); nil names no re-judge after a trunk move. Trunks func() (map[string]string, error) - Now func() time.Time + // TurnState is the gate's merge turns (ADR 0548 §3); without it no check is named as held by a turn. + TurnState StateRead + Now func() time.Time } // VerdictRead is a verdict as stored with the full report of the status of each pull request it judged (Report), when the @@ -312,7 +314,46 @@ type Status struct { // Waiting are the candidates whose check waits, by kind (ADR 0536 §4): new (no verdict for its heads) and nodes (failed // for the nodes, asked again when they change) start before trunk (judged on trunk heads that moved since). Waiting map[string][]string `json:"waiting_by_kind"` - Notes []string `json:"notes,omitempty"` + // 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"` + Notes []string `json:"notes,omitempty"` +} + +// turns reads the gate's merge turns, by repository; nil when they cannot be read, said in a note. +func (r Reader) turns(st *Status) *Turns { + if r.TurnState.Keys == nil || r.TurnState.Get == nil { + st.Notes = append(st.Notes, "the gate's merge turns are not readable here, so no check is named as held by one") + return nil + } + keys, err := r.TurnState.Keys() + if err != nil { + st.Notes = append(st.Notes, "the gate's merge turns cannot be listed, so no check is named as held by one: "+err.Error()) + return nil + } + t := &Turns{} + for _, k := range keys { + if raw, err := r.TurnState.Get(k); err == nil && len(raw) > 0 { + t.Put(k, raw, false) + } + } + return t +} + +// heldBy says the turn that holds a candidate's check off, or "". +func heldBy(t *Turns, c delivery.Candidate, heads map[string]string) string { + cs := CaseOf(c, heads) + if t == nil || cs == nil { + return "" + } + repos := make([]string, 0, len(cs.Trunk)) + for r := range cs.Trunk { + repos = append(repos, r) + } + if repo, w, ok := t.Holding(repos, cs.Heads); ok { + return repo + ": the approved merge of " + w.Group + " waits on it" + } + return "" } // Status reads the build modules' steps and the candidates against their verdicts. @@ -383,6 +424,8 @@ func (r Reader) Status() (Status, error) { } st.Unjudged = []string{} st.Waiting = map[string][]string{KindNew: {}, KindNodes: {}, KindTrunk: {}} + st.HeldByTurn = map[string]string{} + turns := r.turns(&st) var heads map[string]string if r.Trunks == nil { st.Notes = append(st.Notes, "the trunk heads are not readable here, so no check is named as waiting for a trunk move") @@ -435,6 +478,9 @@ func (r Reader) Status() (Status, error) { if len(raw) == 0 { st.Unjudged = append(st.Unjudged, c.Group) st.Waiting[KindNew] = append(st.Waiting[KindNew], c.Group) + if why := heldBy(turns, c, heads); why != "" { + st.HeldByTurn[c.Group] = why + } continue } v, err := delivery.DecodeVerdict(raw) @@ -445,6 +491,9 @@ func (r Reader) Status() (Status, error) { case heads != nil: if cs := CaseOf(c, heads); cs != nil && !sameTrunk(v.Trunk, cs.Trunk) { st.Waiting[KindTrunk] = append(st.Waiting[KindTrunk], c.Group) + if why := heldBy(turns, c, heads); why != "" { + st.HeldByTurn[c.Group] = why + } } } } diff --git a/modules/mesh-delivery-checker/internal/check/read_test.go b/modules/mesh-delivery-checker/internal/check/read_test.go index 4d620fd..0f113c1 100644 --- a/modules/mesh-delivery-checker/internal/check/read_test.go +++ b/modules/mesh-delivery-checker/internal/check/read_test.go @@ -234,6 +234,18 @@ func TestTheStatusCountsAnHQOnlyGroupAsUnjudged(t *testing.T) { if len(st.Unjudged) != 1 || st.Unjudged[0] != "pb-x" { t.Errorf("unjudged = %v, want pb-x", st.Unjudged) } + // ADR 0548 §3: a candidate whose check an approved merge's turn holds off is named with the turn. + held := candidateOf(t, "feat/x", pullOf(repoA, 7, head1)) + hkey, _ := delivery.CandidatesKey("feat/x") + st, err = Reader{CandidateState: fakeState{hkey: held}.read(), VerdictState: fakeState{}.read(), + Trunks: func() (map[string]string, error) { return map[string]string{repoA: trunkA}, nil }, + TurnState: fakeState{turnKey(t, repoA): turnOf(t, repoA, "fix/other", "0123456789ab")}.read()}.Status() + if err != nil { + t.Fatal(err) + } + if why := st.HeldByTurn["feat/x"]; !strings.Contains(why, repoA) || !strings.Contains(why, "fix/other") { + t.Errorf("held_by_turn = %v, want feat/x held by the merge of fix/other on %s", st.HeldByTurn, repoA) + } } // slowBucket wraps a state with a Get that takes delay and counts the values read, the lists and the most Gets in flight. diff --git a/modules/mesh-delivery-checker/internal/check/turns.go b/modules/mesh-delivery-checker/internal/check/turns.go new file mode 100644 index 0000000..4873e33 --- /dev/null +++ b/modules/mesh-delivery-checker/internal/check/turns.go @@ -0,0 +1,78 @@ +package check + +import ( + "sort" + "strings" + "sync" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// Turns holds the gate's merge turns (novox/hq ADR 0548 §3), by repository, as a watch of the gate's turns bucket puts them +// in: the checker claims no check of a repository a turn names, except the check of a waiting delivery's own heads, so an +// approved merge goes before a check not yet started. A bucket not there is read as empty. +type Turns struct { + mu sync.Mutex + byRepo map[string]delivery.Turn +} + +// Put takes a value of the turns bucket in, or its delete, and answers whether what the checker holds changed. A value that +// is not a turn is dropped as a delete would be: a turn the checker cannot read holds nothing. +func (t *Turns) Put(key string, value []byte, deleted bool) bool { + repository, err := delivery.ParseTurnsKey(key) + if err != nil { + return false + } + t.mu.Lock() + defer t.mu.Unlock() + if t.byRepo == nil { + t.byRepo = map[string]delivery.Turn{} + } + cur, had := t.byRepo[repository] + if !deleted { + if turn, err := delivery.DecodeTurn(value); err == nil { + t.byRepo[repository] = turn + return !had || !delivery.SameTurn(cur, turn) + } + } + delete(t.byRepo, repository) + return had +} + +// Holding names the first of the repositories (by name) a turn holds off the check of the heads, and the delivery that +// waits first in it; false when none does. +func (t *Turns) Holding(repositories []string, heads string) (string, delivery.TurnWait, bool) { + if t == nil { + return "", delivery.TurnWait{}, false + } + sorted := make([]string, 0, len(repositories)) + for _, r := range repositories { + sorted = append(sorted, strings.ToLower(r)) + } + sort.Strings(sorted) + t.mu.Lock() + defer t.mu.Unlock() + for _, r := range sorted { + turn, ok := t.byRepo[r] + if !ok || !delivery.TurnHolds(turn, heads) { + continue + } + for _, w := range turn.Waiting { + if w.Heads != heads { + return r, w, true + } + } + } + return "", delivery.TurnWait{}, false +} + +// All is every turn held now, by repository (a copy). +func (t *Turns) All() map[string]delivery.Turn { + t.mu.Lock() + defer t.mu.Unlock() + out := make(map[string]delivery.Turn, len(t.byRepo)) + for r, turn := range t.byRepo { + out[r] = turn + } + return out +} diff --git a/modules/mesh-delivery-checker/manifest_test.go b/modules/mesh-delivery-checker/manifest_test.go index 8df57af..07bb4aa 100644 --- a/modules/mesh-delivery-checker/manifest_test.go +++ b/modules/mesh-delivery-checker/manifest_test.go @@ -97,7 +97,7 @@ func TestManifestDeclarations(t *testing.T) { // 1678/AC1 t.Errorf("state = %+v, want %+v", m.State, want) } if want := []string{"mesh-build-go.steps", "mesh-build-go.toolchains", "mesh-build-node.steps", "mesh-build-node.toolchains", "mesh-controller.assignments", - "mesh-delivery-assigner.compositions", "mesh-delivery-builder.builds", "mesh-delivery-gate.source", "mesh-delivery-indexer.candidates", + "mesh-delivery-assigner.compositions", "mesh-delivery-builder.builds", "mesh-delivery-gate.source", "mesh-delivery-gate.turns", "mesh-delivery-indexer.candidates", "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules", "mesh-delivery-watchman.presence", "node-engine.have"}; !reflect.DeepEqual(sorted(m.Reads), want) { t.Errorf("reads = %v, want %v", m.Reads, want) } diff --git a/modules/mesh-delivery-checker/module.json b/modules/mesh-delivery-checker/module.json index c53c7b5..eafca18 100644 --- a/modules/mesh-delivery-checker/module.json +++ b/modules/mesh-delivery-checker/module.json @@ -25,6 +25,7 @@ "mesh-delivery-assigner.compositions", "mesh-delivery-builder.builds", "mesh-delivery-gate.source", + "mesh-delivery-gate.turns", "mesh-delivery-indexer.candidates", "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules", -- 2.54.0 From 13e4ed851507057f95ce62406a9814561852fccf Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 11:30:46 +0100 Subject: [PATCH 2/9] =?UTF-8?q?checker:=20a=20trunk=20move=20never=20ends?= =?UTF-8?q?=20a=20check;=20once=20it=20ended,=20its=20pass=20is=20carried?= =?UTF-8?q?=20and=20names=20the=20commits=20it=20was=20carried=20from,=20a?= =?UTF-8?q?nd=20a=20failed=20carry=20names=20the=20move=20(ADR=200548=20?= =?UTF-8?q?=C2=A71,=20=C2=A75;=20issue=20568=20D2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../internal/check/adr548_test.go | 58 ++++++++++++ .../internal/check/case.go | 7 +- .../internal/check/checker.go | 94 +++++++++++++++++-- .../internal/check/gate_test.go | 27 ------ 4 files changed, 147 insertions(+), 39 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/adr548_test.go b/modules/mesh-delivery-checker/internal/check/adr548_test.go index 8ff5cdf..dc42f96 100644 --- a/modules/mesh-delivery-checker/internal/check/adr548_test.go +++ b/modules/mesh-delivery-checker/internal/check/adr548_test.go @@ -108,6 +108,64 @@ func TestOneCheckPerDelivery(t *testing.T) { t.Errorf("judged %d times; the running check is not ended by the turn", j.count()) } }, + // §1, §5: a trunk move during a running check calls the judge once, writes its verdict, keeps the status pending (a pass + // on an old trunk is never shown), then asks one carry, on the recheck stream, and no check. + "a trunk move during a check ends nothing and asks one carry after it": func(t *testing.T) { + var r *rig + var kinds []string + var from []map[string]string + judged := make(chan struct{}) + j := &judgeFunc{f: func(ctx context.Context, c *Case, call int) (Outcome, error) { + kinds, from = append(kinds, c.Kind), append(from, c.CarriedFrom) + if call == 1 { + r.setTrunks(t, map[string]string{repoA: trunkA2, repoSDK: trunkS}) // the trunk moves while it runs + r.ch.TrunkMoved(ctx) // as the watch of the source says it + close(judged) + } else if s := owedState(t, r, repoA, 7, head1); s != "pending" { + t.Errorf("the status during the carry is %q, want pending: the pass on the old trunk is never shown", s) + } + return Outcome{Verdict: passing()}, nil + }} + r = newRig(t, j) + cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1)) + r.ch.Candidate(ctx, "feat%2Fx", cand) + <-judged + r.ch.Wait() + v := r.verdict(t, keyOf(t, "feat/x", cand)) + if j.count() != 2 || kinds[0] != KindNew || kinds[1] != KindTrunk || from[0] != nil || from[1][repoA] != trunkA { + t.Fatalf("judged %d times as %v carried from %v; want a whole check, then one carry from %s", j.count(), kinds, from, trunkA[:12]) + } + if v.State != delivery.VerdictPass || v.Trunk[repoA] != trunkA2 || v.CarriedFrom[repoA] != trunkA { + t.Errorf("the verdict is %s on %v carried from %v; want a pass on the new trunk carried from the old", v.State, v.Trunk, v.CarriedFrom) + } + if s := owedState(t, r, repoA, 7, head1); s != "success" { + t.Errorf("the status after the carry is %q, want success", s) + } + if r.verdicts.creates[keyOf(t, "feat/x", cand)] != 1 { + t.Error("the verdict key was made again: a carry rewrites it") + } + }, + // §5: a carry that fails names the move it failed on. + "a carry that fails names the move": func(t *testing.T) { + j := &judgeFunc{f: func(_ context.Context, c *Case, call int) (Outcome, error) { + if call == 1 { + return Outcome{Verdict: passing()}, nil + } + return Outcome{Verdict: delivery.Verdict{State: delivery.VerdictFail, Failed: delivery.FailedFirst{Test: "unit: m/a.TestX", Words: "want 4"}}}, nil + }} + r := newRig(t, j) + cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1)) + r.ch.Candidate(ctx, "feat%2Fx", cand) + r.ch.Wait() + r.setTrunks(t, map[string]string{repoA: trunkA2, repoSDK: trunkS}) + r.ch.TrunkMoved(ctx) + r.ch.Wait() + v := r.verdict(t, keyOf(t, "feat/x", cand)) + if v.State != delivery.VerdictFail || v.CarriedFrom != nil || !strings.HasPrefix(v.Failed.Words, "want 4") || + !strings.Contains(v.Failed.Words, repoA+" "+trunkA[:12]+" to "+trunkA2[:12]) { + t.Errorf("the failed carry is %s carried from %v with words %q; want the test's words, then the move", v.State, v.CarriedFrom, v.Failed.Words) + } + }, } { t.Run(name, run) } diff --git a/modules/mesh-delivery-checker/internal/check/case.go b/modules/mesh-delivery-checker/internal/check/case.go index 1c99029..8f5fe53 100644 --- a/modules/mesh-delivery-checker/internal/check/case.go +++ b/modules/mesh-delivery-checker/internal/check/case.go @@ -46,9 +46,12 @@ type Case struct { notes []string // Progress says what the check is doing, so a check that ran past its bound names the step it was stuck at. Progress *Progress - // Kind is what asked the check (KindNew, KindNodes, KindTrunk): a re-judge after a trunk move is asked on the build - // seats' recheck stream, every other check on check (ADR 0536 §4). + // Kind is what asked the check (KindNew, KindNodes, KindTrunk): a carry after a trunk or sibling move is asked on the + // build seats' recheck stream, every other check on check (ADR 0536 §4, ADR 0548 §5). Kind string + // CarriedFrom are, for the carry of a verdict that passed, the trunk and sibling commits it was judged on (ADR 0548 §5); + // nil for a whole check. + CarriedFrom map[string]string } // Progress is the step a check is at, written by the check and read when its bound passes. diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 86e55bd..02b5115 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "sort" "strings" "sync" "time" @@ -197,7 +198,7 @@ func (ch *Checker) consider(ctx context.Context, key string, c delivery.Candidat // Every check is asked at once: the build seats' streams hold the backlog, a new head's check and a check after a // failure of the nodes on check, a re-judge after a trunk move on recheck, pulled after them (ADR 0525 §4, §9; // ADR 0536 §4). The checker holds no place and no capacity of its own. - cs.Kind = ch.kindOf(ctx, cs) + ch.prepare(ctx, cs) ch.try(ctx, key, cs) ch.mu.Lock() more := ch.again[cs.Key] @@ -217,29 +218,91 @@ func (ch *Checker) consider(ctx context.Context, key string, c delivery.Candidat } // 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 head asked again only because its trunk moved on the recheck stream, pulled after. +// 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. const ( KindNew = "new" KindNodes = "nodes" KindTrunk = "trunk" ) -// kindOf is the kind of a case's check, read from its verdict key: none (or one left running) is a head never judged; a -// failure of the nodes is asked again after the nodes changed; a verdict on other trunk heads is a re-judge after a trunk -// move. A verdict that stands is ranked with the re-judges: its claim finds nothing to do. -func (ch *Checker) kindOf(ctx context.Context, cs *Case) string { +// prepare reads the case's verdict key and says what its check is (ADR 0548 §1, §5, §7): none (or one left running) is a +// head never judged, a whole check; a failure of the nodes is asked again after the nodes changed; a verdict judged on trunk +// or sibling commits that moved since is carried, its packages run only where the move changed their digest, and a pass's +// commits are named as the carry's CarriedFrom. A verdict that stands is ranked with the carries: its claim finds nothing +// to do. +func (ch *Checker) prepare(ctx context.Context, cs *Case) { + cs.Kind, cs.CarriedFrom = KindNew, nil value, _, found, err := ch.Verdicts.Get(ctx, cs.Key) if err != nil || !found { - return KindNew + return } v, err := delivery.DecodeVerdict(value) switch { case err != nil || v.State == delivery.VerdictRunning: - return KindNew + return case IsNodesFailure(v): - return KindNodes + cs.Kind = KindNodes + return } - return KindTrunk + cs.Kind = KindTrunk + if v.State == delivery.VerdictPass && ch.stale(v, cs) { + cs.CarriedFrom = judgedOn(v) + } +} + +// stale says whether a verdict was judged on commits that are no longer the case's: a trunk it merged moved. +func (ch *Checker) stale(v delivery.Verdict, cs *Case) bool { + return !sameTrunk(v.Trunk, cs.Trunk) +} + +// judgedOn are the trunk and sibling commits a verdict was judged on, by repository. +func judgedOn(v delivery.Verdict) map[string]string { + out := make(map[string]string, len(v.Trunk)+len(v.Beside)) + for r, c := range v.Beside { + out[r] = c + } + for r, c := range v.Trunk { + out[r] = c + } + return out +} + +// carried is what a carry's verdict names as carried from (ADR 0548 §5): the commits it was judged on before, of the +// repositories the new verdict still judges or reads; nil when none of them moved. +func carried(from map[string]string, v delivery.Verdict) map[string]string { + out, moved := map[string]string{}, false + for r, c := range from { + now, ok := v.Trunk[r] + if !ok { + now, ok = v.Beside[r] + } + if ok && now != "" && c != "" { + out[r] = c + moved = moved || now != c + } + } + if !moved { + return nil + } + return out +} + +// moveWords say a carry's moves, for the words of a failure: "carried from to of ", each moved +// repository, sorted. +func moveWords(from map[string]string, v delivery.Verdict) string { + var moves []string + for r, c := range from { + now, ok := v.Trunk[r] + if !ok { + now = v.Beside[r] + } + if now != "" && now != c { + moves = append(moves, fmt.Sprintf("%s %s to %s", r, short12(c), short12(now))) + } + } + sort.Strings(moves) + return strings.Join(moves, ", ") } func (ch *Checker) candidate(key string, fallback delivery.Candidate) delivery.Candidate { @@ -390,6 +453,17 @@ func (ch *Checker) try(ctx context.Context, key string, cs *Case) { } v := out.Verdict v.Group, v.Heads, v.Trunk = cs.Group, cs.Heads, cs.Trunk + if cs.CarriedFrom != nil { + // A carry (ADR 0548 §5): a pass names the commits it was carried from; a failure names the move it failed on. + switch v.State { + case delivery.VerdictPass: + v.CarriedFrom = carried(cs.CarriedFrom, v) + case delivery.VerdictFail: + if moves := moveWords(cs.CarriedFrom, v); moves != "" { + v.Failed.Words = strings.TrimSpace(v.Failed.Words + "\nfailed on the carry over the move of " + moves + ": the move changed what these tests read") + } + } + } if v.Took.Queue == 0 && !cs.Queued.IsZero() && began.After(cs.Queued) { v.Took.Queue = began.Sub(cs.Queued).Milliseconds() } diff --git a/modules/mesh-delivery-checker/internal/check/gate_test.go b/modules/mesh-delivery-checker/internal/check/gate_test.go index c66168e..8338154 100644 --- a/modules/mesh-delivery-checker/internal/check/gate_test.go +++ b/modules/mesh-delivery-checker/internal/check/gate_test.go @@ -4,7 +4,6 @@ import ( "context" "strings" "testing" - "time" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -88,29 +87,3 @@ func TestATrunkMoveMarksEveryOpenPullRequestIntoItPendingAtOnce(t *testing.T) { t.Errorf("mesh-sdk#13, alone into a trunk that did not move, is %q, want its success to stand", s) } } - -func TestACheckThatEndsAfterItsTrunkMovedOwesPendingNotItsVerdict(t *testing.T) { // gate (a): a pass on an old trunk is never shown - ctx := context.Background() - var r *rig - release, judged := make(chan struct{}), make(chan struct{}) - j := &judgeFunc{f: func(ctx context.Context, c *Case, call int) (Outcome, error) { - if call == 1 { - r.setTrunks(t, map[string]string{repoA: trunkA2, repoSDK: trunkS}) // the trunk moves while it runs - defer close(judged) - } else { - <-release // the check asked again for the new trunk waits in the stream meanwhile - } - return Outcome{Verdict: passing()}, nil - }} - r = newRig(t, j) - r.ch.Candidate(ctx, "feat%2Fx", candidateOf(t, "feat/x", pullOf(repoA, 7, head1))) - <-judged - for deadline := time.Now().Add(2 * time.Second); owedState(t, r, repoA, 7, head1) == "" && time.Now().Before(deadline); time.Sleep(time.Millisecond) { - } - time.Sleep(20 * time.Millisecond) - if s := owedState(t, r, repoA, 7, head1); s != "pending" { - t.Errorf("the status is %q, want pending: the pass was judged on a trunk that moved", s) - } - close(release) - r.ch.Wait() -} -- 2.54.0 From e2c41ffa5ac08b39b5ff32384e5d702389b83725 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 11:32:31 +0100 Subject: [PATCH 3/9] =?UTF-8?q?checker:=20a=20carry=20keeps=20the=20compos?= =?UTF-8?q?ition=20it=20passed=20while=20no=20composed=20fingerprint=20cha?= =?UTF-8?q?nged,=20and=20composes=20again=20otherwise=20(ADR=200548=20?= =?UTF-8?q?=C2=A75;=20issue=20568=20D3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../internal/check/adr548_test.go | 32 +++++++++++++++++++ .../internal/check/awaiting.go | 3 ++ .../internal/check/case.go | 7 +++- .../internal/check/checker.go | 6 +++- .../internal/check/composition_test.go | 10 ++++++ .../internal/check/judge.go | 10 ++++++ 6 files changed, 66 insertions(+), 2 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/adr548_test.go b/modules/mesh-delivery-checker/internal/check/adr548_test.go index dc42f96..b4db1a8 100644 --- a/modules/mesh-delivery-checker/internal/check/adr548_test.go +++ b/modules/mesh-delivery-checker/internal/check/adr548_test.go @@ -43,6 +43,14 @@ func waitFor(t testing.TB, what string, ok func() bool) { } } +// pullWithFP is a pull request touching the module m at the fingerprint `src2:` and 64 times the hex digit d. +func pullWithFP(repo string, number int, head string, d string) delivery.CandidatePull { + p := pullOf(repo, number, head) + p.Modules = []string{"m"} + p.Fps = map[string]delivery.CandidateModule{"m": {FP2: "src2:" + strings.Repeat(d, 64)}} + return p +} + func TestOneCheckPerDelivery(t *testing.T) { ctx := context.Background() for name, run := range map[string]func(t *testing.T){ @@ -145,6 +153,30 @@ func TestOneCheckPerDelivery(t *testing.T) { t.Error("the verdict key was made again: a carry rewrites it") } }, + // §5: the composition is kept when no fingerprint of a module it composed changed, and runs again otherwise. + "a carry keeps a passed composition while the fingerprints stay the same": func(t *testing.T) { + var kept []bool + j := &judgeFunc{f: func(_ context.Context, c *Case, _ int) (Outcome, error) { + kept = append(kept, c.Composition != nil) + v := passing() + v.Composition = &delivery.Composition{State: delivery.CompositionPass, SDK: "v0.1.14-x", FPSet: c.FPSet} + return Outcome{Verdict: v}, nil + }} + r := newRig(t, j) + cand := candidateOf(t, "feat/x", pullWithFP(repoA, 7, head1, "a")) + r.ch.Candidate(ctx, "feat%2Fx", cand) + r.ch.Wait() + r.setTrunks(t, map[string]string{repoA: trunkA2, repoSDK: trunkS}) + r.ch.TrunkMoved(ctx) // the indexer's fingerprints of the candidate stay the same + r.ch.Wait() + r.ch.Candidate(ctx, "feat%2Fx", candidateOf(t, "feat/x", pullWithFP(repoA, 7, head1, "b"))) // a fingerprint changed with the move + r.setTrunks(t, map[string]string{repoA: trunkA, repoSDK: trunkS}) + r.ch.TrunkMoved(ctx) + r.ch.Wait() + if len(kept) != 3 || kept[0] || !kept[1] || kept[2] { + t.Errorf("the composition was kept in the checks %v; want composed, kept on the carry, composed again once a fingerprint changed", kept) + } + }, // §5: a carry that fails names the move it failed on. "a carry that fails names the move": func(t *testing.T) { j := &judgeFunc{f: func(_ context.Context, c *Case, call int) (Outcome, error) { diff --git a/modules/mesh-delivery-checker/internal/check/awaiting.go b/modules/mesh-delivery-checker/internal/check/awaiting.go index 1fbae2c..74abfda 100644 --- a/modules/mesh-delivery-checker/internal/check/awaiting.go +++ b/modules/mesh-delivery-checker/internal/check/awaiting.go @@ -127,6 +127,9 @@ func (ch *Checker) Composed(ctx context.Context, comp delivery.CompositionVerdic return } withComposition(&v, comp) + if v.Composition != nil && v.Composition.State != delivery.CompositionNotRun { + v.Composition.FPSet = cs.FPSet + } data, err := delivery.EncodeVerdict(v) if err != nil { ch.logf("%s: the verdict with its composition cannot be written: %v", cs.Key, err) diff --git a/modules/mesh-delivery-checker/internal/check/case.go b/modules/mesh-delivery-checker/internal/check/case.go index 8f5fe53..1d4ec4d 100644 --- a/modules/mesh-delivery-checker/internal/check/case.go +++ b/modules/mesh-delivery-checker/internal/check/case.go @@ -52,6 +52,11 @@ type Case struct { // CarriedFrom are, for the carry of a verdict that passed, the trunk and sibling commits it was judged on (ADR 0548 §5); // nil for a whole check. CarriedFrom map[string]string + // FPSet is the candidate's fingerprint set (Candidate.FPSet): a composition names the set it judged. + FPSet string + // Composition is, for a carry, the composition that passed on the same fingerprint set: it is kept and not asked again, + // since no fingerprint of a module it composed changed (ADR 0548 §5). Nil composes. + Composition *delivery.Composition } // Progress is the step a check is at, written by the check and read when its bound passes. @@ -83,7 +88,7 @@ func CaseOf(c delivery.Candidate, trunk map[string]string) *Case { if err != nil { return nil } - cs := &Case{Key: key, Group: c.Group, HeadsHash: c.HeadsHash, Heads: GateHeads(c), Trunk: map[string]string{}, Progress: &Progress{}} + cs := &Case{Key: key, Group: c.Group, HeadsHash: c.HeadsHash, Heads: GateHeads(c), Trunk: map[string]string{}, Progress: &Progress{}, FPSet: c.FPSet} for _, p := range c.Pulls { switch head, known := trunk[p.Repository]; { case !known || head == "": diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 02b5115..af8cd96 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -232,7 +232,7 @@ const ( // commits are named as the carry's CarriedFrom. A verdict that stands is ranked with the carries: its claim finds nothing // to do. func (ch *Checker) prepare(ctx context.Context, cs *Case) { - cs.Kind, cs.CarriedFrom = KindNew, nil + cs.Kind, cs.CarriedFrom, cs.Composition = KindNew, nil, nil value, _, found, err := ch.Verdicts.Get(ctx, cs.Key) if err != nil || !found { return @@ -248,6 +248,10 @@ func (ch *Checker) prepare(ctx context.Context, cs *Case) { cs.Kind = KindTrunk if v.State == delivery.VerdictPass && ch.stale(v, cs) { cs.CarriedFrom = judgedOn(v) + if c := v.Composition; c != nil && c.State == delivery.CompositionPass && c.FPSet != "" && c.FPSet == cs.FPSet { + kept := *c + cs.Composition = &kept + } } } diff --git a/modules/mesh-delivery-checker/internal/check/composition_test.go b/modules/mesh-delivery-checker/internal/check/composition_test.go index 19bb6b6..9320456 100644 --- a/modules/mesh-delivery-checker/internal/check/composition_test.go +++ b/modules/mesh-delivery-checker/internal/check/composition_test.go @@ -28,6 +28,7 @@ func TestAPassingCheckTakesTheAssignersComposition(t *testing.T) { f := &fakeCompositions{v: delivery.CompositionVerdict{Group: "feat/x", Heads: "0123456789ab", State: delivery.CompositionPass, SDK: "v0.1.14-x", Machines: []string{"g14"}, Took: 40}} b.ex.Compositions = f cs := caseOf(t, pullOf(repoA, 7, head1)) + cs.FPSet = "fps-of-the-candidate" v := b.judge(t, cs).Verdict if v.State != delivery.VerdictPass || v.Composition == nil || v.Composition.State != delivery.CompositionPass || v.Composition.SDK != "v0.1.14-x" || v.Took.Composition != 40 { t.Fatalf("verdict = %+v, composition %+v", v, v.Composition) @@ -38,6 +39,15 @@ func TestAPassingCheckTakesTheAssignersComposition(t *testing.T) { if !compositionMentions(v, "passed") { t.Errorf("the line says %q", compositionWords(v.Composition, v.Took.Composition)) } + if v.Composition.FPSet == "" || v.Composition.FPSet != cs.FPSet { + t.Errorf("the composition names the fingerprint set %q, want the candidate's %q (ADR 0548 §5)", v.Composition.FPSet, cs.FPSet) + } + // ADR 0548 §5: a carry whose fingerprints did not change keeps the composition it passed and asks the assigner nothing. + carry := caseOf(t, pullOf(repoA, 7, head1)) + carry.CarriedFrom, carry.Composition = map[string]string{repoA: trunkA2}, v.Composition + if kept := b.judge(t, carry).Verdict; kept.Composition == nil || kept.Composition.SDK != "v0.1.14-x" || len(f.asked) != 1 { + t.Errorf("the carry's composition is %+v and the assigner was asked %d times; want the kept one and no second ask", kept.Composition, len(f.asked)) + } } func compositionMentions(v delivery.Verdict, word string) bool { diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 66a7fcc..0687a57 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -230,6 +230,13 @@ func (st *state) finish(ctx context.Context, v delivery.Verdict) Outcome { // fails the check, naming the machine it failed at. A try is not composed (it has no candidate); a composition that // cannot be had leaves it not run, said; with no assigner running it is not judged, said at once. func (st *state) composed(ctx context.Context, v *delivery.Verdict) { + if st.c.Composition != nil && st.c.Try == nil { + // A carry whose candidate's fingerprints did not change keeps the composition it passed (ADR 0548 §5). + kept := *st.c.Composition + v.Composition = &kept + st.c.note("the composition is kept: no fingerprint of a module it composed changed") + return + } if st.j.Compositions == nil || st.c.Try != nil { return } @@ -243,6 +250,9 @@ func (st *state) composed(ctx context.Context, v *delivery.Verdict) { return } withComposition(v, comp) + if v.Composition != nil && v.Composition.State != delivery.CompositionNotRun { + v.Composition.FPSet = st.c.FPSet // a carry keeps it while the fingerprints stay the same (ADR 0548 §5) + } } // outcome fills the verdict's levels, times, machines and composition from what ran. -- 2.54.0 From 5fed42982770fe3df3082c7b2b080742d969f2d9 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 11:33:35 +0100 Subject: [PATCH 4/9] =?UTF-8?q?checker:=20a=20verdict=20names=20the=20sibl?= =?UTF-8?q?ings=20it=20read=20at=20their=20trunk,=20and=20a=20sibling's=20?= =?UTF-8?q?move=20is=20carried=20as=20a=20trunk=20move=20is=20(ADR=200548?= =?UTF-8?q?=20=C2=A76;=20issue=20568=20D4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../internal/check/adr548_test.go | 34 +++++++++++++++++++ .../internal/check/beside.go | 18 ++++++++-- .../internal/check/checker.go | 34 +++++++++++++++---- .../internal/check/judge.go | 9 ++++- .../internal/check/pins_test.go | 6 +++- .../internal/check/read.go | 2 +- 6 files changed, 92 insertions(+), 11 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/adr548_test.go b/modules/mesh-delivery-checker/internal/check/adr548_test.go index b4db1a8..99e2d95 100644 --- a/modules/mesh-delivery-checker/internal/check/adr548_test.go +++ b/modules/mesh-delivery-checker/internal/check/adr548_test.go @@ -177,6 +177,40 @@ func TestOneCheckPerDelivery(t *testing.T) { t.Errorf("the composition was kept in the checks %v; want composed, kept on the carry, composed again once a fingerprint changed", kept) } }, + // §6: a sibling's move puts the verdict out of date, its status pending saying it is carried, and the carry's verdict + // names the sibling's old commit; the status follows the carry's verdict. Which packages run is the digest's (only + // those that name MESH_CHECK_BESIDE change with a sibling), the build module's part. + "a sibling's move is carried and the status follows the carry": func(t *testing.T) { + var r *rig + var from map[string]string + j := &judgeFunc{f: func(_ context.Context, c *Case, call int) (Outcome, error) { + if call == 2 { + from = c.CarriedFrom + if s := owedState(t, r, repoA, 7, head1); s != "pending" { + t.Errorf("the status during the sibling's carry is %q, want pending", s) + } + } + return Outcome{Verdict: passing(), Beside: map[string]string{catalogueRepository: r.trunks.Heads()[catalogueRepository]}}, nil + }} + r = newRig(t, j) + r.setTrunks(t, map[string]string{repoA: trunkA, repoSDK: trunkS, catalogueRepository: trunkS}) + cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1)) + r.ch.Candidate(ctx, "feat%2Fx", cand) + r.ch.Wait() + if b := r.verdict(t, keyOf(t, "feat/x", cand)).Beside; b[catalogueRepository] != trunkS { + t.Fatalf("the verdict names the siblings %v, want the catalogue at %s", b, trunkS[:12]) + } + r.setTrunks(t, map[string]string{repoA: trunkA, repoSDK: trunkS, catalogueRepository: trunkS2}) + r.ch.TrunkMoved(ctx) + r.ch.Wait() + v := r.verdict(t, keyOf(t, "feat/x", cand)) + if j.count() != 2 || from[catalogueRepository] != trunkS || v.CarriedFrom[catalogueRepository] != trunkS || v.Beside[catalogueRepository] != trunkS2 { + t.Errorf("judged %d times, carried from %v, verdict beside %v carried from %v; want one carry over the catalogue's move", j.count(), from, v.Beside, v.CarriedFrom) + } + if s := owedState(t, r, repoA, 7, head1); s != "success" { + t.Errorf("the status after the carry is %q, want success", s) + } + }, // §5: a carry that fails names the move it failed on. "a carry that fails names the move": func(t *testing.T) { j := &judgeFunc{f: func(_ context.Context, c *Case, call int) (Outcome, error) { diff --git a/modules/mesh-delivery-checker/internal/check/beside.go b/modules/mesh-delivery-checker/internal/check/beside.go index ebe0df5..042d178 100644 --- a/modules/mesh-delivery-checker/internal/check/beside.go +++ b/modules/mesh-delivery-checker/internal/check/beside.go @@ -12,6 +12,10 @@ type BesideRef struct { Repository string `json:"repository"` Commit string `json:"commit"` Ref string `json:"ref,omitempty"` + // atTrunk says the sibling was read at its trunk's head, so a move of that trunk puts the verdict out of date and is + // carried (novox/hq ADR 0548 §6). Never sent: the node-engine at the commit the mesh runs, mesh-sdk at a pin and a + // group's member at its head do not move with a trunk. + atTrunk bool } const ( @@ -51,7 +55,7 @@ func (j *Examiner) besideFor(ctx context.Context, c *Case, p Pull) ([]BesideRef, switch { case repo == catalogueRepository: if head := heads[repo]; head != "" { - out = append(out, BesideRef{Repository: repo, Commit: head}) + out = append(out, BesideRef{Repository: repo, Commit: head, atTrunk: c.member(repo) == nil}) } case repo == nodeEngineRepo: commit := "" @@ -82,7 +86,7 @@ func (j *Examiner) besideFor(ctx context.Context, c *Case, p Pull) ([]BesideRef, if member := c.member(repo); member != nil { out = append(out, BesideRef{Repository: repo, Commit: member.Head, Ref: refOf(member.Number)}) } else if head := heads[repo]; head != "" { - out = append(out, BesideRef{Repository: repo, Commit: head}) + out = append(out, BesideRef{Repository: repo, Commit: head, atTrunk: true}) } else if repo == controllerRepo && p.Repository == nodeEngineRepo { // The node-engine's step-nine test judges the controller's own manifest and fails without it (issue 1295): a // checker that cannot place it says so, where placing none left that test to skip. @@ -108,3 +112,13 @@ func refOf(number int) string { } return "refs/pull/" + strconv.Itoa(number) + "/head" } + +// besideAtTrunk are the siblings of refs read at their trunk's head that the case does not merge, by repository: what a +// verdict names as Beside (ADR 0548 §6). +func besideAtTrunk(c *Case, refs []BesideRef, into map[string]string) { + for _, r := range refs { + if _, merged := c.Trunk[r.Repository]; r.atTrunk && !merged { + into[r.Repository] = r.Commit + } + } +} diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index af8cd96..8b838b7 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -43,6 +43,9 @@ type Outcome struct { // Judged says, by pull request (pullKey), which program judged a change to a build module (ADR 0535 §5); a pull request // that changes none is not named. Judged map[string]string + // Beside are the siblings the check read at their trunk's head, by repository (ADR 0548 §6): the verdict names them, and + // a move of one is carried. + Beside map[string]string } // BuildNote is the state of a candidate's module build, by module: built, failed or waiting (ADR 0484 clause 7). @@ -255,9 +258,20 @@ func (ch *Checker) prepare(ctx context.Context, cs *Case) { } } -// stale says whether a verdict was judged on commits that are no longer the case's: a trunk it merged moved. +// stale says whether a verdict was judged on commits that are no longer the case's: a trunk it merged moved, or a sibling it +// read at its trunk's head (ADR 0548 §6). func (ch *Checker) stale(v delivery.Verdict, cs *Case) bool { - return !sameTrunk(v.Trunk, cs.Trunk) + return !sameTrunk(v.Trunk, cs.Trunk) || besideMoved(v.Beside, ch.heads()) +} + +// besideMoved says whether the trunk of a sibling read at these commits moved: its head now is known and another. +func besideMoved(beside, heads map[string]string) bool { + for r, c := range beside { + if h := heads[r]; h != "" && h != c { + return true + } + } + return false } // judgedOn are the trunk and sibling commits a verdict was judged on, by repository. @@ -361,7 +375,7 @@ func (ch *Checker) claim(ctx context.Context, cs *Case, first bool, epoch uint64 return &claimed{rev}, nil } return nil, nil - case !sameTrunk(cur.Trunk, cs.Trunk): + case ch.stale(cur, cs): retake = true case cur.State == delivery.VerdictFail && IsNodesFailure(cur): ch.mu.Lock() @@ -456,7 +470,15 @@ func (ch *Checker) try(ctx context.Context, key string, cs *Case) { return } v := out.Verdict - v.Group, v.Heads, v.Trunk = cs.Group, cs.Heads, cs.Trunk + v.Group, v.Heads, v.Trunk, v.Beside = cs.Group, cs.Heads, cs.Trunk, nil + for r, c := range out.Beside { + if _, merged := cs.Trunk[r]; !merged && c != "" { + if v.Beside == nil { + v.Beside = map[string]string{} + } + v.Beside[r] = c + } + } if cs.CarriedFrom != nil { // A carry (ADR 0548 §5): a pass names the commits it was carried from; a failure names the move it failed on. switch v.State { @@ -489,7 +511,7 @@ func (ch *Checker) try(ctx context.Context, key string, cs *Case) { ch.nodesFail[cs.Key] = epoch ch.mu.Unlock() } - if ch.movedSince(cs) { + if ch.movedSince(cs) || besideMoved(v.Beside, ch.heads()) { // Judged on a trunk that moved while it ran: the verdict is kept as judged, and the status stays pending until the // check of the new trunk, which Reconsider starts, ends. ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, Outcome{}) @@ -633,7 +655,7 @@ func (ch *Checker) TrunkMoved(ctx context.Context) { if err != nil || !found { continue // never judged: no status to put out of date } - if v, err := delivery.DecodeVerdict(value); err == nil && sameTrunk(v.Trunk, cs.Trunk) { + if v, err := delivery.DecodeVerdict(value); err == nil && !ch.stale(v, cs) { continue } ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, Outcome{}) diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 0687a57..38355b0 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -203,12 +203,15 @@ type state struct { nodes map[string]bool clone time.Duration slow []string + // beside are the siblings read at their trunk's head, by repository (ADR 0548 §6). + beside map[string]string } // Judge runs the case (it is the Judge of a Checker). A failure of the change, of a merge or of a step is a verdict; an error is only a case that cannot // be judged at all. func (j *Examiner) Judge(ctx context.Context, c *Case) (Outcome, error) { - st := &state{j: j, c: c, levels: map[string]*acc{delivery.LevelUnit: {}, delivery.LevelIntegration: {}}, nodes: map[string]bool{}} + st := &state{j: j, c: c, levels: map[string]*acc{delivery.LevelUnit: {}, delivery.LevelIntegration: {}}, nodes: map[string]bool{}, + beside: map[string]string{}} if v := st.judgeAll(ctx); v != nil { return st.finish(ctx, *v), nil } @@ -292,6 +295,9 @@ func (st *state) outcome(v delivery.Verdict) Outcome { } } out := Outcome{Verdict: v, Slowest: slow} + if len(st.beside) > 0 { + out.Beside = st.beside + } for _, t := range st.targets { if t.judged != "" { if out.Judged == nil { @@ -353,6 +359,7 @@ func (st *state) judgeAll(ctx context.Context) *delivery.Verdict { if len(beside) > 0 { args["beside"] = beside t.beside = besideLines(beside) + besideAtTrunk(st.c, beside, st.beside) } if p.Repository == catalogueRepository { t.runner = RunnerWorkspace // used by the build module when the merge changes it (ADR 0535 §1) diff --git a/modules/mesh-delivery-checker/internal/check/pins_test.go b/modules/mesh-delivery-checker/internal/check/pins_test.go index 0c4dba0..a2ddebc 100644 --- a/modules/mesh-delivery-checker/internal/check/pins_test.go +++ b/modules/mesh-delivery-checker/internal/check/pins_test.go @@ -324,7 +324,11 @@ func TestTheChangesOwnRepositoryIsBesideItAtItsTrunk(t *testing.T) { func TestACatalogueChangeHasTheCatalogueBesideItAtItsTrunk(t *testing.T) { b := newBench(t) b.ex.Trunks = trunksOf(t, map[string]string{catalogueRepository: trunkA, SDKRepository: trunkS, controllerRepo: trunkA2}) - b.judge(t, caseOf(t, pullOf(catalogueRepository, 12, head1))) + out := b.judge(t, caseOf(t, pullOf(catalogueRepository, 12, head1))) + // ADR 0548 §6: the outcome names the siblings read at their trunk's head, never the repository it merges. + if out.Beside[SDKRepository] != trunkS || out.Beside[controllerRepo] != trunkA2 || out.Beside[catalogueRepository] != "" { + t.Errorf("the siblings named for the verdict = %v, want mesh-sdk and mesh-controller at their trunks and not the catalogue", out.Beside) + } got := besideOf(t, b, catalogueRepository) if c := got[catalogueRepository]; c.Commit != trunkA || c.Ref != "" { t.Fatalf("the catalogue beside its own change = %+v, want its trunk %s", c, trunkA) diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index 6bfa883..11be7c4 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -489,7 +489,7 @@ func (r Reader) Status() (Status, error) { case IsNodesFailure(v): st.Waiting[KindNodes] = append(st.Waiting[KindNodes], c.Group) case heads != nil: - if cs := CaseOf(c, heads); cs != nil && !sameTrunk(v.Trunk, cs.Trunk) { + if cs := CaseOf(c, heads); cs != nil && (!sameTrunk(v.Trunk, cs.Trunk) || besideMoved(v.Beside, heads)) { st.Waiting[KindTrunk] = append(st.Waiting[KindTrunk], c.Group) if why := heldBy(turns, c, heads); why != "" { st.HeldByTurn[c.Group] = why -- 2.54.0 From d91ea41f6463e34e51d19ebc5e3cd574c1e137b2 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 11:34:08 +0100 Subject: [PATCH 5/9] =?UTF-8?q?checker:=20only=20a=20head=20git=20refused?= =?UTF-8?q?=20gets=20a=20whole=20check=20after=20a=20trunk=20move;=20a=20c?= =?UTF-8?q?onflicting=20carry=20fails=20naming=20the=20files=20and=20asks?= =?UTF-8?q?=20nothing=20(ADR=200548=20=C2=A77;=20issue=20568=20D5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../internal/check/adr548_test.go | 32 +++++++++++++++++++ .../internal/check/checker.go | 11 +++++++ .../internal/check/judge.go | 2 +- 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/modules/mesh-delivery-checker/internal/check/adr548_test.go b/modules/mesh-delivery-checker/internal/check/adr548_test.go index 99e2d95..7bd400c 100644 --- a/modules/mesh-delivery-checker/internal/check/adr548_test.go +++ b/modules/mesh-delivery-checker/internal/check/adr548_test.go @@ -211,6 +211,38 @@ func TestOneCheckPerDelivery(t *testing.T) { t.Errorf("the status after the carry is %q, want success", s) } }, + // §7: a carry whose merge conflicts fails naming the files and asks no check; a later move onto which the head merges + // cleanly asks a whole check, not a carry. + "a carry git refuses fails naming the files, and the next move asks a whole check": func(t *testing.T) { + var kinds []string + var from []map[string]string + j := &judgeFunc{f: func(_ context.Context, c *Case, call int) (Outcome, error) { + kinds, from = append(kinds, c.Kind), append(from, c.CarriedFrom) + if call == 2 { + return Outcome{Verdict: delivery.Verdict{State: delivery.VerdictFail, Failed: delivery.FailedFirst{Machine: "ace", + Words: repoA + "#7 " + ConflictWords + "go.mod"}}}, nil + } + return Outcome{Verdict: passing()}, nil + }} + r := newRig(t, j) + cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1)) + r.ch.Candidate(ctx, "feat%2Fx", cand) + r.ch.Wait() + r.setTrunks(t, map[string]string{repoA: trunkA2, repoSDK: trunkS}) + r.ch.TrunkMoved(ctx) + r.ch.Wait() + r.ch.Reconsider(ctx) // anything else that looks again asks nothing of a conflict + r.ch.Wait() + if v := r.verdict(t, keyOf(t, "feat/x", cand)); j.count() != 2 || !IsConflictFailure(v) || !strings.Contains(v.Failed.Words, "go.mod") { + t.Fatalf("judged %d times, verdict %+v; want the carry failed naming go.mod and nothing asked after it", j.count(), v.Failed) + } + r.setTrunks(t, map[string]string{repoA: trunkA, repoSDK: trunkS}) + r.ch.TrunkMoved(ctx) + r.ch.Wait() + if j.count() != 3 || kinds[1] != KindTrunk || from[1] == nil || kinds[2] != KindNew || from[2] != nil { + t.Errorf("checks %v carried from %v; want a whole check, a carry, then a whole check after the conflict", kinds, from) + } + }, // §5: a carry that fails names the move it failed on. "a carry that fails names the move": func(t *testing.T) { j := &judgeFunc{f: func(_ context.Context, c *Case, call int) (Outcome, error) { diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 8b838b7..4e0f58d 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -247,6 +247,8 @@ func (ch *Checker) prepare(ctx context.Context, cs *Case) { case IsNodesFailure(v): cs.Kind = KindNodes return + case IsConflictFailure(v): + return // a head git refused: its next check is whole (ADR 0548 §7) } cs.Kind = KindTrunk if v.State == delivery.VerdictPass && ch.stale(v, cs) { @@ -440,6 +442,15 @@ func (ch *Checker) Held() map[string]string { return out } +// ConflictWords are what the words of a head that git would not merge onto its trunk say before the files in conflict. +const ConflictWords = "does not merge into the trunk: files in conflict: " + +// IsConflictFailure says whether a verdict failed because its head did not merge onto its trunk (ADR 0548 §7): it has no +// pass to carry, so its next check, at a push or at a later trunk move, is a whole check of its head. +func IsConflictFailure(v delivery.Verdict) bool { + return v.State == delivery.VerdictFail && strings.Contains(v.Failed.Words, ConflictWords) +} + // IsNodesFailure says whether a verdict failed because a part of the mesh the check reads was not heard (UnheardWords): // it is judged again when the nodes change. No check fails for want of a build node any more: its asks wait in the build // seats' streams (ADR 0525 §6, issue 1912). diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 38355b0..b136728 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -431,7 +431,7 @@ func (st *state) ask(ctx context.Context, t *target, merged *[]Fetched) *deliver if st.c.Try == nil { st.j.writePreview(ctx, p, append(*merged, t.fetched)) } - return fail(step.Node, fmt.Sprintf("%s#%d does not merge into the trunk: files in conflict: %s", p.Repository, p.Number, strings.Join(t.fetched.Conflicts, ", "))) + return fail(step.Node, fmt.Sprintf("%s#%d %s%s", p.Repository, p.Number, ConflictWords, strings.Join(t.fetched.Conflicts, ", "))) } t.changed = t.fetched.Changed if !changesBuildGo(p.Repository, t.changed) { -- 2.54.0 From d2eb3e5ecf67c71715d5c1146a0696f7b1a0f638 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 11:35:16 +0100 Subject: [PATCH 6/9] =?UTF-8?q?checker:=20the=20line=20of=20a=20carried=20?= =?UTF-8?q?verdict=20names=20the=20head=20on=20the=20new=20trunk,=20then?= =?UTF-8?q?=20where=20it=20was=20carried=20from;=20a=20verdict=20being=20c?= =?UTF-8?q?arried=20is=20pending=20and=20says=20so=20(ADR=200548=20=C2=A76?= =?UTF-8?q?,=20=C2=A78;=20issue=20568=20D6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../internal/check/adr548_test.go | 34 +++++++++++++ .../internal/check/checker.go | 29 +++++++++-- .../internal/check/line.go | 49 ++++++++++++++++++- 3 files changed, 107 insertions(+), 5 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/adr548_test.go b/modules/mesh-delivery-checker/internal/check/adr548_test.go index 7bd400c..7f2ff65 100644 --- a/modules/mesh-delivery-checker/internal/check/adr548_test.go +++ b/modules/mesh-delivery-checker/internal/check/adr548_test.go @@ -243,6 +243,27 @@ func TestOneCheckPerDelivery(t *testing.T) { t.Errorf("checks %v carried from %v; want a whole check, a carry, then a whole check after the conflict", kinds, from) } }, + // §8, §6: the line of a carried verdict names the head on the new trunk commit first, as the gate and the merge tool + // read a line, then where it was carried from and what the carry ran and kept; while it is carried it is pending and says so. + "the line of a carry says where it was carried from": func(t *testing.T) { + cs, p := lineCase(t, pullOf(repoA, 7, head1)) + v := passVerdict() + v.Trunk = map[string]string{repoA: trunkA} + v.Beside = map[string]string{catalogueRepository: trunkS2} + v.CarriedFrom = map[string]string{repoA: trunkA2, catalogueRepository: trunkS} + line := Line(v, cs, p, nil, nil, nil) + if !strings.HasPrefix(line, "pass 111111111111 on aaaaaaaaaaaa | carried from abababababab, mesh-catalog 555555555555 | unit 12 run") { + t.Errorf("the carried line = %q", line) + } + if head, trunk := checkWordsOf(line); head != head1[:12] || trunk != trunkA[:12] { + t.Errorf("the line names %s on %s first, want the head on the new trunk", head, trunk) + } + cs.CarriedFrom = map[string]string{repoA: trunkA2} + pending := Line(delivery.Verdict{State: delivery.VerdictRunning}, cs, p, nil, nil, nil) + if !strings.HasPrefix(pending, "being carried to 111111111111 on aaaaaaaaaaaa | from abababababab") { + t.Errorf("the pending line of a carry = %q", pending) + } + }, // §5: a carry that fails names the move it failed on. "a carry that fails names the move": func(t *testing.T) { j := &judgeFunc{f: func(_ context.Context, c *Case, call int) (Outcome, error) { @@ -268,3 +289,16 @@ func TestOneCheckPerDelivery(t *testing.T) { t.Run(name, run) } } + +// checkWordsOf is the head and trunk a line names first, as the gate reads it (mesh-delivery-gate onestatus.go checkLine). +func checkWordsOf(line string) (string, string) { + for _, part := range strings.Split(line, " | ") { + f := strings.Fields(part) + for i := 0; i+2 < len(f); i++ { + if f[i+1] == "on" && len(f[i]) == 12 && len(f[i+2]) == 12 { + return f[i], f[i+2] + } + } + } + return "", "" +} diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 4e0f58d..3cfc970 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -525,8 +525,8 @@ func (ch *Checker) try(ctx context.Context, key string, cs *Case) { if ch.movedSince(cs) || besideMoved(v.Beside, ch.heads()) { // Judged on a trunk that moved while it ran: the verdict is kept as judged, and the status stays pending until the // check of the new trunk, which Reconsider starts, ends. - ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, Outcome{}) - ch.logf("%s: %s on a trunk that moved while it ran; the status stays pending", cs.Key, v.State) + ch.owePending(ctx, cs, v) + ch.logf("%s: %s on a trunk that moved while it ran; the status stays pending until it is carried", cs.Key, v.State) return } ch.writeOwed(ctx, cs, v, out) @@ -666,14 +666,37 @@ func (ch *Checker) TrunkMoved(ctx context.Context) { if err != nil || !found { continue // never judged: no status to put out of date } - if v, err := delivery.DecodeVerdict(value); err == nil && !ch.stale(v, cs) { + v, err := delivery.DecodeVerdict(value) + if err == nil && !ch.stale(v, cs) { continue } + if err == nil && v.State == delivery.VerdictPass { + cs.CarriedFrom = judgedOn(v) // the status says it is being carried to the new commit (ADR 0548 §6) + } ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, Outcome{}) } ch.Reconsider(ctx) } +// owePending owes mesh/check pending on the heads of a verdict judged on commits that moved while it ran: a pass says it is +// being carried to the trunk's head now (ADR 0548 §6), anything else that it is checked again. +func (ch *Checker) owePending(ctx context.Context, judged *Case, v delivery.Verdict) { + next := judged + ckey, err := delivery.CandidatesKey(v.Group) + ch.mu.Lock() + c, ok := ch.known[ckey] + ch.mu.Unlock() + if err == nil && ok { + if now := CaseOf(c, ch.heads()); now != nil && now.Key == judged.Key { + next = now + } + } + if v.State == delivery.VerdictPass { + next.CarriedFrom = judgedOn(v) + } + ch.writeOwed(ctx, next, delivery.Verdict{State: delivery.VerdictRunning}, Outcome{}) +} + // movedSince says whether a trunk the case was judged on is no longer the trunk's head. func (ch *Checker) movedSince(cs *Case) bool { now := ch.heads() diff --git a/modules/mesh-delivery-checker/internal/check/line.go b/modules/mesh-delivery-checker/internal/check/line.go index a105441..f09b839 100644 --- a/modules/mesh-delivery-checker/internal/check/line.go +++ b/modules/mesh-delivery-checker/internal/check/line.go @@ -85,18 +85,20 @@ type pieces struct { head, commits, counts, shortCounts, times, shortTimes, over, builds, composition, shortComposition, group string // judged names the program that judged a change to a build module (ADR 0535 §5); never dropped. judged string + // carried says, of a carried verdict, the commits it was carried from (ADR 0548 §8); never dropped. + carried string } // pullKey names a pull request of a case: `#`. func pullKey(p Pull) string { return fmt.Sprintf("%s#%d", p.Repository, p.Number) } func (p pieces) join(drop int) string { - parts := []string{p.head, p.commits, p.counts, p.judged} + parts := []string{p.head, p.commits, p.carried, p.counts, p.judged} switch { case drop < 3 && p.times != "": parts = append(parts, p.times) case drop == 3 && p.shortTimes != "": - parts = []string{p.head, p.commits, p.shortCounts, p.judged, p.shortTimes} + parts = []string{p.head, p.commits, p.carried, p.shortCounts, p.judged, p.shortTimes} } if p.over != "" { parts = append(parts, p.over) @@ -171,6 +173,39 @@ func buildWords(builds []BuildNote) string { return "builds: " + strings.Join(parts, ", ") } +// carriedWords say the commits a verdict was carried from, for the line of pull request p (ADR 0548 §8): its own trunk's +// commit as twelve characters, a sibling's with its name before it; only those that moved, sorted. Empty when none did. +func carriedWords(from map[string]string, v delivery.Verdict, cs *Case, p Pull) string { + var own string + var others []string + for r, old := range from { + now, ok := v.Trunk[r] + if cs != nil && !ok { + now, ok = cs.Trunk[r] + } + if !ok { + now = v.Beside[r] + } + if now == "" || now == old { + continue + } + if strings.EqualFold(r, p.Repository) { + own = short12(old) + continue + } + name := r + if i := strings.LastIndex(r, "/"); i >= 0 { + name = r[i+1:] + } + others = append(others, name+" "+short12(old)) + } + slices.Sort(others) + if own != "" { + others = append([]string{own}, others...) + } + return strings.Join(others, ", ") +} + // parts makes the pieces of the line of a pull request of a case. func parts(v delivery.Verdict, cs *Case, p Pull, builds []BuildNote, slowest []string, judged map[string]string) pieces { var pc pieces @@ -179,8 +214,18 @@ func parts(v delivery.Verdict, cs *Case, p Pull, builds []BuildNote, slowest []s switch v.State { case delivery.VerdictRunning: pc.head = "checking " + commits + if cs != nil && cs.CarriedFrom != nil { + // Out of date and being carried to the new commit (ADR 0548 §6): pending, and saying so. + pc.head = "being carried to " + commits + if w := carriedWords(cs.CarriedFrom, v, cs, p); w != "" { + pc.carried = "from " + w + } + } case delivery.VerdictPass: pc.head = "pass " + commits + if w := carriedWords(v.CarriedFrom, v, cs, p); w != "" { + pc.carried = "carried from " + w // the counts that follow say what the carry ran and kept + } default: pc.head = "failed: " + firstFailed(v) if n := failedCount(v); n > 1 { -- 2.54.0 From f1bb7747aea7b88c8ab6f75cae8983ba0942f2ea Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 11:36:15 +0100 Subject: [PATCH 7/9] =?UTF-8?q?checker:=20one=20integration=20test=20throu?= =?UTF-8?q?gh=20the=20real=20bus=20proves=20a=20merge=20turn=20holds=20a?= =?UTF-8?q?=20check=20off=20until=20it=20goes=20and=20a=20carry=20of=20unc?= =?UTF-8?q?hanged=20digests=20runs=20nothing;=20it=20extends=20the=20equal?= =?UTF-8?q?-inputs=20test,=20so=20the=20count=20stays=20(ADR=200548=20?= =?UTF-8?q?=C2=A73,=20=C2=A75;=20issue=20568=20D7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../mesh-delivery-checker/integration_test.go | 48 +++++++++++++++++-- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go index 9e96ad9..8268215 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go @@ -305,7 +305,8 @@ func startBed(t *testing.T) *bed { for name, history := range map[string]int{delivery.CheckerVerdictsBucket: delivery.CheckerVerdictsHistory, delivery.CheckerKeptBucket: delivery.CheckerKeptHistory, delivery.CheckerOwedBucket: delivery.CheckerOwedHistory, delivery.PreviewsBucket: delivery.PreviewsHistory, delivery.CandidatesBucket: delivery.CandidatesHistory, delivery.ModulesBucket: delivery.ModulesHistory, delivery.ManifestsBucket: delivery.ManifestsHistory, delivery.SourceBucket: delivery.SourceHistory, - delivery.PresenceBucket: delivery.PresenceHistory, delivery.HaveBucket: delivery.HaveHistory, check.TriesBucket: 1} { + delivery.PresenceBucket: delivery.PresenceHistory, delivery.HaveBucket: delivery.HaveHistory, check.TriesBucket: 1, + delivery.TurnsBucket: delivery.TurnsHistory} { kv, err := js.CreateKeyValue(ctx, jetstream.KeyValueConfig{Bucket: name, History: uint8(history)}) if err != nil { t.Fatal(err) @@ -507,9 +508,12 @@ func TestCheckerJudgesACandidateAndOwesItsStatus(t *testing.T) { // N6/AC5, 1300 } } -// TestACheckOfEqualInputsRunsNothing: a second candidate (another head, the same digests) is checked with every package -// taken from the first's kept verdicts: no test step runs and no second kept key appears. -func TestACheckOfEqualInputsRunsNothing(t *testing.T) { // 1287/AC5, 1288/AC6, 1929/AC2 +// TestACheckOrACarryOfEqualInputsRunsNothing: a second candidate (another head, the same digests) is checked with every +// package taken from the first's kept verdicts: no test step runs and no second kept key appears. Then, through the real bus +// (novox/hq ADR 0548 §3, §5): a third candidate is not claimed while the gate's merge turn on its repository stands, and is +// claimed when the turn's key is deleted; and a move of the trunk carries the first verdict on the recheck stream by its +// unchanged digests, running no test, the verdict naming the trunk it was carried from. +func TestACheckOrACarryOfEqualInputsRunsNothing(t *testing.T) { // 1287/AC5, 1288/AC6, 1929/AC2, ADR 0548 §3, §5 b := startBed(t) first := b.candidate(t, "feat/x", pull(repo, 7, head1)) b.start(t, nil) @@ -526,6 +530,42 @@ func TestACheckOfEqualInputsRunsNothing(t *testing.T) { // 1287/AC5, 1288/AC6, 1 if got := keptCount(t, b); got != kept { t.Errorf("%d kept keys, was %d", got, kept) } + + // §3: the gate's turn on the repository holds a new check off; its delete claims it. + at := time.Now().UTC() + turn, err := delivery.EncodeTurn(delivery.Turn{Repository: repo, Waiting: []delivery.TurnWait{{Group: "fix/other", Heads: "0123456789ab", Approved: at, Since: at}}}) + if err != nil { + t.Fatal(err) + } + tk, _ := delivery.TurnsKey(repo) + if _, err := b.buckets[delivery.TurnsBucket].Put(b.ctx, tk, turn); err != nil { + t.Fatal(err) + } + time.Sleep(300 * time.Millisecond) // the checker hears the turn + third := b.candidate(t, "feat/z", pull(repo, 9, "3333333333333333333333333333333333333333")) + time.Sleep(time.Second) + if _, err := b.buckets[delivery.CheckerVerdictsBucket].Get(b.ctx, third); !errors.Is(err, jetstream.ErrKeyNotFound) { + t.Fatalf("verdicts.%s exists (%v) while a merge turn holds its repository", third, err) + } + if err := b.buckets[delivery.TurnsBucket].Delete(b.ctx, tk); err != nil { + t.Fatal(err) + } + b.verdict(t, third, ended) + + // §5: the trunk moves; the first verdict is carried by its digests, nothing runs. + tests = b.fake.count("mesh-build-go.build_go_test") + const moved = "abababababababababababababababababababab" + b.setTrunk(t, map[string]string{repo: moved}) + c := b.verdict(t, first, func(v delivery.Verdict) bool { return ended(v) && v.Trunk[repo] == moved }) + if c.State != delivery.VerdictPass || c.CarriedFrom[repo] != trunk || c.Levels[delivery.LevelUnit].Run != 0 || c.Levels[delivery.LevelIntegration].Run != 0 { + t.Fatalf("the carried verdict = %+v, want a pass carried from %s with no test run", c, trunk[:12]) + } + if got := b.fake.count("mesh-build-go.build_go_test"); got != tests { + t.Errorf("build_go_test called %d times on the carry, was %d: a carry of unchanged digests runs nothing", got, tests) + } + if o := b.owed(t, 7, head1, "success"); !strings.HasPrefix(o.Text, "pass 111111111111 on abababababab | carried from aaaaaaaaaaaa") { + t.Errorf("the carried status says %q", o.Text) + } } func keptCount(t *testing.T, b *bed) int { -- 2.54.0 From 6c3cbee35c49cbe5b224e1d5daccaa166471fe05 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 11:45:13 +0100 Subject: [PATCH 8/9] =?UTF-8?q?checker:=20pin=20mesh-sdk=20at=20the=20grou?= =?UTF-8?q?p's=20head=20f5c233a9288c,=20which=20holds=20the=20merge=20turn?= =?UTF-8?q?s=20and=20the=20carried=20verdict=20(ADR=200548=20=C2=A710;=20i?= =?UTF-8?q?ssue=20568)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- modules/mesh-delivery-checker/go.mod | 2 +- modules/mesh-delivery-checker/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/mesh-delivery-checker/go.mod b/modules/mesh-delivery-checker/go.mod index 87cc899..b765475 100644 --- a/modules/mesh-delivery-checker/go.mod +++ b/modules/mesh-delivery-checker/go.mod @@ -3,7 +3,7 @@ module mesh-delivery-checker go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026095702-79649c3fa9f4 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026102507-f5c233a9288c github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-delivery-checker/go.sum b/modules/mesh-delivery-checker/go.sum index ec7a896..7095bff 100644 --- a/modules/mesh-delivery-checker/go.sum +++ b/modules/mesh-delivery-checker/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026095702-79649c3fa9f4 h1:05sVel/UOvz4/g0dxrDOeVUL7BGakVrxPBlqRq02ZhM= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026095702-79649c3fa9f4/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026102507-f5c233a9288c h1:nciBayWNL5wOUqqgTIK5XxQdd2R+Ce4HuHU2Fwv2AVo= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026102507-f5c233a9288c/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= -- 2.54.0 From 4026f6c188f81865beeb0d39bb89628538dbf235 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 12:14:50 +0100 Subject: [PATCH 9/9] =?UTF-8?q?checker:=20a=20check=20is=20never=20held=20?= =?UTF-8?q?by=20a=20turn=20its=20own=20heads=20wait=20in,=20the=20node-eng?= =?UTF-8?q?ine=20read=20at=20its=20running=20commit=20is=20named=20and=20c?= =?UTF-8?q?arried=20when=20it=20moves,=20and=20a=20gate=20bucket=20made=20?= =?UTF-8?q?after=20the=20start=20is=20opened=20before=20the=20next=20claim?= =?UTF-8?q?=20(ADR=200548=20=C2=A73,=20=C2=A76;=20issue=20568,=20review=20?= =?UTF-8?q?round=201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-delivery-checker/lazyturns.go | 50 ++++++++++++++++++ .../cmd/mesh-delivery-checker/main.go | 40 ++++++++++----- modules/mesh-delivery-checker/go.mod | 2 +- modules/mesh-delivery-checker/go.sum | 4 +- .../internal/check/adr548_test.go | 51 +++++++++++++++++++ .../internal/check/beside.go | 15 ++++-- .../internal/check/checker.go | 28 ++++++++-- .../internal/check/pins_test.go | 7 ++- 8 files changed, 172 insertions(+), 25 deletions(-) create mode 100644 modules/mesh-delivery-checker/cmd/mesh-delivery-checker/lazyturns.go diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/lazyturns.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/lazyturns.go new file mode 100644 index 0000000..4d84c22 --- /dev/null +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/lazyturns.go @@ -0,0 +1,50 @@ +package main + +import ( + "sync" + + "git.novox.be/novox/mesh-sdk/go/delivery" + + "mesh-delivery-checker/internal/check" +) + +// lazyTurns is the gate's merge turns as the checker reads them (novox/hq ADR 0548 §3), opened when the gate's bucket is +// there: before each claim it is opened again until it is, so a checker that started before the gate made the bucket holds +// checks from the gate's first turn on, with no timer and no restart. Until then it holds nothing, and says so once, loudly, +// and once again when it opens. +type lazyTurns struct { + turns *check.Turns + open func() error + logf func(format string, args ...any) + + mu sync.Mutex + opened bool + said bool +} + +// ensure opens the bucket once it is there; a failure is said once. +func (l *lazyTurns) ensure() { + l.mu.Lock() + defer l.mu.Unlock() + if l.opened || l.open == nil { + return + } + if err := l.open(); err != nil { + if !l.said { + l.said = true + l.logf("NO MERGE TURN HOLDS A CHECK: the gate's bucket %s cannot be read (%v); every check is claimed as if no approved merge waited, "+ + "and the bucket is opened again before each claim until it is there", delivery.TurnsBucket, err) + } + return + } + l.opened = true + if l.said { + l.logf("the gate's merge turns are read from %s from now on: an approved merge's turn holds checks off its repositories again", delivery.TurnsBucket) + } +} + +// Holding opens the bucket when it is not yet, then answers what the turns hold. +func (l *lazyTurns) Holding(repositories []string, heads string) (string, delivery.TurnWait, bool) { + l.ensure() + return l.turns.Holding(repositories, heads) +} diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go index a8c0ddc..851b2d5 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go @@ -194,6 +194,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro presence := &buildrun.PresenceSet{} trunks := &check.Trunks{} turns := &check.Turns{} + lazy := &lazyTurns{turns: turns, logf: logger.Printf} // A check is one ask per pull request and build module on the build seats' check stream, pulled by a build node with a // free slot (novox/hq ADR 0525 §1, §3): the checker chooses no node and starts no step. asks := buildrun.JetStreamAsks{JS: js} @@ -201,7 +202,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro Verdicts: kvStore{buckets[delivery.CheckerVerdictsBucket]}, Owed: kvStore{buckets[delivery.CheckerOwedBucket]}, Trunks: trunks, - Turns: turns, + Turns: lazy, Bound: checkBound, Link: link, Logf: logger.Printf, @@ -238,6 +239,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro } } repos := &repoModules{manifests: kvStore{buckets[delivery.ManifestsBucket]}, logf: logger.Printf} + ch.Runs = repos // the node-engine a check reads beside at its running commit (ADR 0548 §6) ch.Judge = newJudge(judgeDeps{ asks: buildrun.Asker{Publisher: asks, Waiter: asks, Bound: stepBound}, tools: tools, trunks: trunks, kept: kvStore{buckets[delivery.CheckerKeptBucket]}, previews: kvStore{buckets[delivery.PreviewsBucket]}, @@ -317,28 +319,40 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro } // The gate's merge turns (novox/hq ADR 0548 §3): no check is claimed of a repository an approved merge waits on, and the // delete of its turn claims it. Read before the candidates, so no check starts past a turn written before this start. - // Where the gate's bucket is not there yet it is read as empty: no turn holds a check until the checker starts again. - if kv := process.OptionalBucket(ctx, js, delivery.TurnsBucket, "no merge turn holds a check until the checker starts again", logger.Printf); kv != nil { + // Where the gate's bucket is not there yet it is read as empty, and opened again before each claim until it is there + // (lazyTurns), so a gate delivered after the checker started holds checks from its first turn on. + lazy.open = func() error { + kv, err := js.KeyValue(ctx, delivery.TurnsBucket) + if err != nil { + return err + } tw, err := settle(ctx, kv, func(e jetstream.KeyValueEntry) { turns.Put(e.Key(), e.Value(), !opOf(e)) }) if err != nil { - process.NotWatched(logger.Printf, delivery.TurnsBucket, err, "no merge turn holds a check until the checker starts again") - } else { - go func() { - errs <- follow(ctx, tw, func(e jetstream.KeyValueEntry) { - if turns.Put(e.Key(), e.Value(), !opOf(e)) { - ch.Reconsider(ctx) // a turn went or changed: a check it held is claimed now - } - }) - }() + return err } + go func() { + errs <- follow(ctx, tw, func(e jetstream.KeyValueEntry) { + if turns.Put(e.Key(), e.Value(), !opOf(e)) { + ch.Reconsider(ctx) // a turn went or changed: a check it held is claimed now + } + }) + }() + return nil } + lazy.ensure() // The modules say which repository holds a TypeScript module, so the Node build module judges it too. mw, err := settle(ctx, buckets[delivery.ModulesBucket], func(e jetstream.KeyValueEntry) { repos.Put(ctx, e.Key(), e.Value(), !opOf(e)) }) if err != nil { return err } go func() { - errs <- follow(ctx, mw, func(e jetstream.KeyValueEntry) { repos.Put(ctx, e.Key(), e.Value(), !opOf(e)) }) + errs <- follow(ctx, mw, func(e jetstream.KeyValueEntry) { + before := repos.CommitOf(check.NodeEngineModule) + repos.Put(ctx, e.Key(), e.Value(), !opOf(e)) + if after := repos.CommitOf(check.NodeEngineModule); after != before { + ch.TrunkMoved(ctx) // the node-engine the mesh runs moved: a verdict that read it beside is carried (ADR 0548 §6) + } + }) }() each := func(e jetstream.KeyValueEntry) { if opOf(e) { diff --git a/modules/mesh-delivery-checker/go.mod b/modules/mesh-delivery-checker/go.mod index b765475..271e3b0 100644 --- a/modules/mesh-delivery-checker/go.mod +++ b/modules/mesh-delivery-checker/go.mod @@ -3,7 +3,7 @@ module mesh-delivery-checker go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026102507-f5c233a9288c + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026111253-9c61918ac36f github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-delivery-checker/go.sum b/modules/mesh-delivery-checker/go.sum index 7095bff..346bdcc 100644 --- a/modules/mesh-delivery-checker/go.sum +++ b/modules/mesh-delivery-checker/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026102507-f5c233a9288c h1:nciBayWNL5wOUqqgTIK5XxQdd2R+Ce4HuHU2Fwv2AVo= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026102507-f5c233a9288c/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026111253-9c61918ac36f h1:9+TZpNq8Ifj1+qmbidE6l/7qkbua4yLdzRMrezNvf1A= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026111253-9c61918ac36f/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= diff --git a/modules/mesh-delivery-checker/internal/check/adr548_test.go b/modules/mesh-delivery-checker/internal/check/adr548_test.go index 7f2ff65..87d7d88 100644 --- a/modules/mesh-delivery-checker/internal/check/adr548_test.go +++ b/modules/mesh-delivery-checker/internal/check/adr548_test.go @@ -264,6 +264,57 @@ func TestOneCheckPerDelivery(t *testing.T) { t.Errorf("the pending line of a carry = %q", pending) } }, + // §3 (review round 1): two approved merges in one turn whose trunk moved each get their carry; neither holds the other's. + "two merges waiting in one turn each get their carry": func(t *testing.T) { + var r *rig + j := &judgeFunc{f: func(context.Context, *Case, int) (Outcome, error) { return Outcome{Verdict: passing()}, nil }} + r = newRig(t, j) + candA, candC := candidateOf(t, "feat/a", pullOf(repoA, 7, head1)), candidateOf(t, "feat/c", pullOf(repoA, 8, head2)) + r.ch.Candidate(ctx, "feat%2Fa", candA) + r.ch.Candidate(ctx, "feat%2Fc", candC) + r.ch.Wait() + a, _ := delivery.DecodeCandidate(candA) + c, _ := delivery.DecodeCandidate(candC) + at := time.Date(2026, 10, 26, 9, 0, 0, 0, time.UTC) + turn, err := delivery.EncodeTurn(delivery.Turn{Repository: repoA, Waiting: []delivery.TurnWait{ + {Group: "feat/a", Heads: GateHeads(a), Approved: at, Since: at}, {Group: "feat/c", Heads: GateHeads(c), Approved: at.Add(time.Minute), Since: at}}}) + if err != nil { + t.Fatal(err) + } + turns := &Turns{} + turns.Put(turnKey(t, repoA), turn, false) + r.ch.Turns = turns + r.setTrunks(t, map[string]string{repoA: trunkA2, repoSDK: trunkS}) // a third merge moves the trunk under both + r.ch.TrunkMoved(ctx) + r.ch.Wait() + for _, cand := range [][]byte{candA, candC} { + group := map[bool]string{true: "feat/a", false: "feat/c"}[string(cand) == string(candA)] + if v := r.verdict(t, keyOf(t, group, cand)); v.Trunk[repoA] != trunkA2 || v.CarriedFrom[repoA] != trunkA { + t.Errorf("%s: verdict on %v carried from %v; want its carry claimed and done", group, v.Trunk, v.CarriedFrom) + } + } + }, + // §6 (review round 1): the node-engine read at the commit the mesh runs is named, and a new running commit is carried. + "a new running node-engine is carried": func(t *testing.T) { + runs := runsAt{nodeEngineModule: trunkS} + j := &judgeFunc{f: func(_ context.Context, c *Case, _ int) (Outcome, error) { + return Outcome{Verdict: passing(), Beside: map[string]string{nodeEngineRepo: runs[nodeEngineModule]}}, nil + }} + r := newRig(t, j) + r.ch.Runs = runs + cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1)) + r.ch.Candidate(ctx, "feat%2Fx", cand) + r.ch.Wait() + r.ch.TrunkMoved(ctx) // nothing moved: nothing is carried + r.ch.Wait() + runs[nodeEngineModule] = trunkS2 + r.ch.TrunkMoved(ctx) // what the indexer's watch does when mesh-host's running commit changes + r.ch.Wait() + v := r.verdict(t, keyOf(t, "feat/x", cand)) + if j.count() != 2 || v.CarriedFrom[nodeEngineRepo] != trunkS || v.Beside[nodeEngineRepo] != trunkS2 { + t.Errorf("judged %d times, verdict beside %v carried from %v; want one carry over the node-engine's new running commit", j.count(), v.Beside, v.CarriedFrom) + } + }, // §5: a carry that fails names the move it failed on. "a carry that fails names the move": func(t *testing.T) { j := &judgeFunc{f: func(_ context.Context, c *Case, call int) (Outcome, error) { diff --git a/modules/mesh-delivery-checker/internal/check/beside.go b/modules/mesh-delivery-checker/internal/check/beside.go index 042d178..fd5ab99 100644 --- a/modules/mesh-delivery-checker/internal/check/beside.go +++ b/modules/mesh-delivery-checker/internal/check/beside.go @@ -16,8 +16,14 @@ type BesideRef struct { // carried (novox/hq ADR 0548 §6). Never sent: the node-engine at the commit the mesh runs, mesh-sdk at a pin and a // group's member at its head do not move with a trunk. atTrunk bool + // runs says the sibling was read at the commit the mesh runs (the node-engine, the indexer's record of mesh-host): a new + // running commit puts the verdict out of date and is carried (ADR 0548 §6). Never sent. + runs bool } +// NodeEngineModule is the module whose running commit the node-engine sibling is read at. +const NodeEngineModule = nodeEngineModule + const ( catalogueRepository = "novox/mesh-catalog" nodeEngineRepo = "novox/mesh-host" @@ -62,11 +68,14 @@ func (j *Examiner) besideFor(ctx context.Context, c *Case, p Pull) ([]BesideRef, if j.Runs != nil { commit = j.Runs.CommitOf(nodeEngineModule) } + atTrunk := false if commit == "" { - commit = heads[repo] + commit, atTrunk = heads[repo], true } if commit != "" { - out = append(out, BesideRef{Repository: repo, Commit: commit}) + // Named in the verdict either way (ADR 0548 §6, issue 568's own example): at the commit the mesh runs, a new + // running commit puts the verdict out of date; at the trunk, a trunk move does (Checker.besideNow). + out = append(out, BesideRef{Repository: repo, Commit: commit, atTrunk: atTrunk, runs: !atTrunk}) } case repo == SDKRepository && p.Repository == controllerRepo && j.Forge != nil: text, err := j.Forge.File(ctx, p.Repository, "go.mod", p.Head) @@ -117,7 +126,7 @@ func refOf(number int) string { // verdict names as Beside (ADR 0548 §6). func besideAtTrunk(c *Case, refs []BesideRef, into map[string]string) { for _, r := range refs { - if _, merged := c.Trunk[r.Repository]; r.atTrunk && !merged { + if _, merged := c.Trunk[r.Repository]; (r.atTrunk || r.runs) && !merged { into[r.Repository] = r.Commit } } diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 3cfc970..e168211 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -69,6 +69,9 @@ type Checker struct { Owed KV Judge Judge Trunks interface{ Heads() map[string]string } + // Runs says which commit the mesh runs of a module (the indexer's modules): the node-engine a verdict read beside at its + // running commit is out of date when that commit moves (ADR 0548 §6); nil reads every sibling at its trunk. + Runs interface{ CommitOf(module string) string } // Turns are the gate's merge turns (ADR 0548 §3): no check is claimed of a repository a turn names, but the check of the // waiting delivery's own heads; nil holds none. Turns interface { @@ -263,13 +266,28 @@ func (ch *Checker) prepare(ctx context.Context, cs *Case) { // stale says whether a verdict was judged on commits that are no longer the case's: a trunk it merged moved, or a sibling it // read at its trunk's head (ADR 0548 §6). func (ch *Checker) stale(v delivery.Verdict, cs *Case) bool { - return !sameTrunk(v.Trunk, cs.Trunk) || besideMoved(v.Beside, ch.heads()) + return !sameTrunk(v.Trunk, cs.Trunk) || besideMoved(v.Beside, ch.besideNow()) } -// besideMoved says whether the trunk of a sibling read at these commits moved: its head now is known and another. -func besideMoved(beside, heads map[string]string) bool { +// besideNow are the commits a sibling is read at now, by repository: its trunk head, and for the node-engine the commit the +// mesh runs when the indexer names one, as besideFor places them. +func (ch *Checker) besideNow() map[string]string { + now := ch.heads() + if ch.Runs != nil { + if c := ch.Runs.CommitOf(nodeEngineModule); c != "" { + if now == nil { + now = map[string]string{} + } + now[nodeEngineRepo] = c + } + } + return now +} + +// besideMoved says whether a sibling read at these commits moved: the commit it is read at now is known and another. +func besideMoved(beside, now map[string]string) bool { for r, c := range beside { - if h := heads[r]; h != "" && h != c { + if h := now[r]; h != "" && h != c { return true } } @@ -522,7 +540,7 @@ func (ch *Checker) try(ctx context.Context, key string, cs *Case) { ch.nodesFail[cs.Key] = epoch ch.mu.Unlock() } - if ch.movedSince(cs) || besideMoved(v.Beside, ch.heads()) { + if ch.movedSince(cs) || besideMoved(v.Beside, ch.besideNow()) { // Judged on a trunk that moved while it ran: the verdict is kept as judged, and the status stays pending until the // check of the new trunk, which Reconsider starts, ends. ch.owePending(ctx, cs, v) diff --git a/modules/mesh-delivery-checker/internal/check/pins_test.go b/modules/mesh-delivery-checker/internal/check/pins_test.go index a2ddebc..727903d 100644 --- a/modules/mesh-delivery-checker/internal/check/pins_test.go +++ b/modules/mesh-delivery-checker/internal/check/pins_test.go @@ -240,7 +240,12 @@ func TestTheSiblingsAreWhereTheTestsStateThem(t *testing.T) { // review D2, ADR catalogue + " go.mod " + head2: "module git.novox.be/novox/mesh-catalog\n"}, onTrunk: map[string]bool{"cb54756c3865": true}} b.ex.Forge = forge // A group of the controller and the catalogue: the controller's tests read the catalogue's trunk, not the group's head. - b.judge(t, caseOf(t, pullOf(controller, 7, head1), pullOf(catalogue, 12, head2))) + out := b.judge(t, caseOf(t, pullOf(controller, 7, head1), pullOf(catalogue, 12, head2))) + // ADR 0548 §6, issue 568's own example: the verdict names the node-engine at the commit the mesh runs, and mesh-sdk at the + // trunk the catalogue's tests read it at; the catalogue, a member here, is not named. + if out.Beside[host] != hostRuns || out.Beside[catalogue] != "" || out.Beside[SDKRepository] != "9999999999999999999999999999999999999999" { + t.Errorf("the siblings named for the verdict = %v, want the node-engine at %s and mesh-sdk at its trunk", out.Beside, hostRuns[:12]) + } got := besideOf(t, b, controller) if c := got[catalogue]; c.Commit != trunkS || c.Ref != "" { t.Errorf("the catalogue beside the controller = %+v, want its trunk %s", c, trunkS) -- 2.54.0