diff --git a/modules/gitea/checks.ts b/modules/gitea/checks.ts index 672ec03b..9208fcc7 100644 --- a/modules/gitea/checks.ts +++ b/modules/gitea/checks.ts @@ -81,3 +81,20 @@ export function mergeableUnder(branch: string, rule: BranchProtection | null | u blocking.map((b) => `${b.context} (${b.state})`).join(", "); return { branch, protected: !!rule, required, blocking, mergeable: blocking.length === 0, says }; } + +/** A commit, by its sha or the start of one: what `gitea_contains` takes, so a branch name or an expression + * never reaches the compare's path. */ +export function isCommit(ref: string): boolean { + return /^[0-9a-f]{7,40}$/i.test(ref); +} + +/** Whether a commit is contained in another — an ancestor of it, or it (novox/hq ADR 0270) — from the forge's + * compare of `in...commit`: the commits on `commit` that `in` lacks. None is contained. An answer that does not + * say how many is no answer. */ +export function containedFrom(compare: unknown): { contains: boolean; ahead: number } { + const total = (compare as Record | null)?.total_commits; + if (typeof total !== "number" || !Number.isInteger(total) || total < 0) { + throw new Error("the forge's compare said no total_commits"); + } + return { contains: total === 0, ahead: total }; +} diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index fe8e4234..10577d13 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -355,6 +355,12 @@ export class GiteaClient { return { sha: String(raw?.sha ?? ""), state: String(raw?.state ?? ""), statuses: Array.isArray(raw?.statuses) ? raw.statuses : [] }; } + /** The forge's compare of two commits, `base...head`: the commits on head that base lacks, and how many + * (novox/hq ADR 0270). Reads only. */ + async compare(owner: string, repo: string, base: string, head: string): Promise { + return this.request(`/repos/${owner}/${repo}/compare/${encodeURIComponent(base)}...${encodeURIComponent(head)}`); + } + /** Replace a comment's body: the delivery's view, kept current in place (novox/hq ADR 0239). */ async editComment(owner: string, repo: string, id: number, body: string): Promise<{ id: number; html_url: string }> { const c = await this.request(`/repos/${owner}/${repo}/issues/comments/${id}`, { method: "PATCH", body: JSON.stringify({ body }) }); diff --git a/modules/gitea/test/checks.test.ts b/modules/gitea/test/checks.test.ts index b29062bb..a6dee70a 100644 --- a/modules/gitea/test/checks.test.ts +++ b/modules/gitea/test/checks.test.ts @@ -46,3 +46,20 @@ test("no rule requires nothing; a rule that cannot be read says nothing, never y assert.equal(unread.mergeable, null); assert.match(unread.says, /could not be read/); }); + +// novox/hq ADR 0270: a held delivery is covered only when a later delivered merge contains it. +test("a commit is contained in another when the compare finds nothing on it the other lacks", async () => { + const { containedFrom, isCommit } = await import("../checks.ts"); + for (const [compare, contains, ahead] of [ + [{ total_commits: 0, commits: [] }, true, 0], + [{ total_commits: 3, commits: [{}, {}, {}] }, false, 3], + ] as const) { + assert.deepEqual(containedFrom(compare), { contains, ahead }); + } + for (const bad of [{}, { total_commits: "0" }, { total_commits: -1 }, null]) { + assert.throws(() => containedFrom(bad), /total_commits/); + } + assert.equal(isCommit("6aabe8e"), true); + assert.equal(isCommit("90516fc0a1b2c3d4e5f60718293a4b5c6d7e8f90"), true); + for (const bad of ["main", "6aabe8e...90516fc", "../x", "6aabe8", ""]) assert.equal(isCommit(bad), false, bad); +}); diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index f18281d8..33b70d41 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -12,7 +12,7 @@ import { registerModuleTools, type ToolDefinition } from "@novox/mesh-sdk/tools" import { emit } from "@novox/mesh-sdk/events"; import { GiteaClient, type BranchProtection } from "../client.js"; import { appendNote, deliveryStatus, run, viewBody, viewComment } from "../delivery.js"; -import { mergeableUnder, newestByContext } from "../checks.js"; +import { containedFrom, isCommit, mergeableUnder, newestByContext } from "../checks.js"; /** The forge's container, where its repositories and git are: the note is written there (novox/hq ADR 0239). */ const forgeContainer = process.env.MESH_GITEA_CONTAINER || "gitea"; @@ -492,6 +492,24 @@ export function getGiteaTools(gitea: GiteaClient): ToolDefinition[] { merge: mergeableUnder(base, rule, statuses, unreadable) }; }, }, + { + name: "gitea_contains", + description: "Whether a commit is contained in another — an ancestor of it, or the same — from the forge's compare of the two: what mesh-delivery reads before it says a held merge is covered by a later one (novox/hq ADR 0270). Commits only, by sha or the start of one. Reads only.", + input: { + owner: { type: "string", description: "the repository owner" }, + repo: { type: "string", description: "the repository name" }, + commit: { type: "string", description: "the commit asked about" }, + in: { type: "string", description: "the commit it may be contained in" }, + }, + run: async (args) => { + const owner = String(args.owner ?? "").trim(), repo = String(args.repo ?? "").trim(); + const commit = String(args.commit ?? "").trim(), within = String(args.in ?? "").trim(); + if (!owner || !repo) throw new Error("name the repository: owner and repo"); + if (!isCommit(commit) || !isCommit(within)) throw new Error("commit and in are commits: a sha, or the start of one"); + const { contains, ahead } = containedFrom(await gitea.compare(owner, repo, within, commit)); + return { commit, in: within, contains, ahead }; + }, + }, { name: "gitea_commit_status", description: "Set one of the mesh's statuses on a commit (mesh/delivery, mesh/delivery-group): pending, success, error, failure or warning, a short description, and the page it links to. The merge check's statuses (mesh/merge-gate, mesh/repo-check) take pending only — a recheck putting them back until the fresh verdict sets them.", diff --git a/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go index 62686249..8ec081e4 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go @@ -133,19 +133,33 @@ func TestAMergeALaterDeliveredWalkCarriedIsCovered(t *testing.T) { modules []string // Whether this one was held for a person before the later merge was delivered. heldFirst bool - want State + // What the forge says of the later merge containing this one: yes, no, or away until asked again. + forge string + want State }{ {"carried every module, delivered after this one was held", "main", 35, walkDone, - []string{"build-agent", "mesh-controller"}, true, Covered}, + []string{"build-agent", "mesh-controller"}, true, "yes", Covered}, {"carried every module, delivered before this one was due to be held", "main", 35, walkDone, - []string{"build-agent", "mesh-controller", "route-proxy"}, false, Covered}, - {"carried one of its two modules", "main", 35, walkDone, []string{"mesh-controller"}, true, Held}, - {"merged before this one", "main", 25, walkDone, []string{"build-agent", "mesh-controller"}, true, Held}, - {"merged into another branch", "release", 35, walkDone, []string{"build-agent", "mesh-controller"}, true, Held}, - {"its walk failed", "main", 35, walkFailed, []string{"build-agent", "mesh-controller"}, true, Held}, + []string{"build-agent", "mesh-controller", "route-proxy"}, false, "yes", Covered}, + {"carried one of its two modules", "main", 35, walkDone, []string{"mesh-controller"}, true, "yes", Held}, + {"merged before this one", "main", 25, walkDone, []string{"build-agent", "mesh-controller"}, true, "yes", Held}, + {"merged into another branch", "release", 35, walkDone, []string{"build-agent", "mesh-controller"}, true, "yes", Held}, + {"its walk failed", "main", 35, walkFailed, []string{"build-agent", "mesh-controller"}, true, "yes", Held}, + {"the forge says the later merge does not contain it", "main", 35, walkDone, + []string{"build-agent", "mesh-controller"}, true, "no", Held}, + {"the forge could not say at first, then said it does", "main", 35, walkDone, + []string{"build-agent", "mesh-controller"}, true, "away", Covered}, } { t.Run(c.name, func(t *testing.T) { w := newWorld(t) + pair := merge + " in nnnnnnnnnnnn8888" + switch c.forge { + case "yes", "away": + w.forge.contains[pair] = true + case "no": + w.forge.contains[pair] = false + } + w.forge.containsDown = c.forge == "away" p := pr(ctl, 1, head, "feat/one", "cmd/a.go") w.h.PullUpdated(p) w.h.Checked(verdictMoving(ctl, 1, head, "mesh-controller", "build-agent")) @@ -172,13 +186,25 @@ func TestAMergeALaterDeliveredWalkCarriedIsCovered(t *testing.T) { t.Fatal(err) } w.settleAll() + if c.forge == "away" { + // A forge that cannot say is "not yet covered": held, and asked again a minute on. + if s := w.state(IDOf(ctl, head)); s != Held { + t.Fatalf("with the forge away it is %s", s) + } + w.forge.mu.Lock() + w.forge.containsDown = false + w.forge.mu.Unlock() + w.later(2 * time.Minute) + w.settleAll() + } d := w.delivery(IDOf(ctl, head)) if d.State != c.want { t.Fatalf("it is %s (%s), want %s", d.State, d.HeldWhy, c.want) } s := statusOn(w, ctl, head) if c.want == Covered { - if !strings.HasPrefix(s, "success covered") || !strings.Contains(s, "nnnnnnnn") { + if !strings.HasPrefix(s, "success covered: a later merge that contains it was delivered") || + !strings.Contains(s, "nnnnnnnn") { t.Fatalf("its pull request's mesh/delivery says %q", s) } last := d.Transitions[len(d.Transitions)-1] @@ -189,6 +215,22 @@ func TestAMergeALaterDeliveredWalkCarriedIsCovered(t *testing.T) { if strings.HasPrefix(s, "error") { t.Fatalf("its pull request's mesh/delivery is an error: %q", s) } + // Asked once for the pair, however many ticks read it — once more only after the forge was away. + asked := 0 + for _, a := range w.forge.containsAsks { + if a == pair { + asked++ + } + } + candidate := c.base == "main" && c.at > 30 && c.state == walkDone && len(c.modules) >= 2 + switch { + case !candidate && asked != 0: + t.Fatalf("the forge was asked whether %s for a merge order and walk that cannot cover it", pair) + case candidate && c.forge != "away" && asked != 1: + t.Fatalf("the forge was asked %d time(s) whether %s, want once", asked, pair) + case candidate && c.forge == "away" && asked < 2: + t.Fatalf("the forge, away, was asked %d time(s) whether %s: never again", asked, pair) + } }) } } @@ -212,6 +254,7 @@ func TestACoveredGroupMemberLetsTheNextOneGo(t *testing.T) { w.h.PullMerged(mergedAt(pl, "m2m2m2m2m2m2", t0.Add(31*time.Minute))) w.h.WalkMoved(w.ctl.put(aWalk("plan-lab", lab, "m2m2m2m2m2m2", true))) w.settleAll() + w.forge.contains["m1m1m1m1m1m1 in m3m3m3m3m3m3"] = true deliverLater(w, ctl, 3, "main", "e3e3e3e3e3e3e3e3", "m3m3m3m3m3m3", t0.Add(35*time.Minute), walkDone, "mesh-controller") w.later(20 * time.Minute) if err := w.h.Reconcile(); err != nil { @@ -241,7 +284,7 @@ func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { coverer := func(repo, module string) *Delivery { return &Delivery{ID: IDOf(repo, "c0c0c0c0c0c0c0c0"), Repository: repo, Commit: "c0c0c0c0c0c0c0c0", Number: 90, Base: "main", State: Delivered, Since: merged2.Add(time.Hour), Created: merged2.Add(-time.Hour), - MergedAs: "f0f0f0f0f0f0f0f0", MergedAt: merged2, Plan: planMoving(repo, module), + MergedAs: "f0f0f0f0f0f0f0f0", MergedAt: merged2, Plan: planMoving(repo, module), Check: &Verdict{Gate: "pass"}, Walk: &WalkSeen{ID: "plan-90", Commit: "f0f0f0f0f0f0f0f0", State: walkDone}, Steps: []Step{{Machine: "anchor", Module: module, State: StepPassed}, {Machine: "the rest running it", Module: module, State: StepSent}}} } @@ -252,6 +295,12 @@ func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { } const unwalkedWhy = "merged, and the controller opened no walk for it within 10m0s — nothing it holds follows that branch, " + "or the merge was not heard: a person decides" + withCoverer := func(d *Delivery, change func(c *Delivery)) []*Delivery { + c := coverer(d.Repository, d.Plan.Moved[0]) + change(c) + return []*Delivery{d, c} + } + const uncheckedWhy = "merged without a passing check: only a person decides that it goes on" for _, c := range []struct { name string kept []*Delivery @@ -266,6 +315,24 @@ func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { nil, Delivered}, {"no walk opened, a later delivered walk carried its module", []*Delivery{held(ctl, planMoving(ctl, "mesh-controller"), unwalkedWhy), coverer(ctl, "mesh-controller")}, nil, Covered}, + {"merged without a passing check, a later delivered walk whose own check passed carried its module", + withCoverer(held(ctl, planMoving(ctl, "mesh-controller"), uncheckedWhy), func(*Delivery) {}), nil, Covered}, + {"merged without a passing check, the later delivery's own check did not pass", + withCoverer(held(ctl, planMoving(ctl, "mesh-controller"), uncheckedWhy), func(c *Delivery) { + c.Check = &Verdict{Gate: "fail"} + }), nil, Held}, + {"no walk opened, the later delivery's walk was of another commit", + withCoverer(held(ctl, planMoving(ctl, "mesh-controller"), unwalkedWhy), func(c *Delivery) { + c.Walk.Commit = "e0e0e0e0e0e0e0e0" + }), nil, Held}, + {"no walk opened, the later walk sent its module to its first machine only", + withCoverer(held(ctl, planMoving(ctl, "mesh-controller"), unwalkedWhy), func(c *Delivery) { + c.Steps = c.Steps[:1] + }), nil, Held}, + {"no walk opened, the later walk's branch is another", + withCoverer(held(ctl, planMoving(ctl, "mesh-controller"), unwalkedWhy), func(c *Delivery) { + c.Walk.Branch = "release" + }), nil, Held}, {"no walk opened, a later delivered walk carried another module", []*Delivery{held(ctl, planMoving(ctl, "mesh-controller"), unwalkedWhy), coverer(ctl, "route-proxy")}, nil, Held}, {"held for its group, its own walk superseded, a later delivered walk carried its module", @@ -298,7 +365,9 @@ func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { } } now := merged2.Add(30 * time.Hour) - w := newWorldOver(t, store, newFakeController(), newFakeForge(), now) + forge := newFakeForge() + forge.contains[merge+" in f0f0f0f0f0f0f0f0"] = true + w := newWorldOver(t, store, newFakeController(), forge, now) if err := w.h.Reconcile(); err != nil { t.Fatal(err) } @@ -312,7 +381,7 @@ func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { t.Fatalf("its pull request's mesh/delivery says %q", s) } // A restart over what it kept: nothing moves again. - again := newWorldOver(t, store, newFakeController(), newFakeForge(), now.Add(time.Hour)) + again := newWorldOver(t, store, newFakeController(), forge, now.Add(time.Hour)) if err := again.h.Reconcile(); err != nil { t.Fatal(err) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/effects.go b/modules/mesh-delivery/cmd/mesh-delivery/effects.go index 83fb8c84..84780839 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/effects.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/effects.go @@ -108,7 +108,7 @@ func statusOf(d *Delivery) (string, string) { case Retired: return "success", "history: merged before this owner delivered anything, never walked" case Covered: - return "success", clip("covered: "+d.CoveredBy, 140) + return "success", clip("covered: a later merge that contains it was delivered: "+d.CoveredBy, 140) } return "pending", string(d.State) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index 1ecacccf..26a02a39 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -235,11 +235,15 @@ type fakeForge struct { required []string protectionDown bool asked []string + // contains answers gitea_contains by " in "; a pair it does not hold is a forge that cannot say. + contains map[string]bool + containsDown bool + containsAsks []string } func newFakeForge() *fakeForge { return &fakeForge{notes: map[string][]string{}, views: map[string]string{}, statuses: map[string]string{}, - pulls: map[string]ForgePull{}} + pulls: map[string]ForgePull{}, contains: map[string]bool{}} } func (f *fakeForge) Note(owner, repo, commit, line string) error { @@ -347,6 +351,21 @@ func (f *fakeForge) Statuses(owner, repo string, number int, commit, base string return out, nil } +// Contains answers what the fake was told of two commits. +func (f *fakeForge) Contains(owner, repo, commit, in string) (bool, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.containsAsks = append(f.containsAsks, commit+" in "+in) + if f.containsDown || f.down { + return false, errors.New("the forge is away") + } + yes, said := f.contains[commit+" in "+in] + if !said { + return false, fmt.Errorf("the forge has no commit %s or %s", commit, in) + } + return yes, nil +} + func (f *fakeForge) notesOn(commit string) []string { f.mu.Lock() defer f.mu.Unlock() diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index a5114dce..060c3c2a 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -764,7 +764,13 @@ func (h *Holder) plan() []func() { continue } if d.CoveredBy == "" { - d.CoveredBy = h.coverOf(d) + var ask []string + d.CoveredBy, ask = h.coverOf(d) + for _, in := range ask { + if a := h.askContains(d, in, now); a != nil { + asks = append(asks, a) + } + } } if ts := settle(d, Facts{Now: now}); len(ts) > 0 { h.moved(d, ts) @@ -819,26 +825,42 @@ func (h *Holder) unwalkedPast(d *Delivery, now time.Time) bool { now.Sub(d.MergedAt) > unwalked && h.reconciled.After(d.MergedAt.Add(unwalked)) } -// coverOf says which later merges cover a merged delivery, or nothing (novox/hq issue 319): for every module -// its plan moves, a delivered delivery of the same repository and trunk, merged after it — so its merge commit -// contains this one's, the trunk being protected and only ever merged into — whose finished walk sent that -// module. Read from this owner's own records: the forge's merge times and the walks' records. -func (h *Holder) coverOf(d *Delivery) string { +// coverOf says which later merges cover a merged delivery, or nothing, and which later merge commits the forge +// must still be asked about (novox/hq ADR 0270). A module is covered by a later delivery that carries it (see +// carries) and whose merge commit the forge said contains this one's merge. Merge order is the cheap filter; +// ancestry decides. The forge is asked only once every module has a candidate, once per pair, and its answer +// is kept on the delivery: an answer not known yet is "not yet covered". +func (h *Holder) coverOf(d *Delivery) (string, []string) { if d.MergedAs == "" || d.MergedAt.IsZero() || d.Base == "" || d.Plan == nil || len(d.Plan.Moved) == 0 { - return "" + return "", nil + } + candidates := map[string][]*Delivery{} + for _, module := range d.Plan.Moved { + for _, c := range h.deliveries { + if carries(c, d, module) { + candidates[module] = append(candidates[module], c) + } + } + if len(candidates[module]) == 0 { + return "", nil // nothing later carried it: nothing to ask + } } carried := map[string][]string{} var by []*Delivery + var ask []string for _, module := range d.Plan.Moved { var first *Delivery - for _, c := range h.deliveries { - if carries(c, d, module) && (first == nil || c.MergedAt.Before(first.MergedAt) || - c.MergedAt.Equal(first.MergedAt) && c.ID < first.ID) { + for _, c := range candidates[module] { + yes, known := d.ContainedIn[c.MergedAs] + if !known && !containsString(ask, c.MergedAs) { + ask = append(ask, c.MergedAs) + } + if yes && (first == nil || c.MergedAt.Before(first.MergedAt) || c.MergedAt.Equal(first.MergedAt) && c.ID < first.ID) { first = c } } if first == nil { - return "" + return "", ask } if carried[first.ID] == nil { by = append(by, first) @@ -850,29 +872,67 @@ func (h *Holder) coverOf(d *Delivery) string { for _, c := range by { mods := carried[c.ID] sort.Strings(mods) - parts = append(parts, fmt.Sprintf("delivered with %s (merged as %s, walk %s): %s", c.ID, shortOf(c.MergedAs, 8), + parts = append(parts, fmt.Sprintf("%s (merged as %s into %s, walk %s): %s", c.ID, shortOf(c.MergedAs, 8), d.Base, c.Walk.ID, strings.Join(mods, ", "))) } - return fmt.Sprintf("%s — merged into %s after %s, which it therefore contains", strings.Join(parts, "; "), d.Base, - shortOf(d.MergedAs, 8)) + return strings.Join(parts, "; "), nil } -// carries is whether c delivered a module after d, from a commit that contains d's merge. +// carries is whether c delivered a module after d: of the same repository, merged later into the same trunk, +// delivered on its own passing check — a merge held without one is covered only by a later merge that passed +// its own — by its own walk, of its merge commit on that trunk, which sent the module to every machine running +// it. Whether c's merge commit contains d's is the forge's to say (coverOf). func carries(c, d *Delivery, module string) bool { - if c.ID == d.ID || c.State != Delivered || !strings.EqualFold(c.Repository, d.Repository) || + if c.ID == d.ID || c.State != Delivered || !c.Check.Passes() || !strings.EqualFold(c.Repository, d.Repository) || !strings.EqualFold(c.Base, d.Base) || c.MergedAs == "" || sameCommit(c.MergedAs, d.MergedAs) || - !c.MergedAt.After(d.MergedAt) || c.Walk == nil || c.Walk.State != walkDone { + !c.MergedAt.After(d.MergedAt) || c.Walk == nil || c.Walk.State != walkDone || !sameCommit(c.Walk.Commit, c.MergedAs) || + c.Walk.Branch != "" && !strings.EqualFold(c.Walk.Branch, d.Base) { return false } - if containsString(c.Walk.Sent, module) { - return true - } + everywhere := false for _, s := range c.Steps { - if s.Module == module && (s.State == StepPassed || s.State == StepSent) { - return true + if s.Module != module { + continue + } + switch { + case s.State == StepFailed || s.State == StepRolledBack: + return false + case s.State == StepSent && (s.Machine == stepEveryMachine || s.Machine == stepTheRest): + everywhere = true } } - return false + return everywhere +} + +// askContains is the ask of the forge whether a later merge commit contains a delivery's merge: at most once a +// minute while the forge cannot say, and never again once it said — its answer is kept on the delivery. +func (h *Holder) askContains(d *Delivery, in string, now time.Time) func() { + key := d.ID + " in " + in + if tried, was := h.askTried[key]; was && now.Sub(tried) < time.Minute { + return nil + } + h.askTried[key] = now + id, owner, repo, merged := d.ID, d.Owner(), d.Repo(), d.MergedAs + return func() { + yes, err := h.Forge.Contains(owner, repo, merged, in) + h.mu.Lock() + defer h.mu.Unlock() + if err != nil { + h.Logf("[mesh-delivery] %s: whether %s contains its merge cannot be read yet (%v): not yet covered", id, + shortOf(in, 12), err) + return + } + delete(h.askTried, key) + d := h.deliveries[id] + if d == nil { + return + } + if d.ContainedIn == nil { + d.ContainedIn = map[string]bool{} + } + d.ContainedIn[in] = yes + h.keep(d) + } } // heldForGroup is whether a member is held only because its group's composed check had not passed. diff --git a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go index 05989ba2..43a2c526 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go @@ -65,7 +65,7 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { want = append(want, "seat:"+ControllerSeat+"."+v) } want = append(want, "gitea.gitea_note_append", "gitea.gitea_delivery_view", "gitea.gitea_commit_status", - "gitea.gitea_commit_statuses") + "gitea.gitea_commit_statuses", "gitea.gitea_contains") if !reflect.DeepEqual(m.Invokes, want) { t.Fatalf("invokes %v, wanted %v", m.Invokes, want) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index fa98d32f..ef31ee77 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -65,6 +65,9 @@ type Delivery struct { // (novox/hq issue 319); Lifted why what held it for a person no longer holds (issue 316). CoveredBy string `json:"covered_by,omitempty"` Lifted string `json:"lifted,omitempty"` + // ContainedIn is the forge's answer, by a later merge commit, to whether that commit contains this one's + // merge (gitea_contains): asked once per pair, and kept (ADR 0270). + ContainedIn map[string]bool `json:"contained_in,omitempty"` // Walk is the controller's walk of its trunk commit, as last read; Steps the machines it reached. Walk *WalkSeen `json:"walk,omitempty"` Steps []Step `json:"steps,omitempty"` @@ -343,8 +346,8 @@ type WalkSeen struct { Tier int `json:"tier"` Tiers int `json:"tiers"` Updated time.Time `json:"updated"` - // Sent are the modules the walk sent to a machine, as its record says. - Sent []string `json:"sent,omitempty"` + // Branch is the branch the walk's commit was merged into; empty in a record kept before it was read. + Branch string `json:"branch,omitempty"` } // Seen is a walk as a delivery keeps it. @@ -357,12 +360,7 @@ func (w Walk) Seen() *WalkSeen { s.GoBy = d.By s.StoppedBy = d.Stopped } - for name, m := range w.Modules { - if m != nil && (m.SentAt != nil || m.FirstAt != nil) { - s.Sent = append(s.Sent, name) - } - } - sort.Strings(s.Sent) + s.Branch = w.Branch return s } @@ -379,6 +377,13 @@ func (s *WalkSeen) LetGoByAPerson() bool { return s != nil && strings.HasPrefix( // Open is whether the controller still works it. func (w Walk) Open() bool { return w.State == walkBuilding || w.State == walkRolling } +// The steps that say a module reached every machine running it: sent to all at once, or to the rest after its +// first machines passed. +const ( + stepEveryMachine = "every machine running it" + stepTheRest = "the rest running it" +) + // StepsOf are the machine steps a walk's record says, for one delivery: every module it moved, on its first // machines from its gate, and the rest once sent. func StepsOf(w Walk) []Step { @@ -392,7 +397,7 @@ func StepsOf(w Walk) []Step { m := w.Modules[name] if m == nil || m.FirstAt == nil { if m != nil && m.SentAt != nil && m.FirstAt == nil && m.Why == "" { - out = append(out, Step{Machine: "every machine running it", Module: name, Build: m.Build, + out = append(out, Step{Machine: stepEveryMachine, Module: name, Build: m.Build, State: StepSent, Since: *m.SentAt}) } continue @@ -419,7 +424,7 @@ func StepsOf(w Walk) []Step { out = append(out, Step{Machine: machine, Module: name, Build: m.Build, State: state, Since: since, Why: why}) } if m.SentAt != nil && state == StepPassed { - out = append(out, Step{Machine: "the rest running it", Module: name, Build: m.Build, State: StepSent, + out = append(out, Step{Machine: stepTheRest, Module: name, Build: m.Build, State: StepSent, Since: *m.SentAt}) } } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ports.go b/modules/mesh-delivery/cmd/mesh-delivery/ports.go index 8b775503..be2bb752 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ports.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ports.go @@ -76,6 +76,8 @@ type Forge interface { // Statuses reads a commit's statuses — a pull request's head's, given its number — and whether the branch // it merges into would let it merge. Reads only. Statuses(owner, repo string, number int, commit, base string) (*ForgeChecks, error) + // Contains is whether a commit is contained in another — an ancestor of it, or it. Reads only. + Contains(owner, repo, commit, in string) (bool, error) } // ForgeChecks is what the forge says of a commit's checks (the gitea module's gitea_commit_statuses). @@ -337,6 +339,24 @@ func (f toolForge) Statuses(owner, repo string, number int, commit, base string) return &c, nil } +func (f toolForge) Contains(owner, repo, commit, in string) (bool, error) { + raw, err := f.ask("gitea.gitea_contains", map[string]any{"owner": owner, "repo": repo, "commit": commit, "in": in}) + if err != nil { + return false, err + } + answer, output, ok := answerOf(raw) + if !ok { + return false, fmt.Errorf("the forge refused gitea_contains: %s", lastLine(output)) + } + var a struct { + Contains *bool `json:"contains"` + } + if err := json.Unmarshal(answer, &a); err != nil || a.Contains == nil { + return false, fmt.Errorf("the forge's gitea_contains is not readable: %s", clip(string(answer), 200)) + } + return *a.Contains, nil +} + // kvStore is the module's own state: `deliveries` and `groups`, one key each. type kvStore struct{} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table.go b/modules/mesh-delivery/cmd/mesh-delivery/table.go index df43b3e3..e554cabf 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table.go @@ -34,10 +34,11 @@ const ( // replayed from the bus when it first held the seat. It reached the machines before this owner existed, // and nothing is delivered, released or put back for it (novox/hq issue 313). Retired State = "retired" - // Covered is a merge whose change reached the machines through a later one: every module it moves was - // sent by a finished walk of a commit merged after it into the same trunk, which therefore contains it. It - // is not walked itself, and it is no failure: its pull request says covered, as a success (novox/hq issue - // 319). + // Covered is a merge a later one that contains it delivered: every module it moves was sent to every machine + // running it by the finished walk of a later merge into the same trunk, whose own check passed and which the + // forge says contains this one. It is not walked itself, and it is no failure: its pull request says covered, + // as a success (novox/hq ADR 0270). A revert merged in between is still possible: covered says what was + // delivered, not what runs. Covered State = "covered" ) @@ -182,8 +183,9 @@ var Table = []Row{ }, Says: func(d *Delivery) string { return "merged, and nothing for a walk to move: " + d.Plan.Summary }}, {From: []State{Ready, Held}, Event: EvCovered, To: Covered, - Guard: "merged, no walk of its own open, and every module it moves sent by a finished walk of a commit merged " + - "after it into the same trunk — which contains it, the trunk being protected: its change already runs", + Guard: "merged, no walk of its own open, and every module it moves sent to every machine running it by the " + + "finished walk of a later merge into the same trunk, whose own check passed and which the forge says contains " + + "it: a later merge that contains it was delivered", Holds: func(d *Delivery, f Facts) error { switch { case d.MergedAs == "" || d.CoveredBy == "": @@ -193,7 +195,9 @@ var Table = []Row{ } return nil }, - Says: func(d *Delivery) string { return "covered: " + d.CoveredBy }}, + Says: func(d *Delivery) string { + return "covered: a later merge that contains it was delivered: " + d.CoveredBy + }}, {From: []State{Proposed, Checked, Rejected}, Event: EvMergedUnchecked, To: Held, Guard: "merged without a passing check — a verdict known with it is taken first: only a person decides that it goes on", Holds: func(d *Delivery, f Facts) error { return unless(d.MergedAs != "", "not merged") }}, diff --git a/modules/mesh-delivery/module.json b/modules/mesh-delivery/module.json index 570e766f..a165b473 100644 --- a/modules/mesh-delivery/module.json +++ b/modules/mesh-delivery/module.json @@ -43,7 +43,8 @@ "gitea.gitea_note_append", "gitea.gitea_delivery_view", "gitea.gitea_commit_status", - "gitea.gitea_commit_statuses" + "gitea.gitea_commit_statuses", + "gitea.gitea_contains" ], "state": [ "deliveries",