diff --git a/cmd/mesh-controller/plan_retry.go b/cmd/mesh-controller/plan_retry.go index e9a55850..902c3ebf 100644 --- a/cmd/mesh-controller/plan_retry.go +++ b/cmd/mesh-controller/plan_retry.go @@ -210,7 +210,7 @@ func retryRefusal(p inventory.Plan, plans []inventory.Plan, onPath map[string]st 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 yetToSend(p) { + 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) @@ -639,25 +639,42 @@ func waitingIn(p inventory.Plan) (asked, unsent, unasked int) { return asked, unsent, unasked } -// yetToSend is every module a stopped walk has still to send: its current tier's modules not sent and every module of -// the tiers after, sorted. -func yetToSend(p inventory.Plan) []string { +// 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 i := p.Tier; i < len(p.Tiers); i++ { - for _, m := range p.Tiers[i] { - if s := p.Modules[m]; i > p.Tier || s == nil || s.SentAt == nil { - out = append(out, m) - } + 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 -// (issue 1886): the next look carries it on as any walk — a build done waits for its reference, and one done on the +// 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++ { @@ -666,10 +683,13 @@ func resumeStopped(p *inventory.Plan) string { 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; nothing asked again — the next look carries it "+ + 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) } diff --git a/cmd/mesh-controller/plan_retry_stopped_test.go b/cmd/mesh-controller/plan_retry_stopped_test.go index 150fdfae..cbe7b69d 100644 --- a/cmd/mesh-controller/plan_retry_stopped_test.go +++ b/cmd/mesh-controller/plan_retry_stopped_test.go @@ -1,6 +1,7 @@ package main import ( + "slices" "strings" "testing" "time" @@ -28,7 +29,7 @@ func TestRetryResumesAWalkStoppedByHand(t *testing.T) { var tier1 []string for _, m := range []string{"build-agent", "mesh-tools"} { tier1 = append(tier1, m) - p.Modules[m] = &inventory.PlanModule{State: "built", BuiltAt: &sent} + 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"} @@ -56,11 +57,23 @@ func TestRetryResumesAWalkStoppedByHand(t *testing.T) { t.Errorf("the note does not record the hand resume and the stop it undid: %q", p.Note) } for _, m := range onKeys { - if s := p.Modules[m]; s.State != "asked" || !s.Key || s.Build != "build-"+m { + 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", "8 asked", "2 built and not sent", "3 not yet asked"} { + 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) } @@ -87,3 +100,40 @@ func TestRetryResumesAWalkStoppedByHand(t *testing.T) { t.Errorf("a stopped walk was resumed beside a related open walk: %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) + } +}