Merge pull request 'Ask build-agent until the builder says it writes references, so a plan never waits on a key nothing can reference (issue 1886, ADR 0522)' (#442 ) from fix/1886-builder-says-references into main

Reviewed-on: #442
2026-10-23 00:27:00 +00:00
2 changed files with 0 additions and 265 deletions
@@ -203,21 +203,6 @@ func retryRefusal(p inventory.Plan, plans []inventory.Plan, onPath map[string]st
 		return oneWalkAtATime(p, plans, onPath)
 	}
 	stopped := stoppedRollouts(p)
-	// **A walk stopped by hand while its tier waited is resumed** (issue 1886): nothing in it failed, so nothing is
-	// asked again; it goes on from where it stopped, unless a newer walk holds what it has yet to send.
-	if len(stopped) == 0 && stoppedByHandWaiting(p) {
-		if q, found := newerOpenPlan(p, plans); found {
-			return fmt.Errorf("%s supersedes it: a newer merge of %s (%s at %s) is open, and resuming %s would build "+
-				"what that one replaced", q.ID, q.Repository, q.ID, short(q.Commit), p.ID)
-		}
-		for _, m := range holdingABuild(p) {
-			if q, found := newerPlanFor(m, p, plans); found {
-				return fmt.Errorf("%s has a newer plan, %s (%s, %s at %s): resuming %s would send the older build of it "+
-					"over the newer one", m, q.ID, q.State, q.Repository, short(q.Commit), p.ID)
-			}
-		}
-		return oneWalkAtATime(p, plans, onPath)
-	}
 	if len(stopped) == 0 {
 		return fmt.Errorf("nothing in tier %d of %s failed to build or stopped rolling out — it stopped at: %s",
 			p.Tier, p.ID, p.Note)
@@ -348,8 +333,6 @@ func plansForRetry(ctx context.Context, inv planReader, p inventory.Plan) ([]inv

 // retryPlan asks a failed plan's failed modules again, under new ids, and sets it building again at
 // that tier: what follows is the plan going on as if they had built the first time.
-// A walk stopped by hand while its tier waited, with nothing failed, is resumed instead: nothing asked again
-// (resumeStopped, issue 1886).
 func retryPlan(ctx context.Context, open *stores, id string) (string, error) {
 	inv := open.inventory
 	release, err := inv.HoldPlans(ctx, true)
@@ -405,13 +388,6 @@ func retryPlan(ctx context.Context, open *stores, id string) (string, error) {
 	if again := unjudgedAtGate(p); len(failed) == 0 && len(again) > 0 {
 		return retryTierWhole(ctx, open, &p, again)
 	}
-	if len(failed) == 0 && len(stoppedRollouts(p)) == 0 && stoppedByHandWaiting(p) {
-		said := resumeStopped(&p)
-		if err := inv.SavePlan(ctx, &p); err != nil {
-			return "", err
-		}
-		return said, nil
-	}
 	if len(failed) == 0 {
 		return retryRollouts(ctx, open, &p)
 	}
@@ -607,89 +583,3 @@ func toRetry(p *inventory.Plan, recorded map[string][]inventory.Build, byID map[
 	}
 	return failedIn(*p)
 }
-
-// stoppedByHandWaiting says a walk was stopped by hand while its current tier still waited (issue 1886): stopped by a
-// person, nothing of the tier failed, and a module of it asked, built and not sent, or not yet asked. A stop that left
-// merges out is not one: retryPlan refuses it before this is read (novox/hq ADR 0299).
-func stoppedByHandWaiting(p inventory.Plan) bool {
-	if p.State != inventory.PlanFailed || p.Delivery == nil || p.Delivery.Stopped == "" ||
-		p.Tier >= len(p.Tiers) || len(failedIn(p)) > 0 {
-		return false
-	}
-	asked, unsent, unasked := waitingIn(p)
-	return asked+unsent+unasked > 0
-}
-
-// waitingIn counts what a plan's current tier waits on: modules asked, built and not sent, not yet asked.
-func waitingIn(p inventory.Plan) (asked, unsent, unasked int) {
-	if p.Tier >= len(p.Tiers) {
-		return 0, 0, 0
-	}
-	for _, m := range p.Tiers[p.Tier] {
-		s := p.Modules[m]
-		switch {
-		case s == nil || s.State == "":
-			unasked++
-		case s.State == "asked":
-			asked++
-		case s.State == "built" && s.SentAt == nil:
-			unsent++
-		}
-	}
-	return asked, unsent, unasked
-}
-
-// holdingABuild is the modules of a stopped walk's current tier that hold a build of its commit, not yet sent: built and
-// not sent, or asked under a build id. Only these could put an older build over a newer plan's (review of issue 1886): a
-// module not yet asked is asked at its followed branch's head when the walk reaches it, and one asked on the builder's key
-// is settled by the look from the key of the commit it reads then. Sorted.
-func holdingABuild(p inventory.Plan) []string {
-	if p.Tier >= len(p.Tiers) {
-		return nil
-	}
-	var out []string
-	for _, m := range p.Tiers[p.Tier] {
-		s := p.Modules[m]
-		if s == nil || s.SentAt != nil || s.Key {
-			continue
-		}
-		if s.State == "built" || (s.State == "asked" && s.Build != "") {
-			out = append(out, m)
-		}
-	}
-	sort.Strings(out)
-	return out
-}
-
-// resumeStopped sets a walk stopped by hand building again at the tier it stopped at, its stop cleared and its asks kept but
-// build-agent's own (issue 1886): the next look carries it on as any walk — a build done waits for its reference, and one done on the
-// builder's key is handed on (novox/hq ADR 0522 §3). Its note keeps who stopped it and that a person resumed it.
-func resumeStopped(p *inventory.Plan) string {
-	// **Build-agent's own are asked of build-agent** (novox/hq ADR 0522 §6): one marked taken from a key it writes itself is
-	// set unasked, and the look asks the rest of a partly asked tier (unaskedOf), of build-agent, whatever the builder says.
-	var again []string
-	for _, m := range p.Tiers[p.Tier] {
-		if s := p.Modules[m]; s != nil && s.State == "asked" && s.Key && isBuildersOwn(m) {
-			s.State, s.Key, s.AskedAt, s.Build, s.Why = "", false, nil, "", ""
-			again = append(again, m)
-		}
-	}
-	sort.Strings(again)
-	asked, unsent, unasked := waitingIn(*p)
-	later := 0
-	for i := p.Tier + 1; i < len(p.Tiers); i++ {
-		later += len(p.Tiers[i])
-	}
-	by, why := p.Delivery.Stopped, p.Delivery.StoppedWhy
-	waits := fmt.Sprintf("%d asked, %d built and not sent, %d not yet asked in it; %d not yet asked in the tiers after",
-		asked, unsent, unasked, later)
-	if len(again) > 0 {
-		waits += "; " + strings.Join(again, ", ") + " set unasked, for the look to ask of build-agent (its own, ADR 0522 §6)"
-	}
-	p.State = inventory.PlanBuilding
-	goesOnAgain(p)
-	p.Delivery.Kept = nil
-	p.Note = fmt.Sprintf("tier %d resumed by hand after the stop by %s (%s): %s", p.Tier, by, why, waits)
-	return fmt.Sprintf("%s resumed at tier %d of %d, stopped by %s: %s; this verb asks nothing — the next look carries it "+
-		"on as any walk", p.ID, p.Tier+1, len(p.Tiers), by, waits)
-}
@@ -1,155 +0,0 @@
-package main
-
-import (
-	"slices"
-	"strings"
-	"testing"
-	"time"
-
-	"github.com/novox/mesh-controller/internal/inventory"
-)
-
-// A walk stopped by hand while its tier waited — modules asked on the builder's keys, two built and never sent, the next
-// tier not yet asked — is resumed by `plans retry` (issue 1886): set building again, its stop cleared, the asks it holds
-// kept, so the next look carries it on as any walk. Nothing in it failed, so the retry of failed builds had nothing to do
-// and refused it, leaving a stopped R4e with no verb to carry its 48 modules on.
-func TestRetryResumesAWalkStoppedByHand(t *testing.T) {
-	at := time.Date(2026, 10, 23, 0, 1, 53, 0, time.UTC)
-	sent := at.Add(-time.Hour)
-	p := inventory.Plan{ID: "plan-r4e", Repository: "novox/mesh-sdk", Branch: "main", Commit: "7aacbc16",
-		Created: at.Add(-2 * time.Hour), State: inventory.PlanFailed, Tier: 1,
-		Note: "stopped by a person at tier 1: issue 1886",
-		Delivery: &inventory.PlanDelivery{Stopped: "a person (fix-1886@shanks)", StoppedWhy: "issue 1886",
-			Kept: []inventory.PlanKept{{Module: "mesh-controller"}}},
-		Modules: map[string]*inventory.PlanModule{}}
-	tier0 := []string{"mesh-controller", "mesh-sdk", "pacman", "route-proxy"}
-	for _, m := range tier0 {
-		p.Modules[m] = &inventory.PlanModule{State: "built", BuiltAt: &sent, SentAt: &sent}
-	}
-	var tier1 []string
-	for _, m := range []string{"build-agent", "mesh-tools"} {
-		tier1 = append(tier1, m)
-		p.Modules[m] = &inventory.PlanModule{State: "built", AskedAt: &sent, BuiltAt: &sent, Build: "build-" + m}
-	}
-	onKeys := []string{"mesh-build-go", "mesh-build-image", "mesh-build-node", "mesh-delivery-checker",
-		"mesh-delivery-forge-bridge", "mesh-delivery-gate", "mesh-delivery-indexer", "mesh-delivery-view"}
-	for _, m := range onKeys {
-		tier1 = append(tier1, m)
-		p.Modules[m] = &inventory.PlanModule{State: "asked", AskedAt: &sent, Key: true, Build: "build-" + m}
-	}
-	tier2 := []string{"gitea", "mesh-catalog", "mongodb"}
-	for _, m := range tier2 {
-		p.Modules[m] = &inventory.PlanModule{}
-	}
-	p.Tiers = [][]string{tier0, tier1, tier2}
-
-	if err := retryRefusal(p, []inventory.Plan{p}, ownPathOf(nil)); err != nil {
-		t.Fatalf("a walk stopped by hand with its tier waiting was refused a retry: %v", err)
-	}
-	said := resumeStopped(&p)
-	if p.State != inventory.PlanBuilding {
-		t.Errorf("resumed walk is %s, wanted building", p.State)
-	}
-	if p.Delivery.Stopped != "" || p.Delivery.StoppedWhy != "" || len(p.Delivery.Kept) > 0 {
-		t.Errorf("resumed walk still says it stopped: %+v", p.Delivery)
-	}
-	if !strings.Contains(p.Note, "resumed by hand") || !strings.Contains(p.Note, "a person (fix-1886@shanks)") {
-		t.Errorf("the note does not record the hand resume and the stop it undid: %q", p.Note)
-	}
-	for _, m := range onKeys {
-		s := p.Modules[m]
-		if isBuildersOwn(m) {
-			// Build-agent's own are asked of build-agent whatever the builder says (ADR 0522 §6): set unasked, for the look.
-			if s.State != "" || s.Key || s.Build != "" || s.AskedAt != nil {
-				t.Errorf("%s, build-agent's own, was left on the builder's key: %+v", m, s)
-			}
-			if !slices.Contains(unaskedOf(p), m) {
-				t.Errorf("%s is not among what the look asks next: %v", m, unaskedOf(p))
-			}
-			continue
-		}
-		if s.State != "asked" || !s.Key || s.Build != "build-"+m {
-			t.Errorf("%s's ask was not kept for the look to hand on: %+v", m, s)
-		}
-	}
-	for _, w := range []string{"plan-r4e", "tier 2 of 3", "5 asked", "2 built and not sent", "3 not yet asked in it",
-		"3 not yet asked in the tiers after", "mesh-build-go, mesh-build-image, mesh-build-node set unasked"} {
-		if !strings.Contains(said, w) {
-			t.Errorf("the answer %q does not say %q", said, w)
-		}
-	}
-
-	// What it still refuses: a walk stopped with nothing left in its tier, and one a related open walk runs beside.
-	done := p
-	done.State, done.Delivery = inventory.PlanFailed, &inventory.PlanDelivery{Stopped: "a person"}
-	done.Modules = map[string]*inventory.PlanModule{}
-	for _, tier := range done.Tiers {
-		for _, m := range tier {
-			done.Modules[m] = &inventory.PlanModule{State: "built", BuiltAt: &sent, SentAt: &sent}
-		}
-	}
-	if err := retryRefusal(done, nil, ownPathOf(nil)); err == nil || !strings.Contains(err.Error(), "nothing in tier 1") {
-		t.Errorf("a stopped walk with nothing left in its tier was retried: %v", err)
-	}
-	stopped := p
-	stopped.State, stopped.Delivery = inventory.PlanFailed, &inventory.PlanDelivery{Stopped: "a person"}
-	beside := inventory.Plan{ID: "plan-442", Repository: "novox/mesh-sdk", Branch: "release", Commit: "bbadbb2a",
-		Created: at, State: inventory.PlanBuilding, Tiers: [][]string{{"mesh-sdk"}},
-		Modules: map[string]*inventory.PlanModule{"mesh-sdk": {State: "asked"}}}
-	if err := retryRefusal(stopped, []inventory.Plan{beside}, ownPathOf(nil)); err == nil || !strings.Contains(err.Error(), "one walk at a time") {
-		t.Errorf("a stopped walk was resumed beside a related open walk: %v", err)
-	}
-	// A newer open plan of its repository and branch supersedes it: resuming would build what that one replaced.
-	newerOpen := inventory.Plan{ID: "plan-newer", Repository: "novox/mesh-sdk", Branch: "main", Commit: "d00dfeed",
-		Created: at, State: inventory.PlanBuilding, Tiers: [][]string{{"zsh"}},
-		Modules: map[string]*inventory.PlanModule{"zsh": {State: "asked"}}}
-	if err := retryRefusal(stopped, []inventory.Plan{stopped, newerOpen}, ownPathOf(nil)); err == nil ||
-		!strings.Contains(err.Error(), "plan-newer supersedes it") {
-		t.Errorf("a stopped walk was resumed under a newer open plan of its branch: %v", err)
-	}
-	// A newer plan holding a build of its tier not yet sent refuses it: resuming would send the older build over the newer.
-	newerBuilt := inventory.Plan{ID: "plan-agent", Repository: "novox/mesh-controller", Branch: "main", Commit: "d00dfeed",
-		Created: at, State: inventory.PlanDone, Tiers: [][]string{{"build-agent"}},
-		Modules: map[string]*inventory.PlanModule{"build-agent": {State: "built"}}}
-	if err := retryRefusal(stopped, []inventory.Plan{stopped, newerBuilt}, ownPathOf(nil)); err == nil ||
-		!strings.Contains(err.Error(), "older build") {
-		t.Errorf("a stopped walk was resumed over a newer plan of a build it has not sent: %v", err)
-	}
-}
-
-// A newer plan holding a module the stopped walk has not yet asked does not refuse its resume (review of issue 1886): that
-// module is asked at its followed branch's head when the walk reaches it, so no older build goes over the newer. Open and
-// related, the newer walk refuses it under one walk at a time; done, it does not. One holding a build the walk has not sent
-// still refuses it.
-func TestResumeIsNotRefusedByANewerPlanOfAModuleNotYetAsked(t *testing.T) {
-	at := time.Date(2026, 10, 23, 0, 1, 53, 0, time.UTC)
-	sent := at.Add(-time.Hour)
-	p := inventory.Plan{ID: "plan-r4e", Repository: "novox/mesh-sdk", Branch: "main", Commit: "7aacbc16",
-		Created: at.Add(-2 * time.Hour), State: inventory.PlanFailed, Tier: 1,
-		Delivery: &inventory.PlanDelivery{Stopped: "a person", StoppedWhy: "issue 1886"},
-		Tiers:    [][]string{{"mesh-sdk"}, {"build-agent", "mesh-delivery-gate"}, {"mesh-delivery-builder", "gitea"}},
-		Modules: map[string]*inventory.PlanModule{
-			"mesh-sdk":              {State: "built", BuiltAt: &sent, SentAt: &sent},
-			"build-agent":           {State: "built", BuiltAt: &sent},
-			"mesh-delivery-gate":    {State: "asked", AskedAt: &sent, Key: true},
-			"mesh-delivery-builder": {}, "gitea": {}}}
-	newer := func(state string, holds string) inventory.Plan {
-		return inventory.Plan{ID: "plan-1792715180015327197", Repository: "novox/mesh-delivery", Branch: "main",
-			Commit: "d00dfeed", Created: at, State: state, Tiers: [][]string{{holds}},
-			Modules: map[string]*inventory.PlanModule{holds: {State: "asked"}}}
-	}
-	if err := retryRefusal(p, []inventory.Plan{p, newer(inventory.PlanDone, "mesh-delivery-builder")}, ownPathOf(nil)); err != nil {
-		t.Errorf("a done newer plan of a module not yet asked refused the resume: %v", err)
-	}
-	if err := retryRefusal(p, []inventory.Plan{p, newer(inventory.PlanDone, "mesh-delivery-gate")}, ownPathOf(nil)); err != nil {
-		t.Errorf("a done newer plan of a module on the builder's key refused the resume: %v", err)
-	}
-	if err := retryRefusal(p, []inventory.Plan{p, newer(inventory.PlanBuilding, "mesh-delivery-builder")}, ownPathOf(nil)); err == nil ||
-		!strings.Contains(err.Error(), "one walk at a time") {
-		t.Errorf("an open newer walk related to it did not refuse the resume under one walk at a time: %v", err)
-	}
-	if err := retryRefusal(p, []inventory.Plan{p, newer(inventory.PlanDone, "build-agent")}, ownPathOf(nil)); err == nil ||
-		!strings.Contains(err.Error(), "older build") {
-		t.Errorf("a newer plan of a built module not yet sent did not refuse the resume: %v", err)
-	}
-}