Judge an ask by the builder's say the walk read, never a second read (issue 1886, review round 1)
mesh/delivery-group group fix/1886-builder-says-references checking: 0 of 2 member(s) ready
mesh/merge-gate building on ace since 01:38
mesh/delivery superseded: a newer head of the same pull request

A second read that failed, or that turned true in between, refused the ask and
failed the whole plan.
This commit is contained in:
2026-10-23 01:03:19 +02:00
parent f67342f0d8
commit 1414ed7f7b
3 changed files with 66 additions and 2 deletions
+40
View File
@@ -181,3 +181,43 @@ func TestTheReferenceWritersAreAskedOfBuildAgentWhateverTheBuilderSays(t *testin
t.Fatalf("build-agent was asked %v", *asked) 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)
}
}
}
+14
View File
@@ -51,6 +51,20 @@ type planKeyReads interface {
SaysReferences(ctx context.Context) (bool, error) 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 // 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 // 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. // keys the running builder writes could never be delivered from. A variable so a test needs no bus.
+12 -2
View File
@@ -457,8 +457,16 @@ func onlyTheBuildersOwn(ctx context.Context, source buildSource, path string) er
if isBuildersOwn(module) { if isBuildersOwn(module) {
return nil return nil
} }
// **While no running builder says it writes references, build-agent builds every module** (issue 1886). // **While no running builder says it writes references, build-agent builds every module** (issue 1886): judged by the
if says, err := builderSaysReferences(ctx); err == nil && !says { // 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 return nil
} }
if module == "" { if module == "" {
@@ -547,6 +555,8 @@ func askModule(ctx context.Context, p *inventory.Plan, name string, byName map[s
takeFromKey(p, state, e, now) takeFromKey(p, state, e, now)
return 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} source := buildSource{Repository: e.Source.Repository, Seat: e.Source.Seat}
fmt.Printf(" tier %d: ", p.Tier) fmt.Printf(" tier %d: ", p.Tier)