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

A build heard and recorded and not registered kept its refusal only when a
plan was open or the plans were held. A tier's first failure fails its plan,
so later refusals were lost; a Retry then settled those builds built from
their records, sent the modules' old versions and reported the plan done.
A build without a readable manifest never reached its plan at all.

Keep the refusal in the store and the process whatever the plans' state, and
fail the asking plan by the build's id when the module is not known.

Issue 1713.

cmd/mesh-controller/build_source.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"os"
 	"regexp"
-	"slices"
 	"strings"
 	"time"
 
@@ -206,9 +205,8 @@ func (f *sourceForms) buildsFrom(entries []inventory.Entry, repository, seat str
 type forgeFacts struct {
 	// ID is the forge's own id of the repository: what tells it from one deleted and made again by its name.
 	ID int64
-	// Guarded is whether the branch is protected as a module's trunk must be: no direct push, a merge whitelist naming
-	// the tools' account and no agent's account, and no administrator merging past the rule (novox/hq ADR 0534 §6; no
-	// required status, which the forge no longer asks, §1). Why says what is missing when it is not.
+	// Guarded is whether the branch is protected as a module's trunk must be: no direct push, at least one
+	// required status, and no administrator merging past one. Why says what is missing when it is not.
 	Guarded bool
 	Why     string
 }
@@ -363,48 +361,27 @@ func askTheForgeOnce(ctx context.Context, owner, repo, branch string) (forgeFact
 	return facts, nil
 }
 
-// forgeToolsAccount is the forge tools' account, which main's merge whitelist names for mesh-delivery (novox/hq ADR 0398
-// §2, the gitea module's tools-account.ts TOOLS_ACCOUNT); beside it the whitelist names only people's own logins, the
-// operator's (ADR 0534 §2), never an agent's account.
-const forgeToolsAccount = "mesh-tools"
-
-// isAgentAccount is whether a forge login is a node's agent's account, agent-<node> (novox/hq ADR 0398 §1): one that
-// may never merge into a module's trunk by itself.
-func isAgentAccount(login string) bool { return strings.HasPrefix(login, "agent-") }
-
 // protectionRule is a branch protection rule as the forge's tool summarises it.
 type protectionRule struct {
 	Rule             string   `json:"rule"`
 	Push             bool     `json:"push"`
 	RequiredStatuses []string `json:"required_statuses"`
 	AdminMayOverride bool     `json:"admin_may_override"`
-	// MergeWhitelist is who may merge into the branch; nil when the rule keeps no merge whitelist (anyone who writes).
-	MergeWhitelist []string `json:"merge_whitelist"`
 }
 
 // judgedRule judges the one rule the forge applies to a branch, as the forge picks it: the rule named for the
 // branch, else the first glob rule, in the forge's order, that covers it (gitea's first matching rule). Never a
-// later rule that happens to be stronger: the forge does not read it. A trunk is trusted for who can land a commit on
-// it, not for a status it requires (novox/hq ADR 0534 §6): no push, a merge whitelist naming the tools' account and
-// beside it only people's own logins, never an agent's, and no administrator merging past the rule; then a commit lands
-// there through mesh-delivery, on a passing mesh/check and the operator's Approve (ADR 0534 §4), or by the operator's
-// own hand at the forge (§2).
+// later rule that happens to be stronger: the forge does not read it.
 func judgedRule(rules []protectionRule, branch string) forgeFacts {
 	switch r := appliedRule(rules, branch); {
 	case r == nil:
 		return forgeFacts{Why: fmt.Sprintf("no protection rule covers %s", branch)}
 	case r.Push:
 		return forgeFacts{Why: fmt.Sprintf("the rule %s lets a person push to %s directly", r.Rule, branch)}
-	case r.MergeWhitelist == nil || len(r.MergeWhitelist) == 0:
-		return forgeFacts{Why: fmt.Sprintf("the rule %s keeps no merge whitelist naming %s alone for %s", r.Rule, forgeToolsAccount, branch)}
-	case !slices.Contains(r.MergeWhitelist, forgeToolsAccount):
-		return forgeFacts{Why: fmt.Sprintf("the rule %s lets %s merge into %s, and not %s", r.Rule,
-			strings.Join(r.MergeWhitelist, ", "), branch, forgeToolsAccount)}
-	case slices.ContainsFunc(r.MergeWhitelist, isAgentAccount):
-		return forgeFacts{Why: fmt.Sprintf("the rule %s lets an agent's account merge into %s: %s", r.Rule, branch,
-			strings.Join(r.MergeWhitelist, ", "))}
+	case len(r.RequiredStatuses) == 0:
+		return forgeFacts{Why: fmt.Sprintf("the rule %s requires no status before a merge into %s", r.Rule, branch)}
 	case r.AdminMayOverride:
-		return forgeFacts{Why: fmt.Sprintf("the rule %s lets an administrator merge into %s past it", r.Rule, branch)}
+		return forgeFacts{Why: fmt.Sprintf("the rule %s lets an administrator merge into %s past a status", r.Rule, branch)}
 	}
 	return forgeFacts{Guarded: true}
 }

cmd/mesh-controller/build_source_test.go

@@ -102,17 +102,16 @@ func init() {
 // The forge applies one rule to a branch: the rule named for it, else the first glob that covers it. A stronger
 // rule later in the list is not what guards the branch, and is not read as if it were (the confirmation review).
 func TestTheRuleJudgedIsTheOneTheForgeApplies(t *testing.T) {
-	tools := []string{forgeToolsAccount}
-	guarded := protectionRule{Rule: "*", MergeWhitelist: tools}
-	open := protectionRule{Rule: "main", Push: true, MergeWhitelist: tools}
+	guarded := protectionRule{Rule: "*", RequiredStatuses: []string{"mesh/merge-gate"}}
+	open := protectionRule{Rule: "main", Push: true, RequiredStatuses: []string{"mesh/merge-gate"}}
 	if f := judgedRule([]protectionRule{guarded, open}, "main"); f.Guarded {
 		t.Error("an exact rule letting pushes was passed over for a glob that guards")
 	}
-	if f := judgedRule([]protectionRule{{Rule: "ma*", MergeWhitelist: nil}, {Rule: "*", MergeWhitelist: tools}},
+	if f := judgedRule([]protectionRule{{Rule: "ma*", RequiredStatuses: nil}, {Rule: "*", RequiredStatuses: []string{"x"}}},
 		"main"); f.Guarded || !strings.Contains(f.Why, "ma*") {
 		t.Errorf("the first glob covering the branch was not the one judged: %+v", f)
 	}
-	if f := judgedRule([]protectionRule{{Rule: "release/*"}, {Rule: "main", MergeWhitelist: tools}}, "main"); !f.Guarded {
+	if f := judgedRule([]protectionRule{{Rule: "release/*"}, {Rule: "main", RequiredStatuses: []string{"x"}}}, "main"); !f.Guarded {
 		t.Errorf("the rule named for the branch was not judged: %+v", f)
 	}
 	if f := judgedRule(nil, "main"); f.Guarded {
@@ -120,39 +119,6 @@ func TestTheRuleJudgedIsTheOneTheForgeApplies(t *testing.T) {
 	}
 }
 
-// **A module's trunk is trusted when it takes no push, only the tools' account and people's own logins merge into it,
-// and no administrator merges past its rule** (novox/hq ADR 0534 §2, §6): the forge requires no status to merge (§1), so
-// a rule requiring none is trusted; one with no merge whitelist, no tools' account in it, an agent's account in it, a
-// push allowed or an administrator's override is not.
-func TestATrunkIsTrustedForItsMergersNotForAStatus(t *testing.T) {
-	tools := []string{forgeToolsAccount}
-	for _, c := range []struct {
-		name string
-		rule protectionRule
-		want bool
-		why  string
-	}{
-		{"no status, no push, the tools alone merge", protectionRule{Rule: "main", MergeWhitelist: tools}, true, ""},
-		{"a status required as well", protectionRule{Rule: "main", MergeWhitelist: tools, RequiredStatuses: []string{"mesh/check"}}, true, ""},
-		{"no merge whitelist", protectionRule{Rule: "main"}, false, "merge whitelist"},
-		{"an empty merge whitelist", protectionRule{Rule: "main", MergeWhitelist: []string{}}, false, "merge whitelist"},
-		{"the operator's own login beside the tools", protectionRule{Rule: "main", MergeWhitelist: []string{forgeToolsAccount, "jschoubben"}}, true, ""},
-		{"an agent's account in the whitelist", protectionRule{Rule: "main", MergeWhitelist: []string{forgeToolsAccount, "agent-ace"}}, false, "agent-ace"},
-		{"an agent's account alone", protectionRule{Rule: "main", MergeWhitelist: []string{"agent-ace"}}, false, "agent-ace"},
-		{"a person's login without the tools", protectionRule{Rule: "main", MergeWhitelist: []string{"jschoubben"}}, false, forgeToolsAccount},
-		{"a push allowed", protectionRule{Rule: "main", Push: true, MergeWhitelist: tools}, false, "push"},
-		{"an administrator's override", protectionRule{Rule: "main", MergeWhitelist: tools, AdminMayOverride: true}, false, "administrator"},
-	} {
-		f := judgedRule([]protectionRule{c.rule}, "main")
-		if f.Guarded != c.want || !strings.Contains(f.Why, c.why) {
-			t.Errorf("%s: %+v, want guarded %v saying %q", c.name, f, c.want, c.why)
-		}
-		if strings.Contains(f.Why, "requires no status") {
-			t.Errorf("%s: a trunk is still judged on a required status: %s", c.name, f.Why)
-		}
-	}
-}
-
 // An id the forge could not give keeps the id already recorded.
 
 // **The operator's Approve of a reviewed group registers a moved module from its new repository** (novox/hq ADR 0420 §7,

cmd/mesh-controller/main.go

@@ -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",

cmd/mesh-controller/take_refused.go

@@ -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)
+}

cmd/mesh-controller/take_refused_test.go

@@ -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)
+	}
+}