Merge main: hold a moving machine back, then deliver the rest grants first

Main sends every machine through deliver (memberships before declarations,
hq ADR 0218); this branch holds back a machine whose running module's data
would move (ADR 0217). Both stand: the held machines are filtered out before
delivery, in the named push and its cascade alike.

internal/inventory/busrecords_test.go

@@ -42,26 +42,37 @@ func theSeatDeclarer() catalogue.Manifest {
 // A module assigned to a machine becomes a user with the authority its manifest declared — and the
 // protocol of a seat declared by a *different* module, which is the whole reason a seat exists.
 func TestAnAssignedModuleBecomesAUserWithWhatItDeclared(t *testing.T) {
+	// It declares where its account is delivered: a module with no own secret named broker can never
+	// be issued one, and is no user at all (novox/hq issue 195) — the case asserted below.
 	shop := catalogue.Manifest{
 		Module: "shop", Version: "1",
 		Emits: []string{"order.placed"}, Tools: []string{"price"},
-		Uses: []string{"telegram-sender"},
+		Uses:       []string{"telegram-sender"},
+		OwnSecrets: catalogue.OwnSecrets{"broker": {Path: "/run/broker"}},
 	}
-	inv, ctx := aMeshWith(t, theSeatDeclarer(), shop)
+	quiet := catalogue.Manifest{Module: "quiet", Version: "1", Emits: []string{"thing.happened"}}
+	inv, ctx := aMeshWith(t, theSeatDeclarer(), shop, quiet)
 	if _, err := inv.AddNode(ctx, "one"); err != nil {
 		t.Fatal(err)
 	}
-	if _, err := inv.Assign(ctx, "one", "shop"); err != nil {
-		t.Fatal(err)
+	for _, module := range []string{"shop", "quiet"} {
+		if _, err := inv.Assign(ctx, "one", module); err != nil {
+			t.Fatal(err)
+		}
 	}
 
 	records, err := inv.BusRecords(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
-	on := records.Assigned["one"]
-	if len(on) != 1 || on[0].Module != "shop" {
-		t.Fatalf("the machine's modules read as %+v", on)
+	var on []broker.Declared
+	for _, d := range records.Assigned["one"] {
+		if d.Module == "shop" {
+			on = append(on, d)
+		}
+	}
+	if len(on) != 1 || on[0].NoAccount {
+		t.Fatalf("the machine's modules read as %+v", records.Assigned["one"])
 	}
 	if len(on[0].Uses) != 1 || on[0].Uses[0].Accepts[0] != "send" {
 		t.Fatalf("the seat it uses carries no protocol: %+v — so it would be granted nothing on a "+
@@ -98,6 +109,11 @@ func TestAnAssignedModuleBecomesAUserWithWhatItDeclared(t *testing.T) {
 	if !found {
 		t.Fatal("no user was derived for the assigned module")
 	}
+	for _, u := range users {
+		if u.Username() == "one.quiet" {
+			t.Fatal("a module with nowhere to read an account was made a user (novox/hq issue 195)")
+		}
+	}
 }
 
 // A machine holding a live token gets an enrolment user; one whose token is spent or expired does

internal/inventory/collection.go

@@ -3,6 +3,7 @@ package inventory
 import (
 	"context"
 	"encoding/json"
+	"sort"
 	"strings"
 
 	"github.com/novox/mesh-controller/internal/catalogue"
@@ -33,7 +34,8 @@ const KeptBuilds = 5
 //   - **a definition names it** — the reference appears in a module's recorded manifest, which is
 //     what the mesh would hand a machine now. No age limit: this is the floor;
 //   - **the mesh can still go back to it** — it is an artifact of one of the KeptBuilds most
-//     recent successful builds of its module;
+//     recent successful builds of a module the mesh still holds. A forgotten module keeps
+//     nothing beyond what a held definition names (novox/hq issue 253);
 //   - it was already collected, in which case there is nothing left to do.
 //
 // Returned in a stated order so two runs over the same records ask for the same things in the
@@ -109,12 +111,19 @@ func (i *Inventory) keptReferences(ctx context.Context) (map[string]bool, error)
 		return nil, err
 	}
 
-	// The KeptBuilds most recent successful builds of each module, whole.
+	// The KeptBuilds most recent successful builds of each module the mesh still holds, whole.
+	//
+	// **Only a module the mesh still holds can be gone back to** (novox/hq issue 253). "Somewhere
+	// to return to" is a reason about a module's releases; a module that has been forgotten has
+	// no releases left to return between, and its build rows stay only as history. Without the
+	// join every module ever built kept five builds' artifacts for ever — and once the store's
+	// collector runs for real, what the keep set says is what the disk holds.
 	recent, err := i.store.Pool().Query(ctx,
 		`select made from (
-		    select made, row_number() over (partition by module order by at desc, id desc) as back
-		      from build
-		     where failed = '' and module is not null and module <> ''
+		    select b.made, row_number() over (partition by b.module order by b.at desc, b.id desc) as back
+		      from build b
+		      join module m on m.name = b.module
+		     where b.failed = '' and b.module is not null and b.module <> ''
 		 ) ranked where back <= $1`, KeptBuilds)
 	if err != nil {
 		return nil, err
@@ -169,6 +178,28 @@ func (i *Inventory) keptReferences(ctx context.Context) (map[string]bool, error)
 	return keep, nil
 }
 
+// KeptArchives is every archive the mesh keeps, in a stated order: the references the sweep must
+// hold by a manifest before it lets anything go, and the ones an operator needs to read as all
+// held before the store's collector is let loose (novox/hq issue 253, ADR 0189).
+//
+// Only references into the mesh's own store, and only blobs: an image is its own manifest, and a
+// reference that is kept because nothing here can speak for it is not one the store can be asked
+// about.
+func (i *Inventory) KeptArchives(ctx context.Context) ([]string, error) {
+	keep, err := i.keptReferences(ctx)
+	if err != nil {
+		return nil, err
+	}
+	var out []string
+	for reference := range keep {
+		if path, ours := catalogue.InArtifactStore(reference); ours && strings.Contains(path, "/blobs/sha256:") {
+			out = append(out, reference)
+		}
+	}
+	sort.Strings(out)
+	return out, nil
+}
+
 // everyReferenceMade is every artifact reference any successful build recorded.
 func (i *Inventory) everyReferenceMade(ctx context.Context) ([]string, error) {
 	rows, err := i.store.Pool().Query(ctx,

internal/inventory/collection_test.go

@@ -20,6 +20,19 @@ func ref(module, artifact string, n int) string {
 	return fmt.Sprintf("%s%s/%s@sha256:%064x", catalogue.ArtifactStoreScheme, module, artifact, n)
 }
 
+// holding registers a definition for each module that names no artifact, so the mesh holds the
+// module and its recent builds are somewhere it can go back to — and nothing more.
+func holding(t *testing.T, inv *Inventory, modules ...string) {
+	t.Helper()
+	for _, module := range modules {
+		m := catalogue.Manifest{Module: module, Version: "1"}
+		if err := inv.RegisterModule(context.Background(), m,
+			Source{Repository: "https://forge.invalid/" + module + ".git"}); err != nil {
+			t.Fatal(err)
+		}
+	}
+}
+
 // built records one successful build of a module publishing one image.
 func built(t *testing.T, inv *Inventory, id, module string, n int) string {
 	t.Helper()
@@ -35,6 +48,7 @@ func built(t *testing.T, inv *Inventory, id, module string, n int) string {
 func TestTheStoreKeepsTheRecentBuildsAndLetsGoOfTheRest(t *testing.T) {
 	inv := fresh(t)
 	ctx := context.Background()
+	holding(t, inv, "web")
 
 	// Eight builds of one module, oldest first. Five are kept — the newest, and the four a
 	// release that turns out wrong can be taken back to.
@@ -98,6 +112,7 @@ func TestWhatHasBeenCollectedIsNotOfferedAgain(t *testing.T) {
 	// time it runs, for ever — a number of requests that grows with the mesh's whole history.
 	inv := fresh(t)
 	ctx := context.Background()
+	holding(t, inv, "web")
 	for i := 1; i <= 7; i++ {
 		built(t, inv, fmt.Sprintf("b%02d", i), "web", i)
 	}
@@ -123,6 +138,7 @@ func TestWhatHasBeenCollectedIsNotOfferedAgain(t *testing.T) {
 func TestAFailedBuildNamesNothingToCollectAndEachModuleIsCountedOnItsOwn(t *testing.T) {
 	inv := fresh(t)
 	ctx := context.Background()
+	holding(t, inv, "web", "db")
 
 	// A failed build published nothing, so it is neither kept nor collected — and it must not
 	// count against the module's five.
@@ -156,6 +172,7 @@ func TestAFailedBuildNamesNothingToCollectAndEachModuleIsCountedOnItsOwn(t *test
 func TestAnArtifactRecordedWithAnAddressIsOfferedAsTheMeshRecordsOne(t *testing.T) {
 	inv := fresh(t)
 	ctx := context.Background()
+	holding(t, inv, "tools")
 
 	// The oldest build published the old way; five newer ones fill the module's five.
 	old := aBuild("a00", "tools", "")
@@ -190,3 +207,82 @@ func TestAnArtifactRecordedWithAnAddressIsOfferedAsTheMeshRecordsOne(t *testing.
 		t.Fatalf("offered %v again after collecting it", again)
 	}
 }
+
+// A forgotten module keeps nothing beyond what a held definition names (novox/hq issue 253).
+//
+// "Somewhere to go back to" is a reason about a module's releases, and a module the mesh no
+// longer holds has none. Its build rows stay as history; its artifacts go — except one a module
+// the mesh still holds names, which is the floor whatever built it.
+func TestAForgottenModuleKeepsNothingAHeldDefinitionDoesNotName(t *testing.T) {
+	inv := fresh(t)
+	ctx := context.Background()
+
+	// Three builds of a module that was never held, or was held and then forgotten: within its
+	// five, and kept for that reason until now.
+	var gone []string
+	for i := 1; i <= 3; i++ {
+		gone = append(gone, built(t, inv, fmt.Sprintf("o%02d", i), "old", 200+i))
+	}
+	// A module the mesh holds, whose definition runs the forgotten module's newest image.
+	named := gone[2]
+	m := catalogue.Manifest{Module: "web", Version: "1", Resources: []map[string]any{{
+		"id": "app", "type": "container", "name": "web", "image": named,
+	}}}
+	if err := inv.RegisterModule(ctx, m, Source{Repository: "https://forge.invalid/web.git"}); err != nil {
+		t.Fatal(err)
+	}
+
+	go_, err := inv.ToCollect(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(go_) != 2 || go_[0] != gone[0] || go_[1] != gone[1] {
+		t.Fatalf("offered %v; want %v — a forgotten module's builds are no release to go back to, "+
+			"and only what a held definition names stays", go_, gone[:2])
+	}
+
+	// And once the module is held again, its five are kept again.
+	holding(t, inv, "old")
+	again, err := inv.ToCollect(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(again) != 0 {
+		t.Fatalf("offered %v for a module the mesh holds, within its five", again)
+	}
+}
+
+// The archives the mesh keeps are what the sweep holds before it lets anything go, and what an
+// operator reads as all held before the store's collector is let loose (novox/hq issue 253).
+func TestKeptArchivesAreTheKeptBlobsOnly(t *testing.T) {
+	inv := fresh(t)
+	ctx := context.Background()
+	holding(t, inv, "shell")
+
+	archive := func(n int) string {
+		return fmt.Sprintf("%sshell/config/blobs/sha256:%064x", catalogue.ArtifactStoreScheme, n)
+	}
+	for i := 1; i <= 6; i++ {
+		b := aBuild(fmt.Sprintf("s%02d", i), "shell", "")
+		b.Made = []Artifact{
+			{Name: "app", Kind: "image", Reference: ref("shell", "app", i)},
+			{Name: "config", Kind: "archive", Reference: archive(i)},
+		}
+		if err := inv.RecordBuild(ctx, b); err != nil {
+			t.Fatal(err)
+		}
+	}
+	kept, err := inv.KeptArchives(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := []string{archive(2), archive(3), archive(4), archive(5), archive(6)}
+	if len(kept) != len(want) {
+		t.Fatalf("kept archives %v; want the five recent ones and no images", kept)
+	}
+	for i := range want {
+		if kept[i] != want[i] {
+			t.Fatalf("kept archives %v; want %v", kept, want)
+		}
+	}
+}

internal/inventory/migrations/0058-a-newer-plan-supersedes-an-older.sql

@@ -0,0 +1,22 @@
+-- A newer plan supersedes the older open plans of the same repository and branch (novox/hq issue 254,
+-- ADR 0218).
+--
+-- A merge produced a plan without looking at the plans still open, so two merges a few minutes apart
+-- were two plans working the same modules, and a plan stuck waiting on something that would never
+-- come stayed open for ever beside the newer ones. The newer plan now takes over what the older had
+-- not yet built and the older is closed as `superseded` — a state of its own, so `plans` can say
+-- which plan replaced it rather than reading as a failure.
+--
+-- `branch` is the branch the merge went into, so only a plan of the same branch is superseded. Empty
+-- for every plan from before this was kept: which branch it answered is not known, and such a plan
+-- is superseded by the next plan of its repository, whichever branch — nothing is lost by it, since
+-- what it had not built is folded into the plan that supersedes it.
+alter table release_plan add column branch text not null default '';
+
+-- And the bus's user list the machine holding the bus was last sent, as a digest (novox/hq issue
+-- 249). A module's new grants are refused by the bus until its user list says them, so that machine
+-- is sent first whenever the list it would be sent differs from the one it was. Read from its whole
+-- declaration, every pending change on it — a recorded upgrade the operator chose not to roll out —
+-- went with every send anywhere. A digest and never the list (ADR 0043: the list is composed on each
+-- push, never kept). Empty for a machine never sent one, which reads as behind once.
+alter table node add column sent_bus_users text not null default '';

internal/inventory/nodes.go

@@ -921,6 +921,26 @@ func (i *Inventory) RecordSent(ctx context.Context, node, digest string) error {
 	return err
 }
 
+// RecordSentBusUsers keeps a digest of the bus's user list a machine was just sent, by its name
+// (novox/hq issue 249): whether the machine holding the bus must go first is whether this differs
+// from the list composed now.
+func (i *Inventory) RecordSentBusUsers(ctx context.Context, name, digest string) error {
+	_, err := i.store.Pool().Exec(ctx,
+		`update node set sent_bus_users = $2 where name = $1`, name, digest)
+	return err
+}
+
+// SentBusUsers is the digest of the bus's user list a machine was last sent, empty for none.
+func (i *Inventory) SentBusUsers(ctx context.Context, name string) (string, error) {
+	var sent string
+	err := i.store.Pool().QueryRow(ctx,
+		`select sent_bus_users from node where name = $1`, name).Scan(&sent)
+	if errors.Is(err, pgx.ErrNoRows) {
+		return "", nil
+	}
+	return sent, err
+}
+
 // Outstanding is the digest of the declaration a machine was last sent, by its name, and empty
 // for one that has never been sent anything.
 //

internal/inventory/plans.go

@@ -15,16 +15,19 @@ import (
 // the store so a controller replaced mid-plan resumes it, and so `status` can say what a merge
 // still waits for.
 type Plan struct {
-	ID         string                 `json:"id"`
-	Repository string                 `json:"repository"`
-	Commit     string                 `json:"commit"`
-	Created    time.Time              `json:"created"`
-	Updated    time.Time              `json:"updated"`
-	State      string                 `json:"state"`
-	Tier       int                    `json:"tier"`
-	Tiers      [][]string             `json:"tiers"`
-	Modules    map[string]*PlanModule `json:"modules"`
-	Note       string                 `json:"note,omitempty"`
+	ID         string `json:"id"`
+	Repository string `json:"repository"`
+	// Branch is the branch the merge went into (novox/hq issue 254): a newer plan supersedes the open
+	// ones of the same repository and branch. Empty for a plan from before it was kept.
+	Branch  string                 `json:"branch,omitempty"`
+	Commit  string                 `json:"commit"`
+	Created time.Time              `json:"created"`
+	Updated time.Time              `json:"updated"`
+	State   string                 `json:"state"`
+	Tier    int                    `json:"tier"`
+	Tiers   [][]string             `json:"tiers"`
+	Modules map[string]*PlanModule `json:"modules"`
+	Note    string                 `json:"note,omitempty"`
 }
 
 // PlanModule is one module's state within a plan.
@@ -37,8 +40,15 @@ type PlanModule struct {
 	// later tier is built by it (ADR 0163's gate): the reports that open the gate are the ones
 	// after this.
 	SentAt *time.Time `json:"sent_at,omitempty"`
-	Commit string     `json:"commit,omitempty"`
-	Why    string     `json:"why,omitempty"`
+	// First is the machines the plan sent the new build to first, and FirstAt when (novox/hq issue
+	// 249, ADR 0218): unless the module's policy rolls it out together, one machine takes it before
+	// the rest, and the rest are sent once that one reports it applied. Kept so a controller
+	// replaced while the plan waits on that report resumes the wait rather than sending again. The
+	// machine holding the bus is among them when its user list had to go first.
+	First   []string   `json:"first,omitempty"`
+	FirstAt *time.Time `json:"first_at,omitempty"`
+	Commit  string     `json:"commit,omitempty"`
+	Why     string     `json:"why,omitempty"`
 }
 
 // The states a plan passes through.
@@ -47,6 +57,9 @@ const (
 	PlanRolling  = "rolling"
 	PlanDone     = "done"
 	PlanFailed   = "failed"
+	// PlanSuperseded is a plan a newer merge of the same repository and branch took over (novox/hq
+	// issue 254, ADR 0218): what it had not built is in the newer plan, and its note names it.
+	PlanSuperseded = "superseded"
 )
 
 // Open says whether the plan is still being worked.
@@ -63,11 +76,11 @@ func (i *Inventory) SavePlan(ctx context.Context, p Plan) error {
 		return err
 	}
 	_, err = i.store.Pool().Exec(ctx,
-		`insert into release_plan (id, repository, commit_hash, created, updated, state, tier, tiers, modules, note)
-		 values ($1, $2, $3, $4, now(), $5, $6, $7, $8, $9)
+		`insert into release_plan (id, repository, commit_hash, created, updated, state, tier, tiers, modules, note, branch)
+		 values ($1, $2, $3, $4, now(), $5, $6, $7, $8, $9, $10)
 		 on conflict (id) do update set updated = now(), state = excluded.state, tier = excluded.tier,
-		        tiers = excluded.tiers, modules = excluded.modules, note = excluded.note`,
-		p.ID, p.Repository, p.Commit, p.Created, p.State, p.Tier, tiers, modules, p.Note)
+		        tiers = excluded.tiers, modules = excluded.modules, note = excluded.note, branch = excluded.branch`,
+		p.ID, p.Repository, p.Commit, p.Created, p.State, p.Tier, tiers, modules, p.Note, p.Branch)
 	return err
 }
 
@@ -95,7 +108,7 @@ func (i *Inventory) PlanByID(ctx context.Context, id string) (Plan, error) {
 
 func (i *Inventory) plans(ctx context.Context, tail string) ([]Plan, error) {
 	rows, err := i.store.Pool().Query(ctx,
-		`select id, repository, commit_hash, created, updated, state, tier, tiers, modules, note
+		`select id, repository, commit_hash, created, updated, state, tier, tiers, modules, note, branch
 		   from release_plan `+tail)
 	if err != nil {
 		return nil, err
@@ -106,7 +119,7 @@ func (i *Inventory) plans(ctx context.Context, tail string) ([]Plan, error) {
 		var p Plan
 		var tiers, modules []byte
 		if err := rows.Scan(&p.ID, &p.Repository, &p.Commit, &p.Created, &p.Updated, &p.State,
-			&p.Tier, &tiers, &modules, &p.Note); err != nil {
+			&p.Tier, &tiers, &modules, &p.Note, &p.Branch); err != nil {
 			return nil, err
 		}
 		if err := json.Unmarshal(tiers, &p.Tiers); err != nil {

internal/inventory/plans_test.go

@@ -46,3 +46,30 @@ func TestAPlanIsKeptAdvancedAndResumedFromTheStore(t *testing.T) {
 		t.Fatalf("a done plan is still among the recent ones: %+v", recent)
 	}
 }
+
+// novox/hq issue 254: a plan keeps the branch its merge went into, and a superseded plan is not open.
+func TestASupersededPlanIsNotOpen(t *testing.T) {
+	inv := ForTest(t)
+	ctx := t.Context()
+	p := Plan{ID: "plan-1", Repository: "novox/mesh-catalog", Branch: "main", Commit: "abc",
+		Created: time.Now().UTC(), State: PlanBuilding, Tiers: [][]string{{"gitea"}},
+		Modules: map[string]*PlanModule{"gitea": {}}}
+	if err := inv.SavePlan(ctx, p); err != nil {
+		t.Fatal(err)
+	}
+	kept, err := inv.PlanByID(ctx, "plan-1")
+	if err != nil || kept.Branch != "main" {
+		t.Fatalf("the branch was not kept: %v %+v", err, kept)
+	}
+	kept.State = PlanSuperseded
+	kept.Note = "superseded at tier 0 by plan-2"
+	if err := inv.SavePlan(ctx, kept); err != nil {
+		t.Fatal(err)
+	}
+	if open, err := inv.OpenPlans(ctx); err != nil || len(open) != 0 {
+		t.Fatalf("a superseded plan is still open: %v %+v", err, open)
+	}
+	if recent, _ := inv.RecentPlans(ctx, 5); len(recent) != 1 || recent[0].State != PlanSuperseded {
+		t.Fatalf("a superseded plan is not among the recent ones as superseded: %+v", recent)
+	}
+}

internal/inventory/sent_bus_users_test.go

@@ -0,0 +1,25 @@
+package inventory
+
+import "testing"
+
+// novox/hq issue 249: the digest of the user list a machine was last sent is kept, by its name, and
+// is empty for a machine never sent one.
+func TestTheUserListAMachineWasSentIsKept(t *testing.T) {
+	inv := ForTest(t)
+	ctx := t.Context()
+	if _, err := inv.AddNode(ctx, "anchor"); err != nil {
+		t.Fatal(err)
+	}
+	if sent, err := inv.SentBusUsers(ctx, "anchor"); err != nil || sent != "" {
+		t.Fatalf("a machine never sent a list has %q: %v", sent, err)
+	}
+	if err := inv.RecordSentBusUsers(ctx, "anchor", "abc"); err != nil {
+		t.Fatal(err)
+	}
+	if sent, err := inv.SentBusUsers(ctx, "anchor"); err != nil || sent != "abc" {
+		t.Fatalf("the list sent was not kept: %q %v", sent, err)
+	}
+	if sent, err := inv.SentBusUsers(ctx, "nobody"); err != nil || sent != "" {
+		t.Fatalf("a machine the mesh does not know: %q %v", sent, err)
+	}
+}