From 3dc4307aec3fc18b1eeb4528bf2de298ce124938 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 09:04:45 +0100 Subject: [PATCH] The node's words for a want it composes nothing of are said once per reason, and proved on the live paths (issue 1966) The said-once keys no longer hold where the indexer's picture stands; a module composed from the manifest for want of a done build is named in Pass.Unmade; the agent's wait is told once per want and again after the composition was there; the two tests are folded into existing ones. Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i --- internal/agent/agent.go | 22 +++- internal/agent/agent_test.go | 52 ++++++---- internal/selfcompose/selfcompose.go | 68 ++++++++++--- internal/selfcompose/selfcompose_test.go | 122 ++++++++++++----------- 4 files changed, 168 insertions(+), 96 deletions(-) diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 01cd6db..4577e49 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -246,6 +246,8 @@ type Agent struct { // prior is a module the node ran before this agent started, so it has something to back up. prior map[string]bool said map[string]string + // waited is, by module, the want (fingerprint and revision) the agent said it waits for the composer for. + waited map[string]string // notComposed is the modules whose have says the node could not compose them: why, and the have before (NotComposed). notComposed map[string]notComposedMark // heldAlone is the modules the composer holds alone (NotComposedEach): what runs of each in the node's own @@ -311,7 +313,7 @@ func New(env Env) *Agent { env.Say = func(string) {} } return &Agent{env: env, wants: map[string]wantNow{}, comps: map[string]compNow{}, running: map[string]nodeengine.HaveByNodeModuleValue{}, applied: map[string]compNow{}, had: map[string]compNow{}, keptData: map[string]bool{}, - attempt: map[string]attempt{}, checked: map[string]bool{}, prior: map[string]bool{}, said: map[string]string{}, notComposed: map[string]notComposedMark{}, + attempt: map[string]attempt{}, checked: map[string]bool{}, prior: map[string]bool{}, said: map[string]string{}, waited: map[string]string{}, notComposed: map[string]notComposedMark{}, pulls: map[string]*pullWait{}, settledAt: map[string]time.Time{}, settledSeq: map[string]int{}, retry: make(chan []string, 8), reconcile: make(chan []string, 64), failedAgain: make(chan string, 8), tries: map[string]int{}, again: make(chan againAsk, 64), transient: make(chan struct{}, 1), stopped: make(chan struct{}), quiet: make(chan chan struct{})} @@ -1327,7 +1329,7 @@ func (a *Agent) HoldsNode() bool { // waitsForComposer says once per want and reason that the agent waits for the node's own composition of module at fp, and // why the composer made none (Env.WhyUnmade), and tells the composer (Env.Waiting) so it hands again what it believes it // handed. Called without a.mu held. -func (a *Agent) waitsForComposer(module, fp string) { +func (a *Agent) waitsForComposer(module, fp string, rev uint64) { why := "the composer has made none yet" if a.env.WhyUnmade != nil { if w := a.env.WhyUnmade(module); w != "" { @@ -1335,9 +1337,14 @@ func (a *Agent) waitsForComposer(module, fp string) { } } line := fmt.Sprintf("%s waits for the composer: no composition of it at %s: %s", module, fp, why) + // Said and told once per want (its fingerprint and revision), whatever the reason says; forgotten once composed. + at := fmt.Sprintf("%s@%d", fp, rev) a.mu.Lock() - first := a.said["composer:"+module] != line - a.sayOnce("composer:"+module, line) + first := a.waited[module] != at + if first { + a.waited[module] = at + a.env.Say(line) + } a.mu.Unlock() if first && a.env.Waiting != nil { a.env.Waiting(module, fp) @@ -1359,9 +1366,14 @@ func (a *Agent) settleOne(ctx context.Context, module string) { comp, have := a.compositionFor(module, w.Want.Fp) a.mu.Unlock() if a.env.SelfComposed { + if have { + a.mu.Lock() + delete(a.waited, module) + a.mu.Unlock() + } if !have || comp.Err != nil { if !have { - a.waitsForComposer(module, w.Want.Fp) + a.waitsForComposer(module, w.Want.Fp, w.Rev) } return // waits for the node's own composition of the want's build } diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index 4285750..74c5d69 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -1556,6 +1556,11 @@ func TestTheNodesOwnCompositionIsAppliedWhenItDiffersAndARecordMoveWaitsForAYes( history[key] = append(history[key], Update{Key: key, Value: raw, Revision: uint64(len(history[key]) + 1)}) } b.env.Held = func() map[string]string { return map[string]string{"mesh-build-go": "its binding is unset"} } + // Issue 1966 (novox, 2026-11-09): a want the composer made no composition of is said once, with why, and the composer told. + var toldMu sync.Mutex + var told []string + b.env.WhyUnmade = func(string) string { return "no manifest in the composer's picture" } + b.env.Waiting = func(module, fp string) { toldMu.Lock(); told = append(told, module+"@"+fp); toldMu.Unlock() } stop := b.run() defer stop() unsigned := func(module, fp string, rs ...json.RawMessage) (string, []byte, string) { @@ -1583,6 +1588,33 @@ func TestTheNodesOwnCompositionIsAppliedWhenItDiffersAndARecordMoveWaitsForAYes( b.comps.put(ck, craw) want("web", fp, "") eventually(t, "the node's own composition was not applied", func() bool { return len(b.appliedList()) == 1 }) + gk := meshwants.WantsByWantNodeModuleKey(node, "ghost") + graw, _ := bucketstest.Want(meshwants.WantsByWantNodeModuleValue{Fp: fpOf("9"), Self: true}) + b.wants.put(gk, graw) + line := "ghost waits for the composer: no composition of it at " + fpOf("9") + ": no manifest in the composer's picture" + saidCount := func() (n int) { + b.mu.Lock() + defer b.mu.Unlock() + for _, l := range b.said { + if l == line { + n++ + } + } + return n + } + eventually(t, "the wait for the composer was not said", func() bool { return saidCount() == 1 }) + // Settled again by another want's change, it is neither said nor told again. + want("web", fp, "") + b.wants.put(gk, graw) + eventually(t, "the composer was not told", func() bool { toldMu.Lock(); defer toldMu.Unlock(); return len(told) == 1 }) + if _, err := b.ag.Composition(context.Background(), "ghost", "", false); err == nil || !strings.Contains(err.Error(), "waits for the composer: no manifest in the composer's picture") { + t.Errorf("the composition read of ghost says %v, want why the composer made none", err) + } + toldMu.Lock() + if saidCount() != 1 || len(told) != 1 || told[0] != "ghost@"+fpOf("9") { + t.Errorf("said %d times, told %q; want once and ghost told once", saidCount(), told) + } + toldMu.Unlock() eventually(t, "the have does not name the composed digest", func() bool { return b.haves.last("web").Composed == dig }) // ADR 0601 §4: what the composer keeps of web as it runs is carried on its have, and goes once it keeps none. b.ag.KeptNow(context.Background(), map[string][]string{"web": {"web runs the tool runner's process here"}}) @@ -1935,23 +1967,3 @@ func testReconcileAppliesARunningModuleAgainAndLeavesTheHeldAndTheUnwantedAlone( t.Fatalf("a failed apply was tried again with no edge: %v", got) } } - -// Issue 1966 (novox, 2026-11-09): an agent that waits for its node's own composition of a want says so once, with why the -// composer made none, and tells the composer, which hands again what it believes it handed. -func TestAnAgentWaitingForTheComposerSaysSoOnceAndTellsIt(t *testing.T) { - b := newBed(t) - b.env.SelfComposed = true - var said []string - b.env.Say = func(s string) { said = append(said, s) } - b.env.WhyUnmade = func(string) string { return "no manifest in the composer's picture" } - var told []string - b.env.Waiting = func(module, fp string) { told = append(told, module+"@"+fp) } - a := New(b.env) - for range 2 { - a.waitsForComposer("web", fpOf("a")) - } - want := "web waits for the composer: no composition of it at " + fpOf("a") + ": no manifest in the composer's picture" - if len(said) != 1 || said[0] != want || len(told) != 1 || told[0] != "web@"+fpOf("a") { - t.Errorf("said %q, told %q; want %q once and web told once", said, told, want) - } -} diff --git a/internal/selfcompose/selfcompose.go b/internal/selfcompose/selfcompose.go index d12b72a..d07bdfa 100644 --- a/internal/selfcompose/selfcompose.go +++ b/internal/selfcompose/selfcompose.go @@ -156,6 +156,8 @@ type Composer struct { handed map[string]string // composed key to the digest handed // notTaken is, by module, why a want of this node's was not taken (it could not be decoded, or names no fingerprint). notTaken map[string]string + // unmadeSaid is, by module, the stable part of the reason its last journal line said (without where the picture stands). + unmadeSaid map[string]string // modulesRev is the revision of the last update of the indexer's modules taken: where the picture of them stands. modulesRev uint64 // rehand asks for a pass that hands again what the agent waits for (Waiting). @@ -351,7 +353,7 @@ func New(env Env) *Composer { } return &Composer{env: env, wants: map[string]meshwants.WantsByWantNodeModuleValue{}, assignments: map[string]meshmoduleassigner.Assignments{}, assignmentKeys: meshmoduleassignerclient.NewKeys(), records: map[string]nodeengine.NodeRecord{}, inOwn: map[string]bool{}, modules: map[string]meshcatalog.ModulesByModuleValue{}, values: map[string]map[string]any{}, - manifests: map[string][]byte{}, archives: map[string][]byte{}, builds: map[string]builds.BuilderBuild{}, handed: map[string]string{}, notTaken: map[string]string{}, rehand: make(chan struct{}, 1), sealedOwn: map[string]ownSealed{}, + manifests: map[string][]byte{}, archives: map[string][]byte{}, builds: map[string]builds.BuilderBuild{}, handed: map[string]string{}, notTaken: map[string]string{}, unmadeSaid: map[string]string{}, rehand: make(chan struct{}, 1), sealedOwn: map[string]ownSealed{}, pkgs: map[string]compositions.Composed{}, asked: make(chan chan Pass, 8)} } @@ -1051,8 +1053,9 @@ func (c *Composer) compose(ctx context.Context) []agent.Update { } c.paid() pass.Held, pass.holds = held, holds - pass.Unmade = c.unmadeOf(fps, compositions, held, skipped, own, inOwn, fromManifest, &keptMu) - c.sayUnmade(pass.Unmade) + var unmadeKeys map[string]string + pass.Unmade, unmadeKeys = c.unmadeOf(fps, compositions, held, skipped, own, inOwn, fromManifest, &keptMu) + c.sayUnmade(pass.Unmade, unmadeKeys) // The provisioner client's account the node runs with, dropped by this pass, is said: every filled write is refused // under a declaration that names none (filling.JudgeAccount), and no module is handed to it (issue 774, ace, // 2026-11-04 01:29). A placed client keeps its account while it is held (compose.Node.RunningProvisioner). @@ -2072,7 +2075,7 @@ func (c *Composer) seatsOf(ctx context.Context, r reader, module string) []strin // unmadeOf is each module wanted at a fingerprint (fps) of which the pass made no composition at it, with why, apart from // the modules held alone (Pass.Unmade), and each want of this node's that was not taken. func (c *Composer) unmadeOf(fps map[string]string, comps map[string]engcomp.Composed, held map[string]string, skipped []string, - own meshmoduleassigner.Assignments, inOwn []string, fromManifest map[string]string, mu *sync.Mutex) map[string]string { + own meshmoduleassigner.Assignments, inOwn []string, fromManifest map[string]string, mu *sync.Mutex) (map[string]string, map[string]string) { made := map[string]string{} for _, comp := range comps { if comp.Module != "" { @@ -2087,51 +2090,86 @@ func (c *Composer) unmadeOf(fps map[string]string, comps map[string]engcomp.Comp defer mu.Unlock() c.mu.Lock() defer c.mu.Unlock() - out := map[string]string{} + out, keys := map[string]string{}, map[string]string{} for module, fp := range fps { - if _, isHeld := held[module]; isHeld || made[module] == fp { + if _, isHeld := held[module]; isHeld { continue } - var why string + if made[module] == fp { + // Composed, but from the indexer's manifest and not a build: said, since what it names no artifacts of is no module + // that runs. + if w, ok := fromManifest[module]; ok { + out[module], keys[module] = w, fp+" "+w + } + continue + } + var why, key string switch m, in := c.modules[module]; { case compose.Folds(slices.Contains(inOwn, module), fp): why = "it is composed inside the node's own composition at " + fp + key = why case !placed[module]: why = fmt.Sprintf("the seat's assignment places no %s on this node (%d modules placed)", module, len(own.Modules)) + key = "not placed" case !in: why = fmt.Sprintf("no manifest in the composer's picture of the indexer's modules (%d modules, last update at revision %d)", len(c.modules), c.modulesRev) + key = "no manifest" case slices.Contains(skipped, module): why = "the composition skipped it: the declaration gives it no fingerprint (indexer's picture: " + m.Fp2 + ")" + key = why default: - why = fmt.Sprintf("the composition made nothing of it at %s (indexer's picture: %s, revision %d)", fp, m.Fp2, c.modulesRev) + if at := made[module]; at != "" { + why = fmt.Sprintf("the pass composed it at %s and the want names %s (indexer's picture: %s, revision %d)", at, fp, m.Fp2, c.modulesRev) + key = "composed at " + at + } else { + why = fmt.Sprintf("the composition made nothing of it at %s (indexer's picture: %s, revision %d)", fp, m.Fp2, c.modulesRev) + key = "made nothing at " + m.Fp2 + } } if w, ok := fromManifest[module]; ok { why += "; " + w + key += "; " + w } - out[module] = why + out[module], keys[module] = why, fp+" "+key } for module, why := range c.notTaken { if _, wanted := fps[module]; !wanted { out[module] = "want not taken: " + why + keys[module] = out[module] } } if len(out) == 0 { - return nil + return nil, keys } - return out + return out, keys } -// sayUnmade says each change of what the pass composed none of, once. -func (c *Composer) sayUnmade(unmade map[string]string) { - if len(unmade) == 0 { +// sayUnmade says what the pass composed none of when a module's reason changed (keys: its reason without where the +// picture of the indexer's modules stands, so a stuck want is not said again at every update of any module), and forgets a +// module once it is composed. +func (c *Composer) sayUnmade(unmade, keys map[string]string) { + c.mu.Lock() + changed := false + for m := range c.unmadeSaid { + if _, still := unmade[m]; !still { + delete(c.unmadeSaid, m) + } + } + for m, k := range keys { + if c.unmadeSaid[m] != k { + c.unmadeSaid[m], changed = k, true + } + } + c.mu.Unlock() + if !changed { return } var lines []string for _, m := range heldModules(unmade) { lines = append(lines, m+": "+unmade[m]) } - c.sayOnceEach("the node composes nothing of " + strings.Join(lines, "; ")) + c.env.Say("the node composes nothing of " + strings.Join(lines, "; ")) } // Waiting is the agent's word that it waits for the composition of module at fp, which it holds none of. When the composer diff --git a/internal/selfcompose/selfcompose_test.go b/internal/selfcompose/selfcompose_test.go index b7617b7..277f4e2 100644 --- a/internal/selfcompose/selfcompose_test.go +++ b/internal/selfcompose/selfcompose_test.go @@ -280,6 +280,72 @@ func TestTheNodeComposesItsWantsFromTheBus(t *testing.T) { b.mu.Unlock() t.Fatalf("handed %v, want %s and %s", got, lemursKey, nodeKey) } + // Issue 1966 (novox, 2026-11-09): every pass names each wanted module it composed nothing of, or composed from the indexer's + // manifest for want of a done build, with why, and a want it did not take; said once; the agent's word that it waits + // hands a composition the node believes handed again. + tfp := meshcatalogclient.SourcePrefix2 + strings.Repeat("7", 64) + tool := `{"module":"tool","version":"1","capabilities":["systemd"],"build":{"artifacts":[{"name":"files","kind":"archive","from":"files"}]},` + + `"resources":[{"id":"service","type":"service","unit":"tool.service","state":"running","boot":"enabled"}]}` + tdigest := meshcatalogclient.ManifestDigest([]byte(tool)) + tman, _ := meshcatalogclient.EncodeManifest(meshcatalog.ManifestsByDigestValue{Module: "tool", Manifest: tool}) + tkey, _ := meshcatalogclient.ManifestsKey(tdigest) + b.mu.Lock() + b.kv[buckets.Manifests+"/"+tkey] = tman + b.mu.Unlock() + tmod, _ := meshcatalogclient.EncodeModule(meshcatalog.ModulesByModuleValue{Fp2: tfp, Manifest: tdigest, Repository: "novox/t", Commit: "c"}) + c.takeModule(agent.Update{Key: "tool", Value: tmod, Revision: 9}) + for module, w := range map[string]meshwants.WantsByWantNodeModuleValue{"tool": {Fp: tfp, Self: true}, "ghost": {Fp: fp, Self: true}, "nofp": {Self: true}} { + raw, _ := bucketstest.Want(w) + c.takeWant(agent.Update{Key: "want.n1." + module, Value: raw}) + } + c.mu.Lock() + placed := c.assignments["n1"] + placed.Modules = append(slices.Clone(placed.Modules), meshmoduleassigner.PlacedModule{Module: "tool"}) + c.assignments["n1"] = placed + c.mu.Unlock() + said := func(sub string) (n int) { + b.mu.Lock() + defer b.mu.Unlock() + for _, l := range b.said { + if strings.Contains(l, sub) { + n++ + } + } + return n + } + c.compose(ctx) + why := map[string]string{"tool": c.WhyUnmade("tool"), "ghost": c.WhyUnmade("ghost"), "nofp": c.WhyUnmade("nofp"), "lemurs": c.WhyUnmade("lemurs")} + if !strings.Contains(why["tool"], "the builder holds no done build of it at "+tfp) || !strings.Contains(why["ghost"], "places no ghost") || + !strings.Contains(why["nofp"], "want not taken: its want names no fingerprint") || why["lemurs"] != "" { + t.Errorf("the pass says it composed nothing of %q", why) + } + // An update of another module moves the picture's revision: the same reasons are not said again. + c.takeModule(agent.Update{Key: "lemurs", Value: mod, Revision: 10}) + c.compose(ctx) + if n := said("the node composes nothing of ghost"); n != 1 { + t.Errorf("the journal says why ghost is not composed %d times, want once", n) + } + // The state of this step is taken away again, and the pass that takes back what it handed is made here. + for _, module := range []string{"tool", "ghost", "nofp"} { + c.takeWant(agent.Update{Key: "want.n1." + module, Deleted: true}) + } + c.takeModule(agent.Update{Key: "tool", Deleted: true}) + c.mu.Lock() + placed.Modules = placed.Modules[:len(placed.Modules)-1] + c.assignments["n1"] = placed + c.mu.Unlock() + c.compose(ctx) + // The agent waits for what the node believes it handed: handed again; for what it never handed, nothing is. + c.Waiting("ghost", fp) + select { + case u := <-out: + t.Errorf("handed %s for a wait of a composition never handed", u.Key) + default: + } + c.Waiting("lemurs", fp) + if u := next(t, out); !strings.Contains(u.Key, "lemurs") { + t.Errorf("handed %s, want lemurs's composition again", u.Key) + } // ADR 0601 §1: the module composed today at the build it runs, as the agent compares it with what runs. The same inputs give // the digest handed; a changed input (here its manifest's service, as a changed setting would change a file) another; a // build that cannot be composed (no done build or manifest of it: held) gives none. @@ -2090,59 +2156,3 @@ func (b *patternBus) putAssigned(vals map[string][]byte, rev uint64) { b.put(buckets.Assignments, k, vals[k], rev) } } - -// Issue 1966 (novox, 2026-11-09): a want the composer made nothing of was said nowhere, and a composition it believed -// handed that the agent never held stayed unhanded. Every pass names each wanted module it composed nothing of, with why, -// in Pass.Unmade and as a journal line; a want not taken is said; and the agent's word that it waits hands the -// composition again. -func TestAWantTheNodeMakesNothingOfSaysWhyAndAWaitingAgentIsHandedItAgain(t *testing.T) { - b := newFakeBus() - fp := meshcatalogclient.SourcePrefix2 + strings.Repeat("a", 64) - gfp := meshcatalogclient.SourcePrefix2 + strings.Repeat("b", 64) - digest := meshcatalogclient.ManifestDigest([]byte(lemurs)) - man, _ := meshcatalogclient.EncodeManifest(meshcatalog.ManifestsByDigestValue{Module: "lemurs", Manifest: lemurs}) - key, _ := meshcatalogclient.ManifestsKey(digest) - b.kv[buckets.Manifests+"/"+key] = man - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - c := New(b.env()) - out, err := c.Watch(ctx, "n1.>") - if err != nil { - t.Fatal(err) - } - place := assignedValues(t, meshmoduleassigner.Assignments{Node: "n1", Decided: meshmoduleassigner.NodeDecided{Hub: true, Address: "10.77.0.1"}, - Modules: []meshmoduleassigner.PlacedModule{{Module: "lemurs"}}}) - have, _ := nodeengineclient.EncodeHaveNode(nodeengine.HaveByNodeNodeValue{Record: &nodeengine.NodeRecord{Node: "n1", TunnelKey: "k", SealingKey: "sk", Capabilities: []string{"systemd"}}}) - mod, _ := meshcatalogclient.EncodeModule(meshcatalog.ModulesByModuleValue{Fp2: fp, Manifest: digest, Repository: "novox/x", Commit: "c"}) - want, _ := bucketstest.Want(meshwants.WantsByWantNodeModuleValue{Fp: fp, Self: true}) - ghost, _ := bucketstest.Want(meshwants.WantsByWantNodeModuleValue{Fp: gfp, Self: true}) - nofp, _ := bucketstest.Want(meshwants.WantsByWantNodeModuleValue{Self: true}) - b.putAssigned(t, place) - b.put(t, buckets.Have, "n1.node", have) - b.put(t, buckets.Modules, "lemurs", mod) - b.put(t, buckets.Wants, "want.n1.lemurs", want) - b.put(t, buckets.Wants, "want.n1.ghost", ghost) - b.put(t, buckets.Wants, "want.n1.nofp", nofp) - for _, bucket := range []string{buckets.Wants, buckets.Assignments, buckets.Have, buckets.Modules, ResolvedBucket, builds.BuilderBuildsBucket} { - b.send(bucket, agent.Update{Caught: true}) - } - for !next(t, out).Caught { - } - why := map[string]string{"ghost": c.WhyUnmade("ghost"), "nofp": c.WhyUnmade("nofp"), "lemurs": c.WhyUnmade("lemurs")} - if !strings.Contains(why["ghost"], "places no ghost") || !strings.Contains(why["nofp"], "names no fingerprint") || why["lemurs"] != "" { - t.Errorf("the pass says it composed nothing of %q", why) - } - b.mu.Lock() - said := strings.Join(b.said, "\n") - b.mu.Unlock() - if !strings.Contains(said, "the node composes nothing of ghost: ") { - t.Errorf("the journal does not say why ghost is not composed: %q", said) - } - // The agent waits for what the node believes it handed: handed again, and a wait for what was never handed does nothing. - c.Waiting("ghost", gfp) - quiet(t, out) - c.Waiting("lemurs", fp) - if u := next(t, out); !strings.Contains(u.Key, "lemurs") { - t.Errorf("handed %s, want lemurs's composition again", u.Key) - } -}