From 1414ed7f7b81152aacf38ca9835d924c8fa35832 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 01:03:19 +0200 Subject: [PATCH] Judge an ask by the builder's say the walk read, never a second read (issue 1886, review round 1) A second read that failed, or that turned true in between, refused the ask and failed the whole plan. --- cmd/mesh-controller/builder_says_test.go | 40 ++++++++++++++++++++++++ cmd/mesh-controller/plan_keys.go | 14 +++++++++ cmd/mesh-controller/release_plan.go | 14 +++++++-- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/cmd/mesh-controller/builder_says_test.go b/cmd/mesh-controller/builder_says_test.go index 1fc5c5ab..cafe16cd 100644 --- a/cmd/mesh-controller/builder_says_test.go +++ b/cmd/mesh-controller/builder_says_test.go @@ -181,3 +181,43 @@ func TestTheReferenceWritersAreAskedOfBuildAgentWhateverTheBuilderSays(t *testin t.Fatalf("build-agent was asked %v", *asked) } } + +// 1886 review round 1 (ADR 0522 ยง2): the guard of the one ask function judges by the say askModule read, never a second +// read: a second read that fails, or that says otherwise, fails no plan. +func TestTheAskGuardUsesTheSayTheWalkRead(t *testing.T) { + was := askedModuleOf + t.Cleanup(func() { askedModuleOf = was }) + askedModuleOf = func(_ context.Context, _ buildSource, path string) (string, error) { + return strings.TrimPrefix(path, "modules/"), nil + } + reads := 0 + wasSays := builderSaysReferences + t.Cleanup(func() { builderSaysReferences = wasSays }) + for _, second := range []struct { + name string + says bool + err error + }{{"a second read that fails", false, errors.New("the bus did not answer")}, {"a say that turned true", true, nil}} { + reads = 0 + builderSaysReferences = func(context.Context) (bool, error) { + reads++ + if reads == 1 { + return false, nil + } + return second.says, second.err + } + wasAsk := askABuild + askABuild = func(ctx context.Context, source buildSource, path, _ string) (string, error) { + if err := onlyTheBuildersOwn(ctx, source, path); err != nil { + return "", err + } + return "build-9", nil + } + p, byName := aWalkOfOne("sudo") + askModule(t.Context(), p, "sudo", byName) + askABuild = wasAsk + if s := p.Modules["sudo"]; p.State == inventory.PlanFailed || s.Build != "build-9" || s.Key { + t.Errorf("%s: the walk %s, the module %+v", second.name, p.State, s) + } + } +} diff --git a/cmd/mesh-controller/plan_keys.go b/cmd/mesh-controller/plan_keys.go index ade65305..abe732bd 100644 --- a/cmd/mesh-controller/plan_keys.go +++ b/cmd/mesh-controller/plan_keys.go @@ -51,6 +51,20 @@ type planKeyReads interface { SaysReferences(ctx context.Context) (bool, error) } +// builderSayKey carries, on an ask's context, the builder's say the walk read before asking (withBuilderSay). +type builderSayKey struct{} + +// withBuilderSay hands the say a walk read to the ask's guard, so the guard never reads it a second time. +func withBuilderSay(ctx context.Context, says bool) context.Context { + return context.WithValue(ctx, builderSayKey{}, says) +} + +// builderSayOf is the say a walk handed the ask, and whether it handed one. +func builderSayOf(ctx context.Context) (says, handed bool) { + says, handed = ctx.Value(builderSayKey{}).(bool) + return says, handed +} + // builderSaysReferences is whether a running builder instance says, in its own bucket of instances, that the keys it // writes name references (issue 1886). Until one does, a walk asks build-agent for every module, as before R4e: the // keys the running builder writes could never be delivered from. A variable so a test needs no bus. diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 83024e40..fda995b1 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -457,8 +457,16 @@ func onlyTheBuildersOwn(ctx context.Context, source buildSource, path string) er if isBuildersOwn(module) { return nil } - // **While no running builder says it writes references, build-agent builds every module** (issue 1886). - if says, err := builderSaysReferences(ctx); err == nil && !says { + // **While no running builder says it writes references, build-agent builds every module** (issue 1886): judged by the + // say the walk read when it has one, read here only for an ask that carries none. + says, read := builderSayOf(ctx) + if !read { + var err error + if says, err = builderSaysReferences(ctx); err != nil { + says = true // not known: refused below, as before the say existed + } + } + if !says { return nil } if module == "" { @@ -547,6 +555,8 @@ func askModule(ctx context.Context, p *inventory.Plan, name string, byName map[s takeFromKey(p, state, e, now) return } + // The ask's guard judges by this say, never by a second read that could fail or differ (review of issue 1886). + ctx = withBuilderSay(ctx, says) } source := buildSource{Repository: e.Source.Repository, Seat: e.Source.Seat} fmt.Printf(" tier %d: ", p.Tier)