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/test/token.test.ts b/modules/gitea/test/token.test.ts index b69edd9b..c6f3b4a7 100644 --- a/modules/gitea/test/token.test.ts +++ b/modules/gitea/test/token.test.ts @@ -411,7 +411,7 @@ test("the tools register once there is a way to a token, and the first call mint "gitea_list_branches", "gitea_delete_branch", "gitea_branch_protection_get", "gitea_branch_protection_set", - "gitea_note_append", "gitea_delivery_view", "gitea_commit_status", + "gitea_note_append", "gitea_delivery_view", "gitea_commit_statuses", "gitea_contains", "gitea_commit_status", "gitea_list_labels", "gitea_create_label", "gitea_api", ], 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 new file mode 100644 index 00000000..8ec081e4 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go @@ -0,0 +1,476 @@ +package main + +import ( + "fmt" + "strings" + "testing" + "time" +) + +// novox/hq issues 319 and 316: a held delivery whose change already runs, or that builds nothing, is not a +// question for a person. A merge that moves nothing is delivered at once; a merge whose every module a later +// merge on the same trunk delivered is covered — never stopped, never an error on its pull request; and a +// group member held for a composed check that had not answered goes on once it passes. + +var t0 = time.Date(2026, 10, 6, 12, 0, 0, 0, time.UTC) + +// mergedAt is the forge's `pull.merged` at a moment. +func mergedAt(p PullEvent, merge string, at time.Time) PullEvent { + p.MergeCommit = merge + p.MergedAt = at.UTC().Format(time.RFC3339) + return p +} + +// verdictMoving is a passing verdict whose plan moves these modules — or builds nothing, given none. +func verdictMoving(repo string, number int, sha string, modules ...string) CheckedEvent { + c := verdict(repo, number, sha, "pass") + c.Plan = planMoving(repo, modules...) + return c +} + +func planMoving(repo string, modules ...string) *DeliveryPlan { + if len(modules) == 0 { + return &DeliveryPlan{Repository: repo, Base: "main", Summary: "builds nothing: the change touches no module of the mesh's graph"} + } + return &DeliveryPlan{Repository: repo, Base: "main", Moved: modules, Summary: "builds " + strings.Join(modules, ", ") + " → anchor"} +} + +// deliverLater is a later merge into a trunk, walked: its walk opened for its merge commit, let go, and left +// in the state given, every module of it sent. +func deliverLater(w *world, repo string, number int, base, headSHA, merge string, at time.Time, state string, + modules ...string) string { + w.t.Helper() + p := pr(repo, number, headSHA, fmt.Sprintf("feat/later-%d", number)) + p.Base = base + w.h.PullUpdated(p) + w.h.Checked(verdictMoving(repo, number, headSHA, modules...)) + w.h.PullMerged(mergedAt(p, merge, at)) + walk := aWalk("plan-"+merge[:6], repo, merge, true) + walk.Branch = base + walk.Modules = map[string]*WalkModule{} + for _, m := range modules { + walk.Modules[m] = &WalkModule{} + } + w.h.WalkMoved(w.ctl.put(walk)) + w.settleAll() + walk = w.ctl.walk(walk.ID) + sent := at.Add(5 * time.Minute) + for _, m := range modules { + walk.Modules[m] = &WalkModule{State: "built", SentAt: &sent} + } + walk.State = state + w.h.WalkMoved(w.ctl.put(walk)) + w.settleAll() + return IDOf(repo, headSHA) +} + +func statusOn(w *world, repo, commit string) string { + w.forge.mu.Lock() + defer w.forge.mu.Unlock() + return w.forge.statuses[repo+"@"+commit+" mesh/delivery"] +} + +// **A merge that moves nothing is delivered at once** (issue 319): there is nothing for a walk to move and +// nothing for a person to decide — whether its check passed, failed or never answered, and whether its plan +// builds nothing at all or only a new module the controller builds and sends nowhere (issue 300). +func TestAMergeThatMovesNothingIsDeliveredAtOnce(t *testing.T) { + for _, c := range []struct { + name string + verdict string // "" is a merge before any verdict + plan *DeliveryPlan + }{ + {"builds nothing, merged before its check answered", "", planMoving("novox/hq")}, + {"builds nothing, merged on a failed check", "fail", planMoving("novox/hq")}, + {"builds nothing, merged on a passing check", "pass", planMoving("novox/hq")}, + {"a new module, sent nowhere", "pass", &DeliveryPlan{Repository: "novox/hq", New: []string{"modules/zfs"}, + Summary: "builds new: modules/zfs, sent nowhere; no bus step"}}, + {"a new module, sent nowhere, merged before its check answered", "", &DeliveryPlan{Repository: "novox/hq", + New: []string{"modules/zfs"}, Summary: "builds new: modules/zfs, sent nowhere; no bus step"}}, + } { + t.Run(c.name, func(t *testing.T) { + w := newWorld(t) + w.ctl.planned = c.plan + p := pr("novox/hq", 7, head, "docs/x", "02-DECISIONS/x.md") + w.h.PullUpdated(p) + if c.verdict != "" { + v := verdict("novox/hq", 7, head, c.verdict) + v.Plan = c.plan + w.h.Checked(v) + } + w.later(31 * time.Minute) + w.h.PullMerged(mergedAt(p, merge, t0.Add(30*time.Minute))) + w.settleAll() + // Past the bound a merge waits for its walk: nothing holds it then either. + w.later(15 * time.Minute) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + d := w.delivery(IDOf("novox/hq", head)) + if d.State != Delivered { + t.Fatalf("a merge whose plan is %q is %s (%s), not delivered", c.plan.Summary, d.State, d.HeldWhy) + } + if s := statusOn(w, "novox/hq", head); !strings.HasPrefix(s, "success") { + t.Fatalf("its pull request's mesh/delivery says %q", s) + } + }) + } +} + +// **A held delivery whose every module a later merge on its trunk delivered is covered** (issue 319): the +// later merge commit contains it, its walk is done, and it sent each module this one moves. Anything short +// of that — a module not carried, an earlier merge, another trunk, a walk that did not finish — leaves it held +// for a person, as before. +func TestAMergeALaterDeliveredWalkCarriedIsCovered(t *testing.T) { + const ctl = "novox/mesh-controller" + for _, c := range []struct { + name string + // The later merge: its trunk, minutes after t0 it merged (this one merged at 30), its walk's end, and + // what it sent. + base string + at int + state string + modules []string + // Whether this one was held for a person before the later merge was delivered. + heldFirst bool + // 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, "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, "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")) + w.later(31 * time.Minute) + w.h.PullMerged(mergedAt(p, merge, t0.Add(30*time.Minute))) + w.settleAll() + if c.heldFirst { + w.later(15 * time.Minute) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + if s := w.state(IDOf(ctl, head)); s != Held { + t.Fatalf("a merge no walk opened for is %s before the later one", s) + } + } + later := deliverLater(w, ctl, 2, c.base, "bbbbbbbbbbbb2222", "nnnnnnnnnnnn8888", t0.Add(time.Duration(c.at)*time.Minute), + c.state, c.modules...) + if c.state == walkDone && w.state(later) != Delivered { + t.Fatalf("the later merge is %s", w.state(later)) + } + w.later(20 * time.Minute) + if err := w.h.Reconcile(); err != nil { + 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: 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] + if !strings.Contains(last.Why, IDOf(ctl, "bbbbbbbbbbbb2222")) { + t.Fatalf("its transition does not name what covered it: %q", last.Why) + } + } + 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) + } + }) + } +} + +// A covered member of a group counts as delivered: the member after it is let go, not stopped. +func TestACoveredGroupMemberLetsTheNextOneGo(t *testing.T) { + w := newWorld(t) + const ctl, lab = "novox/mesh-controller", "novox/mesh-lab" + const ctlHead, labHead = "c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2" + ctlID, labID := IDOf(ctl, ctlHead), IDOf(lab, labHead) + w.ctl.order = func(ms []Member) Order { return Order{Order: []string{ctlID, labID}} } + pc, pl := pr(ctl, 1, ctlHead, "feat/g", "cmd/a.go"), pr(lab, 2, labHead, "feat/g", "replays/a.go") + w.h.PullUpdated(pc) + w.h.PullUpdated(pl) + w.h.Checked(verdictMoving(ctl, 1, ctlHead, "mesh-controller")) + w.h.Checked(verdictMoving(lab, 2, labHead, "mesh-lab")) + w.settleAll() + w.h.Checked(CheckedEvent{Group: "feat/g", ID: "check-1", Verdict: "pass", Summary: "both compose"}) + w.later(31 * time.Minute) + w.h.PullMerged(mergedAt(pc, "m1m1m1m1m1m1", t0.Add(30*time.Minute))) + 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 { + t.Fatal(err) + } + w.settleAll() + if w.state(ctlID) != Covered { + t.Fatalf("the controller's member is %s", w.state(ctlID)) + } + if w.state(labID) != Delivering || !containsString(w.ctl.delivers, "plan-lab") { + t.Fatalf("the member after it is %s, let go %v", w.state(labID), w.ctl.delivers) + } + lw := w.ctl.walk("plan-lab") + lw.State = walkDone + w.h.WalkMoved(w.ctl.put(lw)) + w.settleAll() + if state, why := GroupState(w.h.groups["feat/g"], w.h.membersOf(w.h.groups["feat/g"])); state != "delivered" { + t.Fatalf("the group is %s: %s", state, why) + } +} + +// **On rollout, the deliveries already held close by themselves** (issue 319's held ones), and a restart +// after that moves nothing again: what was read back is where it stays. +func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { + const ctl, cat = "novox/mesh-controller", "novox/mesh-catalog" + merged1, merged2 := t0, t0.Add(2*time.Hour) + 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), 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}}} + } + held := func(repo string, plan *DeliveryPlan, why string) *Delivery { + return &Delivery{ID: IDOf(repo, head), Repository: repo, Commit: head, Number: 7, Base: "main", State: Held, + Since: merged1.Add(10 * time.Minute), Created: merged1.Add(-time.Hour), MergedAs: merge, MergedAt: merged1, + HeldWhy: why, Plan: plan, Transitions: []Transition{{At: merged1, From: Ready, To: Held, Event: EvHold, Why: why}}} + } + 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 + group *Group + want State + }{ + {"merged without a passing check, builds nothing", + []*Delivery{held("novox/hq", planMoving("novox/hq"), "merged without a passing check: only a person decides that it goes on")}, + nil, Delivered}, + {"a new module sent nowhere, no walk opened", + []*Delivery{held(cat, &DeliveryPlan{New: []string{"modules/zfs"}, Summary: "builds new: modules/zfs, sent nowhere"}, unwalkedWhy)}, + 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", + func() []*Delivery { + d := held(cat, planMoving(cat, "systemd"), "its group feat/j's composed check did not pass for the heads that "+ + "merged; a person decides that it goes on") + d.Group = "feat/j" + d.Walk = &WalkSeen{ID: "plan-7", Commit: merge, State: walkSuperseded, Waits: true, WaitedFor: "mesh-delivery"} + other := &Delivery{ID: IDOf("novox/mesh-host", "abababababababab"), Repository: "novox/mesh-host", + Commit: "abababababababab", Number: 3, Base: "main", Group: "feat/j", State: Delivered, + MergedAs: "acacacacacacacac", MergedAt: merged1} + return []*Delivery{d, other, coverer(cat, "systemd")} + }(), + &Group{ID: "feat/j", Members: []string{IDOf(cat, head), IDOf("novox/mesh-host", "abababababababab")}, Closed: true, + Order: []string{IDOf(cat, head), IDOf("novox/mesh-host", "abababababababab")}, + Check: &GroupCheck{Asked: "check-1", Verdict: "pass", Commits: map[string]string{IDOf(cat, head): head, + IDOf("novox/mesh-host", "abababababababab"): "abababababababab"}}}, + Covered}, + } { + t.Run(c.name, func(t *testing.T) { + store := newMemStore() + for _, d := range c.kept { + if err := store.PutDelivery(d); err != nil { + t.Fatal(err) + } + } + if c.group != nil { + if err := store.PutGroup(c.group); err != nil { + t.Fatal(err) + } + } + now := merged2.Add(30 * time.Hour) + 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) + } + w.settleAll() + id := c.kept[0].ID + d := w.delivery(id) + if d.State != c.want { + t.Fatalf("read back held, it is %s, want %s", d.State, c.want) + } + if s := statusOn(w, d.Repository, d.Commit); c.want != Held && !strings.HasPrefix(s, "success") { + 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(), forge, now.Add(time.Hour)) + if err := again.h.Reconcile(); err != nil { + t.Fatal(err) + } + again.settleAll() + if d2 := again.delivery(id); d2.State != d.State || len(d2.Transitions) != len(d.Transitions) { + t.Fatalf("a restart moved it again: %s with %d transitions, was %s with %d", d2.State, len(d2.Transitions), + d.State, len(d.Transitions)) + } + }) + } +} + +// **A group member is not held for a composed check that has not answered** (issue 316): it waits published, +// and goes on once the check passes for the heads that merged. +func TestAGroupMemberWaitsForItsComposedCheckToAnswer(t *testing.T) { + w := newWorld(t) + const a, b = "novox/a", "novox/b" + const aHead, bHead = "a1a1a1a1a1a1a1a1", "b2b2b2b2b2b2b2b2" + aID := IDOf(a, aHead) + pa, pb := pr(a, 1, aHead, "feat/k", "x"), pr(b, 2, bHead, "feat/k", "y") + w.h.PullUpdated(pa) + w.h.PullUpdated(pb) + w.h.Checked(verdictMoving(a, 1, aHead, "a")) + w.h.Checked(verdictMoving(b, 2, bHead, "b")) + w.settleAll() + if len(w.ctl.checks) != 1 { + t.Fatalf("the composed check was asked %v", w.ctl.checks) + } + w.h.PullMerged(mergedAt(pa, "m1m1m1m1m1m1", t0)) + w.h.WalkMoved(w.ctl.put(aWalk("plan-a", a, "m1m1m1m1m1m1", true))) + w.settleAll() + if s := w.state(aID); s != Published { + t.Fatalf("merged while its group's composed check had not answered, it is %s (%s)", s, w.delivery(aID).HeldWhy) + } + w.h.Checked(CheckedEvent{Group: "feat/k", ID: "check-1", Verdict: "pass", Summary: "both compose"}) + w.settleAll() + if s := w.state(aID); s != Delivering || !containsString(w.ctl.delivers, "plan-a") { + t.Fatalf("its composed check passed, and it is %s, let go %v", s, w.ctl.delivers) + } +} + +// **A member already held for its group's composed check goes on when that check passes for the heads that +// merged** (issue 316); a check that failed, or passed for other heads, keeps it held for a person. +func TestAMemberHeldForItsComposedCheckIsLiftedWhenItPasses(t *testing.T) { + const a, b = "novox/a", "novox/b" + const aHead, bHead = "a1a1a1a1a1a1a1a1", "b2b2b2b2b2b2b2b2" + aID, bID := IDOf(a, aHead), IDOf(b, bHead) + for _, c := range []struct { + name string + verdict string + commits map[string]string + want State + }{ + {"passed for the heads that merged", "pass", map[string]string{aID: aHead, bID: bHead}, Delivering}, + {"warned for the heads that merged", "warning", map[string]string{aID: aHead, bID: bHead}, Delivering}, + {"failed", "fail", map[string]string{aID: aHead, bID: bHead}, Held}, + {"passed for other heads", "pass", map[string]string{aID: "a9a9a9a9a9a9a9a9", bID: bHead}, Held}, + } { + t.Run(c.name, func(t *testing.T) { + store := newMemStore() + why := "its group feat/k's composed check did not pass for the heads that merged; a person decides that it goes on" + da := &Delivery{ID: aID, Repository: a, Commit: aHead, Number: 1, Base: "main", Branch: "feat/k", Group: "feat/k", + State: Held, Since: t0, Created: t0.Add(-time.Hour), MergedAs: "m1m1m1m1m1m1", MergedAt: t0, HeldWhy: why, + Plan: planMoving(a, "a"), Check: &Verdict{Gate: "pass"}, + Walk: &WalkSeen{ID: "plan-a", Commit: "m1m1m1m1m1m1", State: walkBuilding, Waits: true, WaitedFor: "mesh-delivery"}} + db := &Delivery{ID: bID, Repository: b, Commit: bHead, Number: 2, Base: "main", Branch: "feat/k", Group: "feat/k", + State: Ready, Since: t0, Created: t0.Add(-time.Hour), Plan: planMoving(b, "b"), Check: &Verdict{Gate: "pass"}} + g := &Group{ID: "feat/k", Members: []string{aID, bID}, Order: []string{aID, bID}, Closed: true, + OrderOf: map[string]string{aID: aHead, bID: bHead}, + Check: &GroupCheck{Asked: "check-1", At: t0, Commits: c.commits}} + for _, d := range []*Delivery{da, db} { + _ = store.PutDelivery(d) + } + _ = store.PutGroup(g) + ctl := newFakeController() + ctl.put(aWalk("plan-a", a, "m1m1m1m1m1m1", true)) + w := newWorldOver(t, store, ctl, newFakeForge(), t0.Add(time.Minute)) + w.settleAll() + if s := w.state(aID); s != Held { + t.Fatalf("before its composed verdict it is %s", s) + } + w.h.Checked(CheckedEvent{Group: "feat/k", ID: "check-1", Verdict: c.verdict, Summary: "composed: " + c.verdict}) + w.settleAll() + if s := w.state(aID); s != c.want { + t.Fatalf("its composed check %s, and it is %s (%s), want %s", c.name, s, w.delivery(aID).HeldWhy, c.want) + } + if c.want == Delivering && !containsString(ctl.delivers, "plan-a") { + t.Fatalf("lifted, its walk was not let go: %v", ctl.delivers) + } + }) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/effects.go b/modules/mesh-delivery/cmd/mesh-delivery/effects.go index b2522fe8..84780839 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/effects.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/effects.go @@ -107,6 +107,8 @@ func statusOf(d *Delivery) (string, string) { return "error", clip("stopped: "+why, 140) case Retired: return "success", "history: merged before this owner delivered anything, never walked" + case Covered: + 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 73506256..26a02a39 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -95,6 +95,8 @@ type fakeController struct { down bool // onCheck runs as a check is asked, before it is answered: what the forge held at that moment. onCheck func() + // planned is what Plan answers instead of "builds app", when set. + planned *DeliveryPlan } func newFakeController() *fakeController { @@ -108,6 +110,10 @@ func (c *fakeController) Plan(repository, base, head string, paths, dirs, remove if c.down { return nil, errors.New("down") } + if c.planned != nil { + p := *c.planned + return &p, nil + } return &DeliveryPlan{Repository: repository, Base: base, Head: head, Moved: []string{"app"}, Summary: "builds app"}, nil } func (c *fakeController) Order(ms []Member) (Order, error) { @@ -229,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 { @@ -341,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 84e96b68..060c3c2a 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -160,7 +160,7 @@ func (d *Delivery) owe(e Effect) { func afterTheTrunk(s State) bool { return s == Published || s == Delivering || s == Held || s == Delivered || s == Failed || s == Stopped || - s == Superseded || s == Retired + s == Superseded || s == Retired || s == Covered } func orNothing(s State) string { @@ -755,10 +755,32 @@ func (h *Holder) plan() []func() { } } + // Held, or merged with no walk opened for it: ended by what is already true of it, never asked of a person + // (novox/hq issue 319). A plan that moves nothing is delivered; one whose every module a later merge into the + // same trunk delivered is covered. Read again each tick, so a delivery read back after a restart, or held + // before this was known, ends by itself. + for _, d := range h.deliveries { + if d.State != Held && !h.unwalkedPast(d, now) { + continue + } + if d.CoveredBy == "" { + 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) + h.closeGroupOf(d) + } + } + // Merged, and no walk opened for it after the walks were read again: never silent — held, saying so. for _, d := range h.deliveries { - if d.State == Ready && d.MergedAs != "" && d.Walk == nil && !d.MovesNothing() && d.HeldWhy == "" && - now.Sub(d.MergedAt) > unwalked && h.reconciled.After(d.MergedAt.Add(unwalked)) { + if h.unwalkedPast(d, now) { d.HeldWhy = "merged, and the controller opened no walk for it within " + unwalked.String() + " — nothing it " + "holds follows that branch, or the merge was not heard: a person decides" h.settleAndKeep(d) @@ -796,6 +818,128 @@ func (h *Holder) plan() []func() { return asks } +// unwalkedPast is whether a merged delivery has gone without a walk past the bound one opens within, read +// again after it: what holds it for a person, unless something already ends it. +func (h *Holder) unwalkedPast(d *Delivery, now time.Time) bool { + return d.State == Ready && d.MergedAs != "" && d.Walk == nil && !d.MovesNothing() && d.HeldWhy == "" && + now.Sub(d.MergedAt) > unwalked && h.reconciled.After(d.MergedAt.Add(unwalked)) +} + +// 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 "", 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 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 "", ask + } + if carried[first.ID] == nil { + by = append(by, first) + } + carried[first.ID] = append(carried[first.ID], module) + } + sort.Slice(by, func(i, j int) bool { return by[i].MergedAt.Before(by[j].MergedAt) }) + var parts []string + for _, c := range by { + mods := carried[c.ID] + sort.Strings(mods) + 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 strings.Join(parts, "; "), nil +} + +// 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 || !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 || !sameCommit(c.Walk.Commit, c.MergedAs) || + c.Walk.Branch != "" && !strings.EqualFold(c.Walk.Branch, d.Base) { + return false + } + everywhere := false + for _, s := range c.Steps { + 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 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. +func heldForGroup(d *Delivery, g *Group) bool { + return d.State == Held && strings.HasPrefix(d.HeldWhy, fmt.Sprintf("its group %s's composed check", g.ID)) +} + // askCheck is the ask of a proposed delivery's own check, when nothing else will bring its verdict: no verdict, // no check this owner asked, and proposed past the grace the controller's own check of its announcement is // given. Nil otherwise — once asked, a late verdict is the bound's and H2's to speak for. @@ -1031,7 +1175,7 @@ func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() { for i, m := range ordered { d := byID[m.ID] switch { - case d.State == Delivered || d.State == Retired: + case d.State.Reached(): continue case d.State == Failed || d.State == Stopped || d.State == Superseded: var asks []func() @@ -1041,7 +1185,27 @@ func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() { } } return asks + case d.State == Held: + // Held for a composed check that has since passed for the heads that merged: lifted, published again, + // and its turn taken in order (novox/hq issue 316). Any other hold is a person's. + if composed && heldForGroup(d, g) && walkCanGoOn(d) { + d.HeldWhy = "" + d.Lifted = fmt.Sprintf("its group %s's composed check said %s for the heads that merged: %s", g.ID, + g.Check.Verdict, g.Check.Summary) + h.settleAndKeep(d) + if d.State == Published { + if a := h.turn(d, fmt.Sprintf("its turn in group %s (%d of %d)", g.ID, i+1, len(ordered))); a != nil { + return []func(){a} + } + } + } + return nil case d.State == Published: + // Asked and not answered is not "did not pass": it waits, published, for the answer (issue 316). + if !composed && d.HeldWhy == "" && g.Check != nil && g.Check.Verdict == "" && + sameMembers(g.Check.Commits, commitsOf(members)) { + return nil + } if !composed && d.HeldWhy == "" { d.HeldWhy = fmt.Sprintf("its group %s's composed check did not pass for the heads that merged; a person "+ "decides that it goes on", g.ID) @@ -1093,10 +1257,10 @@ func GroupState(g *Group, members []*Delivery) (string, string) { return "failed", "a member failed" case count[Stopped] > 0 && g.Closed: return "stopped", "a member was stopped" - case n > 0 && count[Delivered]+count[Retired] == n: + case n > 0 && count[Delivered]+count[Retired]+count[Covered] == n: return "delivered", "every member is delivered" case g.Closed: - return "delivering", fmt.Sprintf("%d of %d delivered", count[Delivered], n) + return "delivering", fmt.Sprintf("%d of %d delivered", count[Delivered]+count[Covered], n) case len(g.Cycle) > 0: return "rejected", cycleSaid(g) case count[Rejected] > 0: 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 c778cbed..ef31ee77 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -61,6 +61,13 @@ type Delivery struct { // History is why it is history, once it is (novox/hq issue 313). FromForge bool `json:"from_forge,omitempty"` History string `json:"history,omitempty"` + // CoveredBy says which later merges into the same trunk delivered every module it moves, once they did + // (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"` @@ -144,10 +151,11 @@ type MachinePlan struct { Waits []string `json:"waits,omitempty"` } -// MovesNothing is whether the delivery's plan moves no module and adds none: nothing for a walk to do. A -// delivery with no plan known is not said to move nothing. +// MovesNothing is whether the delivery's plan moves no module the mesh holds: nothing for a walk to do. A +// module it adds is not moved: the controller builds and registers it and sends it nowhere, and opens no walk +// for it (novox/hq issues 300 and 319). A delivery with no plan known is not said to move nothing. func (d *Delivery) MovesNothing() bool { - return d.Plan != nil && len(d.Plan.Moved)+len(d.Plan.New) == 0 + return d.Plan != nil && len(d.Plan.Moved) == 0 } // Step is one machine's part of a delivering walk. @@ -338,6 +346,8 @@ type WalkSeen struct { Tier int `json:"tier"` Tiers int `json:"tiers"` Updated time.Time `json:"updated"` + // 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. @@ -350,6 +360,7 @@ func (w Walk) Seen() *WalkSeen { s.GoBy = d.By s.StoppedBy = d.Stopped } + s.Branch = w.Branch return s } @@ -366,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 { @@ -379,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 @@ -406,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 d6e1d7dd..e554cabf 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table.go @@ -34,17 +34,27 @@ 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 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" ) // AllStates are every state a delivery can be in, for the table's own test and its verb. var AllStates = []State{Proposed, Checked, Ready, Rejected, Published, Delivering, Held, Delivered, Failed, - Superseded, Stopped, Retired} + Superseded, Stopped, Retired, Covered} // Final is whether nothing follows a state. func (s State) Final() bool { - return s == Delivered || s == Failed || s == Superseded || s == Stopped || s == Retired + return s == Delivered || s == Failed || s == Superseded || s == Stopped || s == Retired || s == Covered } +// Reached is whether a final state says the change reached the machines, or had nothing to reach them +// with: what a group counts as delivered. +func (s State) Reached() bool { return s == Delivered || s == Retired || s == Covered } + // Event is what moves a delivery. type Event string @@ -72,6 +82,8 @@ const ( EvStop Event = "stop" // a person stopped it, or its group did EvHistory Event = "history" // made from the forge's word for a merge long past: history EvRetire Event = "retire" // a person's word that a held delivery is history, never to be walked + EvCovered Event = "covered" // a later merge into the same trunk delivered every module it moves + EvLifted Event = "lifted" // what held it for a person is no longer so: its group's composed check passed ) // Facts are what a guard reads beyond the delivery itself: the moment, and who asks, for an act. @@ -91,6 +103,8 @@ type Row struct { Holds func(d *Delivery, f Facts) error // Act says the row is taken only when asked — a person's or H2's — never by settle. Act bool + // Says is the transition's why when nobody gave one: the fact that moved it, rather than the guard. + Says func(d *Delivery) string } func unless(cond bool, why string) error { @@ -161,6 +175,29 @@ var Table = []Row{ {From: []State{Checked}, Event: EvRefused, To: Rejected, Guard: "the gate failed or could not run, or the repository's own check failed", Holds: func(d *Delivery, f Facts) error { return unless(!d.Check.Passes(), "the verdict passes") }}, + {From: []State{Proposed, Checked, Rejected, Held}, Event: EvDone, To: Delivered, + Guard: "merged, and its plan builds nothing for a walk to move — nothing at all, or only a new module the " + + "controller builds and sends nowhere: nothing to deliver and nothing for a person to decide, whatever its check said", + Holds: func(d *Delivery, f Facts) error { + return unless(d.MergedAs != "" && d.MovesNothing() && d.Walk == nil, "not merged, or a walk moves it") + }, + 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 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 == "": + return errors.New("not merged, or nothing later carried its modules") + case d.Walk != nil && (d.Walk.State == walkBuilding || d.Walk.State == walkRolling): + return fmt.Errorf("its own walk %s is open: the controller delivers it", d.Walk.ID) + } + return nil + }, + 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") }}, @@ -208,6 +245,13 @@ var Table = []Row{ return unless(d.Walk != nil && d.Walk.Started() && (!d.Walk.Waited() || d.Walk.LetGoByAPerson()), "its walk was not started by a person or the controller's own path") }}, + {From: []State{Held}, Event: EvLifted, To: Published, + Guard: "held only for its group's composed check, which has since passed or warned for the heads that merged: " + + "it is published again and waits for its turn", + Holds: func(d *Delivery, f Facts) error { + return unless(d.Lifted != "" && d.HeldWhy == "" && d.MergedAs != "" && walkCanGoOn(d), "nothing lifted its hold") + }, + Says: func(d *Delivery) string { return "lifted: " + d.Lifted }}, {From: []State{Held}, Event: EvRelease, To: Delivering, Act: true, Guard: "a person's decision, with why", Holds: func(d *Delivery, f Facts) error { @@ -240,6 +284,15 @@ var Table = []Row{ Holds: func(d *Delivery, f Facts) error { return unless(f.Why != "" && f.By != "", "a stop says who and why") }}, } +// walkCanGoOn is whether a delivery published again would be delivered by what it has: its own walk still +// open, or nothing for a walk to move. +func walkCanGoOn(d *Delivery) bool { + if d.Walk == nil { + return d.MovesNothing() + } + return d.Walk.State == walkBuilding || d.Walk.State == walkRolling +} + // historyLate is how long after its merge a merge heard only from the forge's word is history: the forge // announces a merge within seconds, so one heard this late was replayed from the bus's history. const historyLate = 15 * time.Minute @@ -309,6 +362,9 @@ func Apply(d *Delivery, ev Event, act bool, f Facts) (Transition, error) { return Transition{}, ErrRefused{ID: d.ID, From: d.State, Event: ev, Why: "its guard does not hold: " + err.Error()} } why := f.Why + if why == "" && r.Says != nil { + why = r.Says(d) + } if why == "" { why = r.Guard } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table_test.go b/modules/mesh-delivery/cmd/mesh-delivery/table_test.go index 3ea121c3..73cbde8e 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table_test.go @@ -55,11 +55,19 @@ func aDeliveryFor(r Row) (*Delivery, Facts) { case EvHold: d.HeldWhy, d.MergedAs = "its group's check did not pass", "aaaa1111" case EvDone: - if r.From[0] == Published { + switch { + case r.From[0] == Published: d.Plan = &DeliveryPlan{Summary: "moves nothing"} - } else { + case r.From[0] == Proposed: + d.MergedAs, d.Plan = "aaaa1111", &DeliveryPlan{Summary: "builds nothing"} + default: d.Walk = &WalkSeen{ID: "plan-1", State: walkDone} } + case EvCovered: + d.MergedAs, d.CoveredBy = "aaaa1111", "delivered with novox/app@bbbb (merged as bbbb2222, walk plan-2): app" + case EvLifted: + d.MergedAs, d.Lifted = "aaaa1111", "its group's composed check passed" + d.Walk = &WalkSeen{ID: "plan-1", State: walkBuilding, Waits: true, WaitedFor: "mesh-delivery"} case EvFailed: d.Walk = &WalkSeen{ID: "plan-1", State: walkFailed} case EvSuperseded: @@ -104,7 +112,7 @@ func TestEveryRowOfTheTableIsTakenWhenItsGuardHoldsAndRefusedWhenNot(t *testing. func TestEveryPairTheTableDoesNotHoldIsRefusedByName(t *testing.T) { events := []Event{EvAnnounced, EvAppeared, EvAdopted, EvChecked, EvAccepted, EvRefused, EvRecheck, EvReask, EvNewHead, EvClosed, EvMerged, EvMergedUnchecked, EvGo, EvHold, EvRelease, EvDone, EvFailed, EvSuperseded, EvStop, - EvHistory, EvRetire} + EvHistory, EvRetire, EvCovered, EvLifted} for _, from := range append([]State{None}, AllStates...) { for _, ev := range events { for _, act := range []bool{false, true} { @@ -140,10 +148,16 @@ func TestTheTableKeepsItsRules(t *testing.T) { t.Errorf("%s goes back to %s by %s", from, r.To, r.Event) } } - // The trunk rule: only a ready delivery is published, by its merge; nothing off the trunk delivers. - if r.To == Published && (r.Event != EvMerged || len(r.From) != 1 || r.From[0] != Ready) { + // The trunk rule: only a ready delivery is published, by its merge — or a held one published again when + // what held it is gone (issue 316); nothing off the trunk delivers. + if r.To == Published && !(r.Event == EvMerged && len(r.From) == 1 && r.From[0] == Ready) && + !(r.Event == EvLifted && len(r.From) == 1 && r.From[0] == Held) { t.Errorf("%v —%s→ published: only a ready delivery's merge publishes", r.From, r.Event) } + // Nothing a person did not decide ends a delivery as stopped or failed by observation from held. + if contains(r.From, Held) && !r.Act && (r.To == Stopped || r.To == Failed) { + t.Errorf("held —%s→ %s by observation", r.Event, r.To) + } if r.To == Delivering { for _, from := range r.From { if from == Proposed || from == Checked || from == Ready || from == Rejected { 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",