From 980d396d40fde728af67b75732302a70ca597597 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 16:18:39 +0200 Subject: [PATCH 1/4] Keep a first merge whose manifest word the running build-agent does not read, and name it (issue 1173) ADR 0482 section 5: a merged manifest the running reader refuses waits for the newer reader, never failed. --- cmd/mesh-controller/batches.go | 3 + cmd/mesh-controller/first_builds.go | 93 ++++++++++++++++++- cmd/mesh-controller/first_builds_held_test.go | 85 +++++++++++++++++ cmd/mesh-controller/main.go | 7 ++ internal/inventory/batches.go | 10 ++ internal/inventory/chain_rollouts.go | 3 + internal/inventory/plans.go | 3 + 7 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 cmd/mesh-controller/first_builds_held_test.go diff --git a/cmd/mesh-controller/batches.go b/cmd/mesh-controller/batches.go index 2a3fd21a..12bc80a1 100644 --- a/cmd/mesh-controller/batches.go +++ b/cmd/mesh-controller/batches.go @@ -800,6 +800,9 @@ type keptMerge struct { // FirstBuild says the merge is a first merge into a repository the mesh builds nothing from, kept to ask the first // build of its modules once mesh-delivery's word on it is kept (novox/hq ADR 0432, issue 1146). FirstBuild bool `json:"first_build,omitempty"` + // Held says the first build was refused for a word the running build-agent does not read: the merge waits for a + // build-agent that reads it, and is never failed (novox/hq ADR 0482 §5, issue 1173). + Held *heldFirstBuild `json:"held,omitempty"` } // announcedOf is a kept merge as the forge announced it, and what it moved; empty where the record says none. diff --git a/cmd/mesh-controller/first_builds.go b/cmd/mesh-controller/first_builds.go index bd48ac6a..f173948d 100644 --- a/cmd/mesh-controller/first_builds.go +++ b/cmd/mesh-controller/first_builds.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "regexp" "strings" "time" @@ -102,7 +103,7 @@ func (f following) resumeFirstBuilds(ctx context.Context, d deliveryMerge) error } for _, w := range waiting { k := announcedOf(w) - if !k.FirstBuild || !strings.EqualFold(k.Owner+"/"+k.Repo, d.Owner+"/"+d.Repo) || k.Number != d.Number || + if !k.FirstBuild || k.Held != nil || !strings.EqualFold(k.Owner+"/"+k.Repo, d.Owner+"/"+d.Repo) || k.Number != d.Number || k.HeadSHA != d.HeadSHA { continue } @@ -172,12 +173,79 @@ func firstBuildsOf(ctx context.Context, inv catalogueReader, m link.SourceMoved, return nil, "", err } +// heldFirstBuild says a first build was refused because its manifest uses a word the running build-agent does not read, +// and its merge is kept waiting for a build-agent that reads it (novox/hq ADR 0482 §5, issue 1173). +type heldFirstBuild struct { + Module string `json:"module"` + Word string `json:"word"` + At time.Time `json:"at"` +} + +// judgeRefusedFirstBuild is the hold a first build's outcome puts its merge under: nil unless the build failed on a word +// of its manifest the running build-agent does not know. +func judgeRefusedFirstBuild(result link.BuildResult, req inventory.BuildRequest, merge inventory.BatchedMerge) *heldFirstBuild { + word := refusedWord(result.Failed) + if word == "" || req.For != "merge" || merge.Plan != inventory.WaitingRecord { + return nil + } + return &heldFirstBuild{Module: req.Name(), Word: word, At: time.Now().UTC()} +} + +// unknownWord is how the strict reading of a manifest says a word is not one it knows: encoding/json's own words, which +// the build-agent's answer carries whole. The build's answer has no typed field for it (Failed is the builder's words). +var unknownWord = regexp.MustCompile(`json: unknown field "([^"]+)"`) + +// refusedWord is the word a build's failure says the reader did not know; empty when it says none. +func refusedWord(failed string) string { + if m := unknownWord.FindStringSubmatch(failed); m != nil { + return m[1] + } + return "" +} + +// holdRefusedFirstBuild keeps a first build's merge waiting when the build was refused for a word the running build-agent +// does not read (novox/hq ADR 0482 §5, issue 1173): no failed build is recorded, nothing expires the merge, and it is +// named by a condition until a delivery of build-agent asks the build again. Answers whether the outcome was dealt with. +func holdRefusedFirstBuild(ctx context.Context, inv *inventory.Inventory, result link.BuildResult) (bool, error) { + if result.Failed == "" || refusedWord(result.Failed) == "" { + return false, nil + } + req, found, err := inv.BuildRequestByID(ctx, result.ID) + if err != nil || !found { + return false, err + } + merge, found, err := inv.MergeOf(ctx, req.Repository, req.Commit) + if err != nil || !found { + return false, err + } + held := judgeRefusedFirstBuild(result, req, merge) + if held == nil { + return false, nil + } + k := announcedOf(merge) + k.Held = held + event, err := json.Marshal(k) + if err != nil { + return false, err + } + if err := inv.HoldMerge(ctx, merge.Repository, merge.Commit, event); err != nil { + return false, err + } + fmt.Printf("%s: the first build of %s was refused for the word %q, which the running build-agent does not read; the "+ + "merge %.8s of %s is kept waiting for a build-agent that reads it, and the build is asked again when one is delivered\n", + result.ID, held.Module, held.Word, merge.Commit, merge.Repository) + return true, nil +} + // probeFirstBuildsID is the probe that reads the first merges waiting for mesh-delivery's word. const probeFirstBuildsID = "D18" // kindFirstBuildUnasked is what a first merge waiting too long for mesh-delivery's word raises. const kindFirstBuildUnasked = "first-build-unasked" +// kindFirstBuildHeld is what a first merge held for a build-agent that reads its manifest raises. +const kindFirstBuildHeld = "first-build-held" + // firstBuildWaitBound is how long a first merge waits for mesh-delivery's announcement of the operator's Approve before // it is said: a bound in the sense of ADR 0426 §3, not a delay. Nothing waits on it: the word's arrival asks the builds // at once, and the merge is said, never dropped, once it is past this (issue 1146). @@ -189,6 +257,10 @@ func firstBuildObservations(waiting []inventory.BatchedMerge, now time.Time) []c var out []conditions.Observation for _, w := range waiting { k := announcedOf(w) + if k.Held != nil { + out = append(out, heldFirstBuildObservation(k)) + continue + } if !k.FirstBuild || now.Sub(w.Heard) <= firstBuildWaitBound { continue } @@ -211,6 +283,25 @@ func firstBuildObservations(waiting []inventory.BatchedMerge, now time.Time) []c return out } +// heldFirstBuildObservation names a first merge held for a build-agent that reads its manifest: at once, since nothing +// is lost and nothing times out, but the operator can see what the merge waits for. +func heldFirstBuildObservation(k keptMerge) conditions.Observation { + repository := k.Owner + "/" + k.Repo + return conditions.Observation{Scope: conditions.ScopeMerge, ID: fmt.Sprintf("%s_%.8s", repository, k.Commit), + Kind: kindFirstBuildHeld, Severity: conditions.Warning, + Summary: fmt.Sprintf("the merge %.8s of %s adds the module %s, whose manifest uses the word %q: the running "+ + "build-agent does not read it, so the merge waits for a build-agent that reads it", k.Commit, repository, + k.Held.Module, k.Held.Word), + Said: fmt.Sprintf("%s at %s waits for a build-agent that reads %q (module %s)", repository, k.Commit, k.Held.Word, + k.Held.Module), + Headline: fmt.Sprintf("The first build of %s waits for a newer build-agent", k.Held.Module), + Explanation: fmt.Sprintf("The merge %.8s into %s adds the module %s, and its manifest uses the word %q, which "+ + "the build-agent now running does not know. The merge is kept, not failed: the first build is asked again "+ + "when a delivery of build-agent ends.", k.Commit, repository, k.Held.Module, k.Held.Word), + Resolved: "a delivery of build-agent ends and the first build registers the module, or a hand `build` does", + } +} + // probeFirstBuilds is D18: no first merge waits past its bound for mesh-delivery's word unsaid. func probeFirstBuilds(ctx context.Context, d *doctor) ([]conditions.Observation, error) { waiting, err := d.open.inventory.WaitingMerges(ctx) diff --git a/cmd/mesh-controller/first_builds_held_test.go b/cmd/mesh-controller/first_builds_held_test.go new file mode 100644 index 00000000..fbfe8db6 --- /dev/null +++ b/cmd/mesh-controller/first_builds_held_test.go @@ -0,0 +1,85 @@ +package main + +import ( + "encoding/json" + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/conditions" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +const refusedKeyTTL = `this is not a module manifest: a state is either a name or {name, history, ttl-seconds, per-machine}: ` + + `json: unknown field "key-ttl"` + +// aFirstMergeAnswered is a first merge as askFirstBuilds leaves it: answered by the record of merges no plan answers. +func aFirstMergeAnswered() (inventory.BuildRequest, inventory.BatchedMerge) { + req := inventory.BuildRequest{ID: "build-1", Repository: "novox/mesh-delivery", Seat: "git", Path: "watchman", Ref: "main", + Commit: "0177ea7a00", For: "merge"} + event, _ := json.Marshal(keptMerge{SourceMoved: link.SourceMoved{Owner: "novox", Repo: "mesh-delivery", Base: "main", + Commit: "0177ea7a00", Number: 3, HeadSHA: "a1a1a1a1a1a1"}, FirstBuild: true}) + return req, inventory.BatchedMerge{Repository: "novox/mesh-delivery", Branch: "main", Commit: "0177ea7a00", + Plan: inventory.WaitingRecord, Event: event} +} + +// 1173/AC1: a first build refused for a word its manifest uses and the running build-agent does not know holds its merge. +func TestAFirstBuildRefusedForAnUnknownWordHoldsItsMerge(t *testing.T) { + req, merge := aFirstMergeAnswered() + held := judgeRefusedFirstBuild(link.BuildResult{ID: "build-1", Failed: refusedKeyTTL}, req, merge) + if held == nil || held.Word != "key-ttl" || held.Module != "watchman" { + t.Fatalf("1173/AC1: a first build refused for \"key-ttl\" is judged %+v, want a hold on that word for watchman", held) + } +} + +// 1173/AC2: a held first merge is named at once by one condition: the merge, the module, the word and what it waits for. +func TestAHeldFirstMergeIsNamedByACondition(t *testing.T) { + now := time.Date(2026, 10, 22, 14, 0, 0, 0, time.UTC) + _, merge := aFirstMergeAnswered() + var k keptMerge + _ = json.Unmarshal(merge.Event, &k) + k.Held = &heldFirstBuild{Module: "watchman", Word: "key-ttl", At: now} + merge.Event, _ = json.Marshal(k) + merge.Heard = now.Add(-time.Second) + got := firstBuildObservations([]inventory.BatchedMerge{merge}, now) + if len(got) != 1 { + t.Fatalf("1173/AC2: %d conditions for a held first merge, want one: %+v", len(got), got) + } + o := got[0] + if o.Kind != kindFirstBuildHeld || o.Scope != conditions.ScopeMerge { + t.Fatalf("1173/AC2: the condition is %+v", o) + } + for _, want := range []string{"0177ea7a", "watchman", "key-ttl", "build-agent"} { + if !strings.Contains(o.Summary, want) { + t.Fatalf("1173/AC2: the summary %q does not name %q", o.Summary, want) + } + } +} + +// 1173/AC4: a first build that fails for any other reason is not held. +func TestAFirstBuildFailingForAnyOtherReasonIsNotHeld(t *testing.T) { + req, merge := aFirstMergeAnswered() + if judgeRefusedFirstBuild(link.BuildResult{Failed: refusedKeyTTL}, req, merge) == nil { + t.Fatal("1173/AC4: the control, a refusal for an unknown word, is not held") + } + for _, c := range []struct { + what string + failed string + change func(*inventory.BuildRequest, *inventory.BatchedMerge) + }{ + {what: "a compile error", failed: "main.go:3: undefined: foo"}, + {what: "no failure", failed: ""}, + {what: "a missing field", failed: `json: cannot unmarshal string into Go struct field`}, + {what: "a build a person asked", failed: refusedKeyTTL, change: func(r *inventory.BuildRequest, _ *inventory.BatchedMerge) { r.For = "build" }}, + {what: "a merge a plan answers", failed: refusedKeyTTL, change: func(_ *inventory.BuildRequest, m *inventory.BatchedMerge) { m.Plan = "plan-7" }}, + } { + r, m := req, merge + if c.change != nil { + c.change(&r, &m) + } + if held := judgeRefusedFirstBuild(link.BuildResult{Failed: c.failed}, r, m); held != nil { + t.Fatalf("1173/AC4: %s is held: %+v", c.what, held) + } + } +} diff --git a/cmd/mesh-controller/main.go b/cmd/mesh-controller/main.go index d3f380b2..7f204dac 100644 --- a/cmd/mesh-controller/main.go +++ b/cmd/mesh-controller/main.go @@ -408,6 +408,13 @@ func (b builds) Built(ctx context.Context, result link.BuildResult) error { fmt.Printf("%s: a dry run of %s on %s, not taken in\n", result.ID, result.Repository, result.Ref) return nil } + // **A first build refused for a word the running build-agent does not read waits for one that does** (novox/hq ADR + // 0482 §5, issue 1173): its merge is kept, named, and never failed; no failed build is recorded. + if held, err := holdRefusedFirstBuild(ctx, b.inv, result); err != nil { + return err + } else if held { + return nil + } manifest, _, err := takeIn(ctx, b.inv, result) // The assignments pending on this build, made or ended (novox/hq issue 325), said in the daemon's log // after what became of the build. diff --git a/internal/inventory/batches.go b/internal/inventory/batches.go index 7ca63dce..8d5cf20e 100644 --- a/internal/inventory/batches.go +++ b/internal/inventory/batches.go @@ -114,6 +114,16 @@ func (i *Inventory) AnswerMerge(ctx context.Context, repository, commit, plan st return i.chainSavePlans(ctx, c, nil, []MergeMove{{Repository: repository, Commit: commit, Plan: plan, Alone: alone}}) } +// HoldMerge puts a merge back to waiting, no plan answering it, with the event it is kept under replaced (novox/hq issue 1173: +// a first merge held for a build-agent that reads its manifest, the hold said in its event). +func (i *Inventory) HoldMerge(ctx context.Context, repository, commit string, event json.RawMessage) error { + c, err := i.theChain(ctx) + if err != nil { + return err + } + return i.chainSavePlans(ctx, c, nil, []MergeMove{{Repository: repository, Commit: commit, Event: event}}) +} + // Batches is every batch not yet cut: assembling or queued, oldest first. One at a time is the rule; more // are read so a fault is seen. func (i *Inventory) Batches(ctx context.Context) ([]Plan, error) { diff --git a/internal/inventory/chain_rollouts.go b/internal/inventory/chain_rollouts.go index 3988f558..4dfd89fe 100644 --- a/internal/inventory/chain_rollouts.go +++ b/internal/inventory/chain_rollouts.go @@ -758,6 +758,9 @@ func (i *Inventory) chainSavePlans(ctx context.Context, c *chain.Chain, plans [] if m.Plan == "" { f := w.m f.Waiting, f.Alone = true, m.Alone + if m.Event != nil { + f.Event = m.Event + } editOf(w.plan).flag[key] = f continue } diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index ac646f7e..109b6104 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -2,6 +2,7 @@ package inventory import ( "context" + "encoding/json" "errors" "fmt" "slices" @@ -677,6 +678,8 @@ func (i *Inventory) SavePlan(ctx context.Context, p *Plan) error { type MergeMove struct { Repository, Commit, Plan string Alone bool + // Event replaces the merge's kept announcement when it is put back to waiting (Plan empty); nil leaves it. + Event json.RawMessage } // SavePlansMoving writes plans, as SavePlan does each, and gives merges to the plans that answer them, all in one From 4538d057325c10a273d6568774479f924c756c4b Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 16:19:45 +0200 Subject: [PATCH 2/4] Ask a held first build again when a delivery of build-agent ends (issue 1173) ADR 0482 section 5: the build waiting for a build-agent that reads its manifest is asked once the plan delivering it ends. --- cmd/mesh-controller/first_builds.go | 55 +++++++++++ .../first_builds_held_integration_test.go | 91 +++++++++++++++++++ cmd/mesh-controller/first_builds_held_test.go | 22 +++++ cmd/mesh-controller/plan_looks.go | 4 + 4 files changed, 172 insertions(+) create mode 100644 cmd/mesh-controller/first_builds_held_integration_test.go diff --git a/cmd/mesh-controller/first_builds.go b/cmd/mesh-controller/first_builds.go index f173948d..961e20cf 100644 --- a/cmd/mesh-controller/first_builds.go +++ b/cmd/mesh-controller/first_builds.go @@ -310,3 +310,58 @@ func probeFirstBuilds(ctx context.Context, d *doctor) ([]conditions.Observation, } return firstBuildObservations(waiting, time.Now().UTC()), nil } + +// askHeldFirstBuildsAgain asks again the first build of each first merge held for a build-agent that reads its manifest, +// when the plan that ended delivered build-agent (novox/hq issue 1173): once for each end, since the ask answers the merge +// and a refusal that comes again holds it anew. Answers how many merges it asked for. +func askHeldFirstBuildsAgain(ctx context.Context, open *stores, p inventory.Plan) int { + if !deliversBuildAgent(p) { + return 0 + } + actingOnMerges.Lock() + defer actingOnMerges.Unlock() + inv := open.inventory + waiting, err := inv.WaitingMerges(ctx) + if err != nil { + fmt.Printf("%s delivered build-agent, and the first merges held for it could not be read: %v\n", p.ID, err) + return 0 + } + asked := 0 + for _, w := range waiting { + k := announcedOf(w) + if k.Held == nil { + continue + } + entries, err := inv.Catalogued(ctx) + if err != nil { + fmt.Printf("%s delivered build-agent, and the catalogue could not be read: %v\n", p.ID, err) + return asked + } + read, err := readForPlanning(ctx, inv) + if err != nil { + fmt.Printf("%s delivered build-agent, and what the builds read could not be read: %v\n", p.ID, err) + return asked + } + added, why, err := firstBuildsOf(ctx, inv, k.SourceMoved, entries, read) + if err != nil || len(added) == 0 { + fmt.Printf("%s delivered build-agent, and the merge %.8s of %s held for it asks no build now (%v): it stays "+ + "waiting\n", p.ID, w.Commit, w.Repository, err) + continue + } + fmt.Printf("%s delivered build-agent: the first build of %s, held for the word %q, is asked again\n", p.ID, + k.Held.Module, k.Held.Word) + if err := askFirstBuilds(ctx, inv, k.SourceMoved, time.Now().UTC(), added, why); err != nil { + fmt.Printf(" and could not be asked: %v\n", err) + continue + } + asked++ + } + return asked +} + +// deliversBuildAgent says a delivery plan ended delivered with build-agent built in it: the reader a held first build +// waits for is the one now running. +func deliversBuildAgent(p inventory.Plan) bool { + m := p.Modules["build-agent"] + return p.WalkSaid == inventory.WalkEnded && p.Outcome() == inventory.WalkDelivered && m != nil && m.State == "built" +} diff --git a/cmd/mesh-controller/first_builds_held_integration_test.go b/cmd/mesh-controller/first_builds_held_integration_test.go new file mode 100644 index 00000000..8fb33193 --- /dev/null +++ b/cmd/mesh-controller/first_builds_held_integration_test.go @@ -0,0 +1,91 @@ +//go:build integration + +package main + +import ( + "context" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/asks" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// 1173/AC1, 1173/AC3 whole against the store: a first merge's build refused for a word the running build-agent does not +// read keeps the merge waiting with no failed build recorded; a delivery plan that does not deliver build-agent asks +// nothing; the one that ends delivering it asks the first build again once, and its pass registers the module. +func TestAHeldFirstMergeIsAskedAgainWhenABuildAgentIsDelivered(t *testing.T) { + const head, commit = "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1", "0177ea7a00000000" + open := aMesh(t) + ctx := t.Context() + inv := open.inventory + + a := aDeliveryMergeOf(t, head, map[string]string{"novox/mesh-delivery@a1a1a1a1a1a1": head}) + wasKept, wasAsk, realAsk := keptDeliveryMerge, routerAskOf, askABuild + keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) { + return a.merge, true, nil + } + routerAskOf = func(context.Context, catalogueReader, string, string) (*asks.Ask, *asks.Warrant, error) { + return &a.ask, &a.warrant, nil + } + var asked []string + askABuild = func(_ context.Context, source buildSource, path, ref string) (string, error) { + id := "build-" + string(rune('1'+len(asked))) + asked = append(asked, id) + return id, nil + } + t.Cleanup(func() { keptDeliveryMerge, routerAskOf, askABuild = wasKept, wasAsk, realAsk }) + + m := link.SourceMoved{Owner: "novox", Repo: "mesh-delivery", Base: "main", HeadSHA: head, Commit: commit, Number: 7, + MergedAt: time.Now().Add(-time.Hour).UTC().Format(time.RFC3339Nano), Paths: []string{"module.json", "main.go"}, + ModuleDirs: []string{"."}, ModuleDirsSaid: true} + if err := (following{open: open}).SourceMoved(ctx, m); err != nil { + t.Fatal(err) + } + if len(asked) != 1 { + t.Fatalf("1173/AC3: the first merge asked %v, want the one first build", asked) + } + + // The running build-agent refuses the manifest's word. + refused := onTrunk("build-1", "novox/mesh-delivery", "git", "", nil) + refused.Failed, refused.Manifest, refused.Commit = refusedKeyTTL, nil, commit + if err := (builds{inv: inv, open: open}).Built(ctx, refused); err != nil { + t.Fatal(err) + } + if _, recorded, _ := inv.BuildByID(ctx, "build-1"); recorded { + t.Fatal("1173/AC1: a refused first build was recorded as a failed build") + } + waiting, err := inv.WaitingMerges(ctx) + if err != nil || len(waiting) != 1 || announcedOf(waiting[0]).Held == nil { + t.Fatalf("1173/AC1: the merges waiting are %+v (%v), want the first merge, held", waiting, err) + } + if obs := firstBuildObservations(waiting, time.Now()); len(obs) != 1 || obs[0].Kind != kindFirstBuildHeld { + t.Fatalf("1173/AC2: the held merge is named by %+v", obs) + } + + // A plan that ended without delivering build-agent asks nothing; one that delivered it asks once. + other := inventory.Plan{ID: "p1", State: inventory.PlanDone, WalkSaid: inventory.WalkEnded, + Modules: map[string]*inventory.PlanModule{"mesh-delivery": {State: "built"}}} + if n := askHeldFirstBuildsAgain(ctx, open, other); n != 0 || len(asked) != 1 { + t.Fatalf("1173/AC3: a plan that delivered no build-agent asked %d builds again (%v)", n, asked) + } + delivered := inventory.Plan{ID: "p2", State: inventory.PlanDone, WalkSaid: inventory.WalkEnded, + Modules: map[string]*inventory.PlanModule{"build-agent": {State: "built"}}} + if n := askHeldFirstBuildsAgain(ctx, open, delivered); n != 1 || len(asked) != 2 { + t.Fatalf("1173/AC3: the plan that delivered build-agent asked %d builds again (%v), want one", n, asked) + } + if n := askHeldFirstBuildsAgain(ctx, open, delivered); n != 0 || len(asked) != 2 { + t.Fatalf("1173/AC3: the same end asked again: %d (%v)", n, asked) + } + + // Its pass registers the module. + pass := onTrunk("build-2", "novox/mesh-delivery", "git", "", map[string]any{"module": "mesh-delivery-watchman", "version": "1"}) + pass.Commit = commit + if err := (builds{inv: inv, open: open}).Built(ctx, pass); err != nil { + t.Fatal(err) + } + if _, known := inventoryHolds(t, inv, "mesh-delivery-watchman"); !known { + t.Fatal("1173/AC3: the module is not registered after the first build passed") + } +} diff --git a/cmd/mesh-controller/first_builds_held_test.go b/cmd/mesh-controller/first_builds_held_test.go index fbfe8db6..7190024d 100644 --- a/cmd/mesh-controller/first_builds_held_test.go +++ b/cmd/mesh-controller/first_builds_held_test.go @@ -83,3 +83,25 @@ func TestAFirstBuildFailingForAnyOtherReasonIsNotHeld(t *testing.T) { } } } + +// 1173/AC3 (the part that needs no store): only a plan that ended delivered with build-agent built in it is the end a held +// first build waits for. +func TestOnlyADeliveredPlanOfBuildAgentIsWhatAHeldFirstBuildWaitsFor(t *testing.T) { + plan := func(state, walk string, modules map[string]*inventory.PlanModule) inventory.Plan { + return inventory.Plan{ID: "p", State: state, WalkSaid: walk, Modules: modules} + } + built := map[string]*inventory.PlanModule{"build-agent": {State: "built"}} + if !deliversBuildAgent(plan(inventory.PlanDone, inventory.WalkEnded, built)) { + t.Fatal("1173/AC3: a plan that delivered build-agent is not the end a held first build waits for") + } + for what, p := range map[string]inventory.Plan{ + "a plan still running": plan(inventory.PlanBuilding, "", built), + "a plan that failed": plan(inventory.PlanFailed, inventory.WalkEnded, built), + "a failed build-agent": plan(inventory.PlanDone, inventory.WalkEnded, map[string]*inventory.PlanModule{"build-agent": {State: "failed"}}), + "another module's plan": plan(inventory.PlanDone, inventory.WalkEnded, map[string]*inventory.PlanModule{"mailu": {State: "built"}}), + } { + if deliversBuildAgent(p) { + t.Fatalf("1173/AC3: %s is the end a held first build waits for", what) + } + } +} diff --git a/cmd/mesh-controller/plan_looks.go b/cmd/mesh-controller/plan_looks.go index 855d6e02..c29e4065 100644 --- a/cmd/mesh-controller/plan_looks.go +++ b/cmd/mesh-controller/plan_looks.go @@ -209,6 +209,10 @@ func servingPlanSaved(ctx context.Context, bus link.Bus, open *stores) func(*inv keepBuildingKey(ctx, p) if p.WalkSaid == inventory.WalkEnded { go forgetChecksOfWalk(context.WithoutCancel(ctx), open.inventory, p) + // A first build refused for a word only a newer build-agent reads is asked again (novox/hq issue 1173). + if deliversBuildAgent(p) { + go askHeldFirstBuildsAgain(context.WithoutCancel(ctx), open, p) + } } } } From 45a09f90e31c5948ddc673b70efffac942955704 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 16:33:28 +0200 Subject: [PATCH 3/4] Carry a held merge's new announcement into its record (issue 1173) HoldMerge replaced the event on the flag only; the record kept the old one. --- internal/inventory/chain_rollouts.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/inventory/chain_rollouts.go b/internal/inventory/chain_rollouts.go index 4dfd89fe..ea69cdb7 100644 --- a/internal/inventory/chain_rollouts.go +++ b/internal/inventory/chain_rollouts.go @@ -634,6 +634,10 @@ func (e *recordEdit) apply(held []chain.HeldMerge) []chain.HeldMerge { } if f, has := e.flag[k]; has { h.Waiting, h.Alone = f.Waiting, f.Alone + // A merge put back to waiting may carry a new announcement (HoldMerge); every other flag copies its own. + if len(f.Event) > 0 { + h.Event = f.Event + } } out = append(out, h) } From 1e9b87e03acd30700b5284c3d01153cdc176c8ca Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 16:50:06 +0200 Subject: [PATCH 4/4] Hold every refused module of a first merge and ask each again by its own path (issue 1173) A held merge was asked again by the rule of a first merge, which finds nothing once a sibling module of the same merge registered, so it stayed held forever; and a second refusal in a merge already held fell through to a failed build. The hold now keeps a list of module, path and word, a refusal in a held waiting merge joins it, and the delivery of build-agent asks each held path again on the merge's Approve warrant. --- cmd/mesh-controller/batches.go | 6 +- cmd/mesh-controller/first_builds.go | 134 +++++++++++++----- .../first_builds_held_integration_test.go | 132 ++++++++++++++++- cmd/mesh-controller/first_builds_held_test.go | 40 +++++- 4 files changed, 267 insertions(+), 45 deletions(-) diff --git a/cmd/mesh-controller/batches.go b/cmd/mesh-controller/batches.go index 12bc80a1..ae7627a1 100644 --- a/cmd/mesh-controller/batches.go +++ b/cmd/mesh-controller/batches.go @@ -800,9 +800,9 @@ type keptMerge struct { // FirstBuild says the merge is a first merge into a repository the mesh builds nothing from, kept to ask the first // build of its modules once mesh-delivery's word on it is kept (novox/hq ADR 0432, issue 1146). FirstBuild bool `json:"first_build,omitempty"` - // Held says the first build was refused for a word the running build-agent does not read: the merge waits for a - // build-agent that reads it, and is never failed (novox/hq ADR 0482 §5, issue 1173). - Held *heldFirstBuild `json:"held,omitempty"` + // Held says which first builds of the merge were refused for a word the running build-agent does not read: the merge + // waits for a build-agent that reads them, and is never failed (novox/hq ADR 0482 §5, issue 1173). + Held []heldFirstBuild `json:"held,omitempty"` } // announcedOf is a kept merge as the forge announced it, and what it moved; empty where the record says none. diff --git a/cmd/mesh-controller/first_builds.go b/cmd/mesh-controller/first_builds.go index 961e20cf..fb05302b 100644 --- a/cmd/mesh-controller/first_builds.go +++ b/cmd/mesh-controller/first_builds.go @@ -103,7 +103,7 @@ func (f following) resumeFirstBuilds(ctx context.Context, d deliveryMerge) error } for _, w := range waiting { k := announcedOf(w) - if !k.FirstBuild || k.Held != nil || !strings.EqualFold(k.Owner+"/"+k.Repo, d.Owner+"/"+d.Repo) || k.Number != d.Number || + if !k.FirstBuild || len(k.Held) > 0 || !strings.EqualFold(k.Owner+"/"+k.Repo, d.Owner+"/"+d.Repo) || k.Number != d.Number || k.HeadSHA != d.HeadSHA { continue } @@ -177,18 +177,55 @@ func firstBuildsOf(ctx context.Context, inv catalogueReader, m link.SourceMoved, // and its merge is kept waiting for a build-agent that reads it (novox/hq ADR 0482 §5, issue 1173). type heldFirstBuild struct { Module string `json:"module"` + Path string `json:"path"` Word string `json:"word"` At time.Time `json:"at"` } -// judgeRefusedFirstBuild is the hold a first build's outcome puts its merge under: nil unless the build failed on a word -// of its manifest the running build-agent does not know. -func judgeRefusedFirstBuild(result link.BuildResult, req inventory.BuildRequest, merge inventory.BatchedMerge) *heldFirstBuild { +// judgeRefusedFirstBuild is the hold a first build's outcome puts its merge under: every module the merge holds already, +// and this one, unless it is among them; nil unless the build failed on a word of its manifest the running build-agent +// does not know, and was asked for a first merge, answered by the record of merges no plan answers or put back to waiting +// by an earlier hold (a second refusal in one merge is held too, review of issue 1173). +func judgeRefusedFirstBuild(result link.BuildResult, req inventory.BuildRequest, merge inventory.BatchedMerge) []heldFirstBuild { word := refusedWord(result.Failed) - if word == "" || req.For != "merge" || merge.Plan != inventory.WaitingRecord { + if word == "" || req.For != "merge" { return nil } - return &heldFirstBuild{Module: req.Name(), Word: word, At: time.Now().UTC()} + var held []heldFirstBuild + switch k := announcedOf(merge); { + case merge.Plan == inventory.WaitingRecord: + // Answered when its builds were asked: a hold from before (a build asked again) is over, and starts anew. + case merge.Plan == "" && len(k.Held) > 0: + held = k.Held + default: + return nil + } + this := heldFirstBuild{Module: req.Name(), Path: strings.Trim(req.Path, "/"), Word: word, At: time.Now().UTC()} + for i, h := range held { + if strings.Trim(h.Path, "/") == this.Path { + out := append([]heldFirstBuild(nil), held...) + out[i] = this + return out + } + } + return append(append([]heldFirstBuild(nil), held...), this) +} + +// heldDirsOf is the directories a held merge asks again: its held modules' own, each once, "." for the repository's root. +func heldDirsOf(k keptMerge) []string { + var dirs []string + seen := map[string]bool{} + for _, h := range k.Held { + dir := strings.Trim(h.Path, "/") + if dir == "" { + dir = "." + } + if !seen[dir] { + seen[dir] = true + dirs = append(dirs, dir) + } + } + return dirs } // unknownWord is how the strict reading of a manifest says a word is not one it knows: encoding/json's own words, which @@ -219,7 +256,7 @@ func holdRefusedFirstBuild(ctx context.Context, inv *inventory.Inventory, result return false, err } held := judgeRefusedFirstBuild(result, req, merge) - if held == nil { + if len(held) == 0 { return false, nil } k := announcedOf(merge) @@ -231,9 +268,15 @@ func holdRefusedFirstBuild(ctx context.Context, inv *inventory.Inventory, result if err := inv.HoldMerge(ctx, merge.Repository, merge.Commit, event); err != nil { return false, err } + this := held[len(held)-1] + for _, h := range held { + if h.Path == strings.Trim(req.Path, "/") { + this = h + } + } fmt.Printf("%s: the first build of %s was refused for the word %q, which the running build-agent does not read; the "+ - "merge %.8s of %s is kept waiting for a build-agent that reads it, and the build is asked again when one is delivered\n", - result.ID, held.Module, held.Word, merge.Commit, merge.Repository) + "merge %.8s of %s is kept waiting for a build-agent that reads it (holding %s), and the build is asked again when "+ + "one is delivered\n", result.ID, this.Module, this.Word, merge.Commit, merge.Repository, heldNames(held)) return true, nil } @@ -257,7 +300,7 @@ func firstBuildObservations(waiting []inventory.BatchedMerge, now time.Time) []c var out []conditions.Observation for _, w := range waiting { k := announcedOf(w) - if k.Held != nil { + if len(k.Held) > 0 { out = append(out, heldFirstBuildObservation(k)) continue } @@ -283,22 +326,34 @@ func firstBuildObservations(waiting []inventory.BatchedMerge, now time.Time) []c return out } -// heldFirstBuildObservation names a first merge held for a build-agent that reads its manifest: at once, since nothing -// is lost and nothing times out, but the operator can see what the merge waits for. +// heldNames is the modules a merge holds, each with the word it waits for: "watchman (key-ttl), view (per-machine)". +func heldNames(held []heldFirstBuild) string { + names := make([]string, 0, len(held)) + for _, h := range held { + names = append(names, fmt.Sprintf("%s (%q)", h.Module, h.Word)) + } + return strings.Join(names, ", ") +} + +// heldFirstBuildObservation names a first merge held for a build-agent that reads its manifests: at once, since nothing +// is lost and nothing times out, but the operator can see what the merge waits for. One condition names every module held. func heldFirstBuildObservation(k keptMerge) conditions.Observation { repository := k.Owner + "/" + k.Repo + modules := make([]string, 0, len(k.Held)) + for _, h := range k.Held { + modules = append(modules, h.Module) + } + held := heldNames(k.Held) return conditions.Observation{Scope: conditions.ScopeMerge, ID: fmt.Sprintf("%s_%.8s", repository, k.Commit), Kind: kindFirstBuildHeld, Severity: conditions.Warning, - Summary: fmt.Sprintf("the merge %.8s of %s adds the module %s, whose manifest uses the word %q: the running "+ - "build-agent does not read it, so the merge waits for a build-agent that reads it", k.Commit, repository, - k.Held.Module, k.Held.Word), - Said: fmt.Sprintf("%s at %s waits for a build-agent that reads %q (module %s)", repository, k.Commit, k.Held.Word, - k.Held.Module), - Headline: fmt.Sprintf("The first build of %s waits for a newer build-agent", k.Held.Module), - Explanation: fmt.Sprintf("The merge %.8s into %s adds the module %s, and its manifest uses the word %q, which "+ - "the build-agent now running does not know. The merge is kept, not failed: the first build is asked again "+ - "when a delivery of build-agent ends.", k.Commit, repository, k.Held.Module, k.Held.Word), - Resolved: "a delivery of build-agent ends and the first build registers the module, or a hand `build` does", + Summary: fmt.Sprintf("the merge %.8s of %s adds modules whose manifests use words the running build-agent does not "+ + "read: %s; the merge waits for a build-agent that reads them", k.Commit, repository, held), + Said: fmt.Sprintf("%s at %s waits for a build-agent that reads the words of %s", repository, k.Commit, held), + Headline: fmt.Sprintf("The first build of %s waits for a newer build-agent", strings.Join(modules, ", ")), + Explanation: fmt.Sprintf("The merge %.8s into %s adds modules whose manifests use words the build-agent now "+ + "running does not know: %s. The merge is kept, not failed: their first builds are asked again when a delivery "+ + "of build-agent ends.", k.Commit, repository, held), + Resolved: "a delivery of build-agent ends and the first builds register the modules, or a hand `build` does", } } @@ -329,29 +384,32 @@ func askHeldFirstBuildsAgain(ctx context.Context, open *stores, p inventory.Plan asked := 0 for _, w := range waiting { k := announcedOf(w) - if k.Held == nil { + if len(k.Held) == 0 { continue } - entries, err := inv.Catalogued(ctx) + // Its own held modules, not the rule of a first merge: a module of the merge that passed is registered by now, + // and the repository no longer counts as one the mesh builds nothing from (review of issue 1173). + why, err := approvedAnnounced(ctx, inv, k.Owner, k.Repo, keptMerge{SourceMoved: k.SourceMoved}) if err != nil { - fmt.Printf("%s delivered build-agent, and the catalogue could not be read: %v\n", p.ID, err) - return asked - } - read, err := readForPlanning(ctx, inv) - if err != nil { - fmt.Printf("%s delivered build-agent, and what the builds read could not be read: %v\n", p.ID, err) - return asked - } - added, why, err := firstBuildsOf(ctx, inv, k.SourceMoved, entries, read) - if err != nil || len(added) == 0 { fmt.Printf("%s delivered build-agent, and the merge %.8s of %s held for it asks no build now (%v): it stays "+ "waiting\n", p.ID, w.Commit, w.Repository, err) continue } - fmt.Printf("%s delivered build-agent: the first build of %s, held for the word %q, is asked again\n", p.ID, - k.Held.Module, k.Held.Word) - if err := askFirstBuilds(ctx, inv, k.SourceMoved, time.Now().UTC(), added, why); err != nil { - fmt.Printf(" and could not be asked: %v\n", err) + dirs := heldDirsOf(k) + fmt.Printf("%s delivered build-agent: the first builds of %s are asked again\n", p.ID, heldNames(k.Held)) + paths := make([]string, 0, len(dirs)) + for _, dir := range dirs { + if dir == "." { + dir = "" + } + paths = append(paths, dir) + } + repository := k.Owner + "/" + k.Repo + askBuilds(withFirstBuildWarrant(ctx, repository, paths, why), inv, k.SourceMoved, + buildSource{Repository: repository, Seat: gitSeat}, dirs) + // Answered: it no longer waits, so this end asks nothing twice; a refusal that comes again holds it anew. + if err := inv.AnswerMerge(ctx, w.Repository, w.Commit, inventory.WaitingRecord, false); err != nil { + fmt.Printf(" and the merge could not be marked answered: %v\n", err) continue } asked++ diff --git a/cmd/mesh-controller/first_builds_held_integration_test.go b/cmd/mesh-controller/first_builds_held_integration_test.go index 8fb33193..fb69d5e2 100644 --- a/cmd/mesh-controller/first_builds_held_integration_test.go +++ b/cmd/mesh-controller/first_builds_held_integration_test.go @@ -4,6 +4,8 @@ package main import ( "context" + "fmt" + "strings" "testing" "time" @@ -57,7 +59,7 @@ func TestAHeldFirstMergeIsAskedAgainWhenABuildAgentIsDelivered(t *testing.T) { t.Fatal("1173/AC1: a refused first build was recorded as a failed build") } waiting, err := inv.WaitingMerges(ctx) - if err != nil || len(waiting) != 1 || announcedOf(waiting[0]).Held == nil { + if err != nil || len(waiting) != 1 || len(announcedOf(waiting[0]).Held) == 0 { t.Fatalf("1173/AC1: the merges waiting are %+v (%v), want the first merge, held", waiting, err) } if obs := firstBuildObservations(waiting, time.Now()); len(obs) != 1 || obs[0].Kind != kindFirstBuildHeld { @@ -89,3 +91,131 @@ func TestAHeldFirstMergeIsAskedAgainWhenABuildAgentIsDelivered(t *testing.T) { t.Fatal("1173/AC3: the module is not registered after the first build passed") } } + +// aTwoModuleFirstMerge is a first merge into novox/mesh-delivery adding the modules watchman and view, made on the +// operator's Approve of its head, whose first builds are asked; asked says the build id asked for each path, in order. +func aTwoModuleFirstMerge(t *testing.T, open *stores, commit string) *[][2]string { + t.Helper() + const head = "b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2" + a := aDeliveryMergeOf(t, head, map[string]string{"novox/mesh-delivery@b2b2b2b2b2b2": head}) + wasKept, wasAsk, realAsk := keptDeliveryMerge, routerAskOf, askABuild + keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) { + return a.merge, true, nil + } + routerAskOf = func(context.Context, catalogueReader, string, string) (*asks.Ask, *asks.Warrant, error) { + return &a.ask, &a.warrant, nil + } + asked := &[][2]string{} + askABuild = func(_ context.Context, _ buildSource, path, _ string) (string, error) { + id := fmt.Sprintf("build-%d", len(*asked)+1) + *asked = append(*asked, [2]string{path, id}) + return id, nil + } + t.Cleanup(func() { keptDeliveryMerge, routerAskOf, askABuild = wasKept, wasAsk, realAsk }) + m := link.SourceMoved{Owner: "novox", Repo: "mesh-delivery", Base: "main", HeadSHA: head, Commit: commit, Number: 7, + MergedAt: time.Now().Add(-time.Hour).UTC().Format(time.RFC3339Nano), + Paths: []string{"view/module.json", "view/main.go", "watchman/module.json", "watchman/main.go"}, + ModuleDirs: []string{"view", "watchman"}, ModuleDirsSaid: true} + if err := (following{open: open}).SourceMoved(t.Context(), m); err != nil { + t.Fatal(err) + } + if len(*asked) != 2 { + t.Fatalf("the first merge asked %v, want the two first builds", *asked) + } + return asked +} + +// idOf is the build asked last for path. +func idOf(t *testing.T, asked [][2]string, path string) string { + t.Helper() + for i := len(asked) - 1; i >= 0; i-- { + if asked[i][0] == path { + return asked[i][1] + } + } + t.Fatalf("no build asked for %s: %v", path, asked) + return "" +} + +// built hands the controller a build's outcome: refused for the word, or passing as module. +func built(t *testing.T, open *stores, id, path, commit, refusedFor, module string) { + t.Helper() + r := onTrunk(id, "novox/mesh-delivery", "git", path, map[string]any{"module": module, "version": "1"}) + r.Commit = commit + if refusedFor != "" { + r.Failed, r.Manifest = `this is not a module manifest: json: unknown field "`+refusedFor+`"`, nil + } + if err := (builds{inv: open.inventory, open: open}).Built(t.Context(), r); err != nil { + t.Fatal(err) + } +} + +var buildAgentDelivered = inventory.Plan{ID: "p-ba", State: inventory.PlanDone, WalkSaid: inventory.WalkEnded, + Modules: map[string]*inventory.PlanModule{"build-agent": {State: "built"}}} + +// 1173/AC3 (review round 1, item 1): in a first merge of two modules where view passes and watchman is refused, the +// held watchman is asked again when build-agent is delivered, though the repository builds view by then. +func TestAHeldModuleIsAskedAgainAfterItsSiblingRegistered(t *testing.T) { + const commit = "0177ea7b00000000" + open := aMesh(t) + ctx := t.Context() + asked := aTwoModuleFirstMerge(t, open, commit) + + built(t, open, idOf(t, *asked, "view"), "view", commit, "", "mesh-delivery-view") + if _, known := inventoryHolds(t, open.inventory, "mesh-delivery-view"); !known { + t.Fatal("view is not registered after its first build passed") + } + refused := idOf(t, *asked, "watchman") + built(t, open, refused, "watchman", commit, "key-ttl", "") + if _, recorded, _ := open.inventory.BuildByID(ctx, refused); recorded { + t.Fatal("1173/AC1: the refused first build of watchman was recorded as a failed build") + } + waiting, err := open.inventory.WaitingMerges(ctx) + if err != nil || len(waiting) != 1 || len(announcedOf(waiting[0]).Held) != 1 { + t.Fatalf("1173/AC1: the merges waiting are %+v (%v), want the first merge, holding watchman", waiting, err) + } + + if n := askHeldFirstBuildsAgain(ctx, open, buildAgentDelivered); n != 1 || len(*asked) != 3 || (*asked)[2][0] != "watchman" { + t.Fatalf("1173/AC3: the delivery of build-agent asked %d merges again (%v), want watchman's first build alone", n, *asked) + } + built(t, open, idOf(t, *asked, "watchman"), "watchman", commit, "", "mesh-delivery-watchman") + if _, known := inventoryHolds(t, open.inventory, "mesh-delivery-watchman"); !known { + t.Fatal("1173/AC3: watchman is not registered after the first build asked again passed") + } +} + +// 1173/AC1, AC2, AC3 (review round 1, item 2): in a first merge of two modules both refused, neither is recorded as a +// failed build, the condition names both, and the delivery of build-agent asks both again. +func TestTwoRefusedModulesOfOneFirstMergeAreBothHeld(t *testing.T) { + const commit = "0177ea7c00000000" + open := aMesh(t) + ctx := t.Context() + asked := aTwoModuleFirstMerge(t, open, commit) + + for _, path := range []string{"watchman", "view"} { + id := idOf(t, *asked, path) + built(t, open, id, path, commit, "key-ttl", "") + if _, recorded, _ := open.inventory.BuildByID(ctx, id); recorded { + t.Fatalf("1173/AC1: the refused first build of %s was recorded as a failed build", path) + } + } + waiting, err := open.inventory.WaitingMerges(ctx) + if err != nil || len(waiting) != 1 || len(announcedOf(waiting[0]).Held) != 2 { + t.Fatalf("1173/AC1: the merges waiting are %+v (%v), want the first merge, holding both", waiting, err) + } + obs := firstBuildObservations(waiting, time.Now()) + if len(obs) != 1 || !strings.Contains(obs[0].Summary, "watchman") || !strings.Contains(obs[0].Summary, "view") { + t.Fatalf("1173/AC2: the held merge is named by %+v, want one condition naming both", obs) + } + + if n := askHeldFirstBuildsAgain(ctx, open, buildAgentDelivered); n != 1 || len(*asked) != 4 { + t.Fatalf("1173/AC3: the delivery of build-agent asked %d merges again (%v), want both first builds", n, *asked) + } + built(t, open, idOf(t, *asked, "watchman"), "watchman", commit, "", "mesh-delivery-watchman") + built(t, open, idOf(t, *asked, "view"), "view", commit, "", "mesh-delivery-view") + for _, module := range []string{"mesh-delivery-watchman", "mesh-delivery-view"} { + if _, known := inventoryHolds(t, open.inventory, module); !known { + t.Fatalf("1173/AC3: %s is not registered after the first build asked again passed", module) + } + } +} diff --git a/cmd/mesh-controller/first_builds_held_test.go b/cmd/mesh-controller/first_builds_held_test.go index 7190024d..96110263 100644 --- a/cmd/mesh-controller/first_builds_held_test.go +++ b/cmd/mesh-controller/first_builds_held_test.go @@ -28,7 +28,7 @@ func aFirstMergeAnswered() (inventory.BuildRequest, inventory.BatchedMerge) { func TestAFirstBuildRefusedForAnUnknownWordHoldsItsMerge(t *testing.T) { req, merge := aFirstMergeAnswered() held := judgeRefusedFirstBuild(link.BuildResult{ID: "build-1", Failed: refusedKeyTTL}, req, merge) - if held == nil || held.Word != "key-ttl" || held.Module != "watchman" { + if len(held) != 1 || held[0].Word != "key-ttl" || held[0].Module != "watchman" || held[0].Path != "watchman" { t.Fatalf("1173/AC1: a first build refused for \"key-ttl\" is judged %+v, want a hold on that word for watchman", held) } } @@ -39,7 +39,8 @@ func TestAHeldFirstMergeIsNamedByACondition(t *testing.T) { _, merge := aFirstMergeAnswered() var k keptMerge _ = json.Unmarshal(merge.Event, &k) - k.Held = &heldFirstBuild{Module: "watchman", Word: "key-ttl", At: now} + k.Held = []heldFirstBuild{{Module: "watchman", Path: "watchman", Word: "key-ttl", At: now}, + {Module: "view", Path: "view", Word: "per-machine", At: now}} merge.Event, _ = json.Marshal(k) merge.Heard = now.Add(-time.Second) got := firstBuildObservations([]inventory.BatchedMerge{merge}, now) @@ -50,7 +51,7 @@ func TestAHeldFirstMergeIsNamedByACondition(t *testing.T) { if o.Kind != kindFirstBuildHeld || o.Scope != conditions.ScopeMerge { t.Fatalf("1173/AC2: the condition is %+v", o) } - for _, want := range []string{"0177ea7a", "watchman", "key-ttl", "build-agent"} { + for _, want := range []string{"0177ea7a", "watchman", "key-ttl", "view", "per-machine", "build-agent"} { if !strings.Contains(o.Summary, want) { t.Fatalf("1173/AC2: the summary %q does not name %q", o.Summary, want) } @@ -73,6 +74,7 @@ func TestAFirstBuildFailingForAnyOtherReasonIsNotHeld(t *testing.T) { {what: "a missing field", failed: `json: cannot unmarshal string into Go struct field`}, {what: "a build a person asked", failed: refusedKeyTTL, change: func(r *inventory.BuildRequest, _ *inventory.BatchedMerge) { r.For = "build" }}, {what: "a merge a plan answers", failed: refusedKeyTTL, change: func(_ *inventory.BuildRequest, m *inventory.BatchedMerge) { m.Plan = "plan-7" }}, + {what: "a waiting merge held for nothing", failed: refusedKeyTTL, change: func(_ *inventory.BuildRequest, m *inventory.BatchedMerge) { m.Plan = "" }}, } { r, m := req, merge if c.change != nil { @@ -84,6 +86,38 @@ func TestAFirstBuildFailingForAnyOtherReasonIsNotHeld(t *testing.T) { } } +// 1173/AC1, AC2 (review round 1, item 2): a second refusal in a merge already held is held too, its module added to the +// hold, so no failed build is recorded and the condition names both. +func TestASecondRefusalInAHeldMergeIsAddedToTheHold(t *testing.T) { + req, merge := aFirstMergeAnswered() + first := judgeRefusedFirstBuild(link.BuildResult{Failed: refusedKeyTTL}, req, merge) + k := announcedOf(merge) + k.Held = first + merge.Event, _ = json.Marshal(k) + merge.Plan = "" // HoldMerge put it back to waiting + req.ID, req.Path = "build-2", "view" + held := judgeRefusedFirstBuild(link.BuildResult{Failed: `json: unknown field "per-machine"`}, req, merge) + if len(held) != 2 || held[0].Module != "watchman" || held[1].Module != "view" || held[1].Word != "per-machine" { + t.Fatalf("1173/AC1: the second refusal in a held merge is judged %+v, want watchman and view held", held) + } + // The same module refused again is held once. + k.Held = held + merge.Event, _ = json.Marshal(k) + if again := judgeRefusedFirstBuild(link.BuildResult{Failed: refusedKeyTTL}, req, merge); len(again) != 2 { + t.Fatalf("1173/AC1: view refused again is held as %+v, want it once", again) + } +} + +// 1173/AC3 (review round 1, item 1): a held merge is asked again for its own held modules' directories, not by the rule of +// a first merge, which finds nothing once another module of the repository is registered. +func TestAHeldMergeIsAskedAgainForItsHeldModulesAlone(t *testing.T) { + k := keptMerge{Held: []heldFirstBuild{{Module: "watchman", Path: "watchman"}, {Module: "mesh-delivery", Path: ""}, + {Module: "watchman", Path: "/watchman/"}}} + if got := heldDirsOf(k); strings.Join(got, ",") != "watchman,." { + t.Fatalf("1173/AC3: a held merge asks again %v, want watchman and the root once each", got) + } +} + // 1173/AC3 (the part that needs no store): only a plan that ended delivered with build-agent built in it is the end a held // first build waits for. func TestOnlyADeliveredPlanOfBuildAgentIsWhatAHeldFirstBuildWaitsFor(t *testing.T) {