diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 8db2bca5..97500f76 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -136,8 +136,9 @@ func letGo(ctx context.Context, inv *inventory.Inventory, id, by, why string) (i return p, fmt.Errorf("%s was let go by %s at %s already", p.ID, p.Delivery.By, p.Delivery.Go.Local().Format("15:04:05")) } - // **One walk at a time** (novox/hq ADR 0276): the word is taken, and the walk starts once no other walk is - // started (advanceOnce), so the delivery's owner says it once and is not refused. + // **One walk at a time beside a related one** (novox/hq ADR 0276, ADR 0407 decision 4, issue 973): the word is + // taken, and the walk starts once no started walk it is related to is open (advanceOnce), so the delivery's + // owner says it once and is not refused. now := time.Now().UTC() p.Delivery.Go, p.Delivery.By, p.Delivery.Why = &now, by, why p.Note = "let go by " + by + "; its first tier is asked next" diff --git a/cmd/mesh-controller/plan_retry.go b/cmd/mesh-controller/plan_retry.go index acb722c8..fbc075e8 100644 --- a/cmd/mesh-controller/plan_retry.go +++ b/cmd/mesh-controller/plan_retry.go @@ -197,12 +197,23 @@ func retryRefusal(p inventory.Plan, plans []inventory.Plan) error { return oneWalkAtATime(p, plans) } -// oneWalkAtATime refuses a retry while another walk is open, started or waiting for its word (novox/hq ADR -// 0276): a walk retried beside it would be two walks at once. +// oneWalkAtATime refuses a retry while an open walk, started or waiting for its word, is related to it (novox/hq +// ADR 0276, ADR 0407 decision 4, issue 973): a walk retried beside a related one would be two walks on the same +// things at once. A walk related to none may be retried beside them. The retried walk is judged by the modules it +// plans, which already hold their dependents, and the repositories it carries. func oneWalkAtATime(p inventory.Plan, plans []inventory.Plan) error { + mine := footprintOfWalk(p, nil) for _, q := range plans { - if q.ID != p.ID && q.Open() && q.Release == nil { - return fmt.Errorf("%s is open (%s): one walk at a time — retry %s once it ended", q.ID, q.Named(), p.ID) + if q.ID == p.ID || !q.Open() || q.Release != nil { + continue + } + if ownWalk(p) || ownWalk(q) { + return fmt.Errorf("%s is open (%s) and one of them is on the controller's own path: one walk at a time — "+ + "retry %s once it ended", q.ID, q.Named(), p.ID) + } + if ok, why := related(mine, footprintOfWalk(q, nil), nil); ok { + return fmt.Errorf("%s is open (%s) and %s: one walk at a time beside a related one — retry %s once it ended", + q.ID, q.Named(), why, p.ID) } } return nil diff --git a/cmd/mesh-controller/related.go b/cmd/mesh-controller/related.go index 1d9858aa..fe0dc647 100644 --- a/cmd/mesh-controller/related.go +++ b/cmd/mesh-controller/related.go @@ -1,8 +1,10 @@ package main import ( + "context" "sort" "strings" + "time" "github.com/novox/mesh-controller/internal/inventory" ) @@ -139,3 +141,55 @@ func relatedOpen(cand footprint, candOwn bool, open []openWalk, edges []inventor } return "", "" } + +// ownWalk says a walk is on the controller's own path: its batch says so, or it plans a module of that path +// (issue 973). +func ownWalk(p inventory.Plan) bool { + if p.OwnPath() { + return true + } + for name := range p.Modules { + if _, own := onTheControllersPath[name]; own { + return true + } + } + return false +} + +// startedWalks are the open walks started beside the walk id, made at created: a merge's walk past its wait, not +// the backlog's, with a tier asked, or on its way (let go, or waiting for nobody) before this one. +func startedWalks(plans []inventory.Plan, id string, created time.Time) []inventory.Plan { + var out []inventory.Plan + for _, q := range plans { + if q.ID == id || q.Release != nil || q.Waiting() { + continue + } + if q.Tier > 0 || askedAny(q) || q.Created.Before(created) { + out = append(out, q) + } + } + return out +} + +// openWalkOf is an open walk as a change is judged against it: what it reaches, from its planned modules and the +// merges the record holds for it. +func openWalkOf(ctx context.Context, inv *inventory.Inventory, p inventory.Plan) (openWalk, error) { + kept, err := inv.MergesOf(ctx, p.ID) + if err != nil { + return openWalk{}, err + } + return openWalk{id: p.ID, fp: footprintOfWalk(p, kept), own: ownWalk(p)}, nil +} + +// openWalksOf are openWalkOf for each of plans. +func openWalksOf(ctx context.Context, inv *inventory.Inventory, plans []inventory.Plan) ([]openWalk, error) { + var out []openWalk + for _, p := range plans { + w, err := openWalkOf(ctx, inv, p) + if err != nil { + return nil, err + } + out = append(out, w) + } + return out, nil +} diff --git a/cmd/mesh-controller/related_test.go b/cmd/mesh-controller/related_test.go index 41593367..f45fcdf0 100644 --- a/cmd/mesh-controller/related_test.go +++ b/cmd/mesh-controller/related_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "strings" "testing" + "time" "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" @@ -155,3 +156,67 @@ func TestRelatedOpenNamesTheFirstOpenWalkAChangeIsRelatedTo(t *testing.T) { }) } } + +// A walk is on the controller's own path when its batch says so or it plans a module of that path (issue 973). +func TestAWalkOnTheControllersPathIsKnownByItsFlagOrItsModules(t *testing.T) { + if ownWalk(walkOf("novox/one", "notes")) { + t.Error("an ordinary walk read as on the controller's own path") + } + flagged := walkOf("novox/one", "notes") + flagged.Delivery = &inventory.PlanDelivery{Batch: &inventory.PlanBatch{Own: true}} + if !ownWalk(flagged) { + t.Error("a walk whose batch is own-path read as ordinary") + } + var module string + for m := range onTheControllersPath { + module = m + } + if !ownWalk(walkOf("novox/one", "notes", module)) { + t.Errorf("a walk planning %s read as ordinary", module) + } +} + +// A walk about to ask its first tier waits only for a started walk it is related to (issue 973): started is a +// tier asked, or a walk made before it; a waiting walk, the backlog's and itself are not. +func TestWalksStartedBesideAreTheOpenOnesBeforeIt(t *testing.T) { + at := time.Date(2026, 10, 17, 12, 0, 0, 0, time.UTC) + mk := func(id string, created time.Time, tier int) inventory.Plan { + p := walkOf("novox/"+id, id) + p.ID, p.Created, p.Tier, p.State = id, created, tier, inventory.PlanBuilding + return p + } + older, newer, ticking := mk("older", at, 0), mk("newer", at.Add(time.Minute), 0), mk("ticking", at.Add(2*time.Minute), 1) + backlog := mk("backlog", at, 1) + backlog.Release = &inventory.PlanRelease{} + got := startedWalks([]inventory.Plan{older, newer, ticking, backlog, mk("me", at.Add(time.Minute), 0)}, "me", at.Add(time.Minute)) + var ids []string + for _, p := range got { + ids = append(ids, p.ID) + } + if strings.Join(ids, ",") != "older,ticking" { + t.Errorf("started beside me = %v; want older, ticking", ids) + } +} + +// A retry is refused beside an open walk it is related to and allowed beside one it shares nothing with +// (issue 973); a retry on the controller's own path is refused beside any open walk. +func TestARetryWaitsOnlyForAnOpenWalkItIsRelatedTo(t *testing.T) { + failed := walkOf("novox/one", "notes") + failed.ID, failed.State = "plan-1", inventory.PlanFailed + open := func(id, repo string, modules ...string) inventory.Plan { + p := walkOf(repo, modules...) + p.ID, p.State = id, inventory.PlanBuilding + return p + } + if err := oneWalkAtATime(failed, []inventory.Plan{open("plan-2", "novox/two", "app")}); err != nil { + t.Errorf("refused beside an unrelated walk: %v", err) + } + err := oneWalkAtATime(failed, []inventory.Plan{open("plan-3", "novox/three", "notes")}) + if err == nil || !strings.Contains(err.Error(), "plan-3") || !strings.Contains(err.Error(), "both move notes") { + t.Errorf("a retry beside a walk moving the same module: %v", err) + } + failed.Delivery = &inventory.PlanDelivery{Batch: &inventory.PlanBatch{Own: true}} + if err := oneWalkAtATime(failed, []inventory.Plan{open("plan-2", "novox/two", "app")}); err == nil { + t.Error("an own-path retry went beside an open walk") + } +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 46cf11fe..99a8fbbd 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -618,23 +618,32 @@ func planBuiltSaying(ctx context.Context, open *stores, module, commit, failed s advanceHeld(ctx, open) } -// startedBeside is the id of a started walk open beside this one — a merge's walk past its wait, not the -// backlog's — or empty: one walk at a time (novox/hq ADR 0276). -func startedBeside(ctx context.Context, inv *inventory.Inventory, id string, created time.Time) (string, error) { +// startedBeside is the id of a started walk open beside this one that it is related to — a merge's walk past +// its wait, not the backlog's — with why, or empty: a walk waits only for a started walk it shares something with +// (novox/hq ADR 0276 decision 4, ADR 0407 decision 4, issue 973). +func startedBeside(ctx context.Context, inv *inventory.Inventory, p inventory.Plan) (behind, why string, err error) { plans, err := inv.OpenPlans(ctx) if err != nil { - return "", err + return "", "", err } - for _, q := range plans { - if q.ID == id || q.Release != nil || q.Waiting() { - continue - } - // Started: a tier asked, or on its way (let go, or waiting for nobody) before this one. - if q.Tier > 0 || askedAny(q) || q.Created.Before(created) { - return q.ID, nil - } + started := startedWalks(plans, p.ID, p.Created) + if len(started) == 0 { + return "", "", nil } - return "", nil + edges, err := inv.Dependencies(ctx) + if err != nil { + return "", "", err + } + open, err := openWalksOf(ctx, inv, started) + if err != nil { + return "", "", err + } + mine, err := openWalkOf(ctx, inv, p) + if err != nil { + return "", "", err + } + behind, why = relatedOpen(mine.fp, mine.own, open, edges) + return behind, why, nil } // deferredNote begins the note of a walk deferred behind another. @@ -793,13 +802,14 @@ func advanceOnce(ctx context.Context, open *stores, p *inventory.Plan, tier := p.Tiers[p.Tier] // Not yet asked: ask — the whole tier, or what of it is still unasked (novox/hq issue 754). if unasked := unaskedOf(*p); len(unasked) > 0 { - // **One walk at a time** (novox/hq ADR 0276): a walk about to ask its first tier while another started - // walk is open waits for that one to end, let go or not, and says so. + // **One walk at a time beside a related one** (novox/hq ADR 0276, ADR 0407 decision 4, issue 973): a walk + // about to ask its first tier while a started walk it is related to is open waits for that one to end, + // let go or not, and says so and why. A started walk it shares nothing with does not hold it. if p.Tier == 0 && !askedInTier(*p) { - if behind, err := startedBeside(ctx, inv, p.ID, p.Created); err != nil { + if behind, why, err := startedBeside(ctx, inv, *p); err != nil { return false, err } else if behind != "" { - note := fmt.Sprintf("%s%s ended — one walk at a time", deferredNote, behind) + note := fmt.Sprintf("%s%s ended — %s", deferredNote, behind, why) changed := p.Note != note p.Note = note return changed, nil @@ -1722,7 +1732,7 @@ func planLineSaid(p inventory.Plan, now time.Time, pause pauseView, bound time.D return fmt.Sprintf("%s · %s, %s, for %s", name, where, waitingSaid(p, waits[p.ID]), since) } if deferred(p) { - // Nor is waiting for the walk before it to end (one walk at a time, novox/hq ADR 0276). + // Nor is waiting for the walk before it to end (a related walk, novox/hq ADR 0276, ADR 0407). return fmt.Sprintf("%s · %s, %s, for %s", name, where, p.Note, since) } if waiting, paused := pausedWaiting(p, pause, now); paused {