Merge pull request 'Keep every refused build by its id, so a plan never settles a refused build built and sends the old version (issue 1713)' (#446) from fix/1713-keep-every-refusal into main

2026-10-23 10:58:08 +00:00
parent 0173d9c70e 149f89e8f9
commit c834e4f821
3 changed files with 173 additions and 3 deletions
@@ -448,9 +448,12 @@ func (b builds) Built(ctx context.Context, result link.BuildResult) error {
 		return nil
 	case err != nil:
 		fmt.Printf("%s: heard and recorded, and not registered: %v\n", result.ID, err)
-		if manifest.Module != "" {
-			planBuilt(ctx, b.open, manifest.Module, result.Commit, err.Error(), asked, result.ID)
-		}
+		takeRefused(ctx, b.inv, result, manifest.Module, err.Error(), refusedOutcome{
+			built: func(module, why string) {
+				planBuilt(ctx, b.open, module, result.Commit, why, asked, result.ID)
+			},
+			failedBuild: func(failed link.BuildResult) { planFailedBuild(ctx, b.open, failed) },
+		})
 		return nil
 	}
 	fmt.Printf("%s: %s %s registered, built on %s from %s\n",
@@ -0,0 +1,48 @@
+package main
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/novox/mesh-controller/internal/link"
+)
+
+// refusalKeeper keeps a refused registration in the store (*inventory.Inventory, or a test's).
+type refusalKeeper interface {
+	RecordRefused(ctx context.Context, id, module, why string) error
+}
+
+// refusedOutcome is how a refused build reaches the plans: built takes it in by its module (planBuilt), failedBuild
+// by the id the plan asked it under (planFailedBuild).
+type refusedOutcome struct {
+	built       func(module, why string)
+	failedBuild func(result link.BuildResult)
+}
+
+// takeRefused takes in a build heard and recorded, and not registered (novox/hq issue 1713).
+//
+// **The refusal is kept by the build's id, whatever the plans' state**: in the store (RecordRefused) and in this
+// process (keepRefusalFor). Taking it into the plans keeps it only when a plan is open or the plans are held, and a
+// tier's first failure fails its plan, which no outcome reaches after that: the build record says the builder's words,
+// which worked, and a Retry settled from it sent the module's old version and reported the plan done (on 2026-10-23
+// 12:01, plan-1792748906937927069 sent mesh-build-go, mesh-build-node, mosquitto and ssh-client so).
+//
+// **A build whose module is not known fails the plan that asked it by its id** (planFailedBuild, moduleAskedAs), as
+// a failed build that names no module does: a manifest that could not be read is no reason for the plan not to hear.
+func takeRefused(ctx context.Context, keeper refusalKeeper, result link.BuildResult, module, why string,
+	plans refusedOutcome) {
+	if result.ID != "" {
+		if err := keeper.RecordRefused(ctx, result.ID, module, why); err != nil {
+			fmt.Printf("%s: the refusal could not be kept in the store, only in this controller: %v\n", result.ID, err)
+		}
+		keepRefusalFor(result.ID, why, time.Now())
+	}
+	if module != "" {
+		plans.built(module, why)
+		return
+	}
+	failed := result
+	failed.Failed = why
+	plans.failedBuild(failed)
+}
@@ -0,0 +1,119 @@
+package main
+
+import (
+	"context"
+	"reflect"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/novox/mesh-controller/internal/inventory"
+	"github.com/novox/mesh-controller/internal/link"
+)
+
+// keptRefusals is a store that keeps refusals, by id.
+type keptRefusals map[string]string
+
+func (k keptRefusals) RecordRefused(_ context.Context, id, _, why string) error {
+	if _, ok := k[id]; !ok {
+		k[id] = why
+	}
+	return nil
+}
+
+// failedPlanOf is a plan already failed on gitea, its tier also asking module under id: open() does not count it, so
+// no outcome taken in reaches it (as plan-1792748906937927069 on 2026-10-23 11:51).
+func failedPlanOf(module, id string, asked time.Time) inventory.Plan {
+	return inventory.Plan{ID: "plan-1713", State: inventory.PlanFailed, Tier: 0,
+		Tiers: [][]string{{"gitea", module}}, Note: "gitea failed to build in tier 0",
+		Modules: map[string]*inventory.PlanModule{
+			"gitea": {State: "failed", AskedAt: &asked, Build: "build-1713-gitea", Why: "not registered"},
+			module:  {State: "asked", AskedAt: &asked, Build: id},
+		}}
+}
+
+// retried is what a Retry of the plan asks again, its records read as recordsOfAsked reads them: the build record of
+// id (which says the builder's words, worked) with every refusal kept for it in the store and in this process.
+func retried(p *inventory.Plan, store keptRefusals, id, module string, at time.Time) []string {
+	byID := map[string]inventory.Build{id: {ID: id, Module: module, Commit: "6c0024e4", At: at}}
+	refused := map[string]string{}
+	for k, why := range store {
+		refused[k] = why
+	}
+	for k, why := range refusalsKeptFor([]string{id}) {
+		refused[k] = why
+	}
+	overlayRefusals(nil, byID, refused)
+	return toRetry(p, nil, byID)
+}
+
+// 1713/AC1: given a build of a module heard and recorded and not registered while its plan is failed and the plans are
+// not held, when the controller takes it in, then the refusal is kept by id, and a Retry asks the module again instead
+// of settling it built and sending its old version (plan-1792748906937927069 sent mosquitto's on 2026-10-23 12:01).
+func TestARefusalWhileThePlanIsFailedIsKeptAndRetried(t *testing.T) {
+	asked := time.Date(2026, 10, 23, 11, 51, 0, 0, time.UTC)
+	id := "build-1713-ac1-mosquitto"
+	p := failedPlanOf("mosquitto", id, asked)
+	store := keptRefusals{}
+	var took []string
+	takeRefused(t.Context(), store, link.BuildResult{ID: id, Commit: "6c0024e4"}, "mosquitto",
+		"the trunk requires no status", refusedOutcome{
+			built:       func(module, why string) { took = append(took, module+": "+why) },
+			failedBuild: func(link.BuildResult) { t.Fatal("a refusal with its module was taken by id") },
+		})
+
+	if !reflect.DeepEqual(took, []string{"mosquitto: the trunk requires no status"}) {
+		t.Fatalf("the plans were told %v", took)
+	}
+	if store[id] == "" {
+		t.Fatal("the refusal was not kept in the store")
+	}
+	if got := retried(&p, keptRefusals{}, id, "mosquitto", asked.Add(time.Minute)); !reflect.DeepEqual(got,
+		[]string{"gitea", "mosquitto"}) {
+		t.Fatalf("a Retry, the refusal kept only in this controller, asks %v; mosquitto was settled %q",
+			got, p.Modules["mosquitto"].State)
+	}
+}
+
+// 1713/AC2: given a build that came back without a readable manifest and was not registered, when the controller takes
+// it in, then the asking plan is failed by the build's id and the refusal is kept by id, so a Retry asks it again.
+func TestARefusalWithoutAModuleFailsItsPlanByIDAndIsRetried(t *testing.T) {
+	asked := time.Date(2026, 10, 23, 11, 51, 0, 0, time.UTC)
+	id := "build-1713-ac2-ssh-client"
+	p := failedPlanOf("ssh-client", id, asked)
+	store := keptRefusals{}
+	var failed []link.BuildResult
+	takeRefused(t.Context(), store, link.BuildResult{ID: id, Commit: "6c0024e4"}, "",
+		"what came back is not a manifest", refusedOutcome{
+			built:       func(module, _ string) { t.Fatalf("a refusal without a module was taken as %q", module) },
+			failedBuild: func(r link.BuildResult) { failed = append(failed, r) },
+		})
+
+	if len(failed) != 1 || failed[0].ID != id || !strings.Contains(failed[0].Failed, "not a manifest") {
+		t.Fatalf("the asking plan was not failed by the build's id: %+v", failed)
+	}
+	if store[id] == "" {
+		t.Fatal("the refusal was not kept in the store")
+	}
+	if got := retried(&p, store, id, "", asked.Add(time.Minute)); !reflect.DeepEqual(got,
+		[]string{"gitea", "ssh-client"}) {
+		t.Fatalf("a Retry asks %v; ssh-client was settled %q", got, p.Modules["ssh-client"].State)
+	}
+}
+
+// 1713/AC3: given a refused build kept by id, when the records settle a building plan's tier, then the module and the
+// plan are failed, never built: the plan never reports done for it.
+func TestAPlanNeverSettlesARefusedBuildBuilt(t *testing.T) {
+	asked := time.Date(2026, 10, 23, 11, 51, 0, 0, time.UTC)
+	id := "build-1713-ac3-mesh-build-go"
+	takeRefused(t.Context(), keptRefusals{}, link.BuildResult{ID: id}, "mesh-build-go", "not registered",
+		refusedOutcome{built: func(string, string) {}, failedBuild: func(link.BuildResult) {}})
+	p := inventory.Plan{ID: "plan-1713-ac3", State: inventory.PlanBuilding, Tiers: [][]string{{"mesh-build-go"}},
+		Modules: map[string]*inventory.PlanModule{"mesh-build-go": {State: "asked", AskedAt: &asked, Build: id}}}
+	byID := map[string]inventory.Build{id: {ID: id, Module: "mesh-build-go", At: asked.Add(time.Minute)}}
+	overlayRefusals(nil, byID, refusalsKeptFor([]string{id}))
+	settleFromRecords(&p, p.Tiers[0], nil, byID)
+	if s := p.Modules["mesh-build-go"]; s.State != "failed" || p.State != inventory.PlanFailed {
+		t.Fatalf("a refused build settled as %q, the plan %q", s.State, p.State)
+	}
+}