From 980d396d40fde728af67b75732302a70ca597597 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 16:18:39 +0200 Subject: [PATCH] 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