Merge pull request 'Drop the node's own copy of a resource a module composition declares, and give the kept declaration's filled paths to their modules (issue 1945)' (#140) from fix/r6-own-composition-dupes into main

2026-10-25 13:51:00 +00:00
parent 57b553ac5a 1bdbf47ffa
commit 0dfbe1a179
4 changed files with 239 additions and 25 deletions
@@ -158,8 +158,31 @@ func awaitBackup(ctx context.Context, events <-chan []byte, node, module string,
 // declarationOf is the declaration the compositions make together: their resources, and the node's own parts when the
 // node's composition is among them. It is what the node judges and says its health by, and what one module's apply is
 // made of (with a single composition).
+//
+// **A resource the node's own composition shares with a module's is the module's** (issue 1945): from 2026-10-25 12:43
+// the controller put modules it knew no fingerprint of the new form for into the node's own composition while their
+// wants named their own, and the declaration joined of both declared each such file twice, which the filling refuses
+// ("2 resources declare X") on every node. The node's own copy is left out, and which were left out is said once.
 func declarationOf(cs ...delivery.Composed) (*declaration.Declaration, error) {
 	d := &declaration.Declaration{Version: declaration.Version}
+	var modules []delivery.Composed
+	for _, c := range cs {
+		if c.Module != "" {
+			modules = append(modules, c)
+		}
+	}
+	if len(modules) > 0 {
+		joined := make([]delivery.Composed, 0, len(cs))
+		for _, c := range cs {
+			if c.Module == "" {
+				var dropped []string
+				c, dropped = ownWithout(c, modules)
+				sayDroppedOnce(dropped)
+			}
+			joined = append(joined, c)
+		}
+		cs = joined
+	}
 	for _, c := range cs {
 		for _, raw := range c.Resources {
 			r, err := declaration.ParseResource(raw)
@@ -180,6 +203,66 @@ func declarationOf(cs ...delivery.Composed) (*declaration.Declaration, error) {
 	return d, nil
 }

+// ownWithout is the node's own composition without the resources a module's composition among modules also declares:
+// the same kind and id. A resource that shares only its target (a unit or container of the same name, another id) is
+// not the module's, and stays: dropped, its record would be an orphan the apply removes, stopping the module's unit. It
+// answers what it left out, as "<id> (<module>)".
+func ownWithout(own delivery.Composed, modules []delivery.Composed) (delivery.Composed, []string) {
+	byID := map[string]string{}
+	for _, c := range modules {
+		if c.Module == "" {
+			continue
+		}
+		for _, raw := range c.Resources {
+			r, err := declaration.ParseResource(raw)
+			if err != nil {
+				continue
+			}
+			byID[string(r.Kind())+"\x00"+r.Identity()] = c.Module
+		}
+	}
+	if len(byID) == 0 {
+		return own, nil
+	}
+	var dropped []string
+	kept := make([]json.RawMessage, 0, len(own.Resources))
+	for _, raw := range own.Resources {
+		r, err := declaration.ParseResource(raw)
+		if err != nil {
+			kept = append(kept, raw) // refused where it is parsed, with its words
+			continue
+		}
+		module, dup := byID[string(r.Kind())+"\x00"+r.Identity()]
+		if dup {
+			dropped = append(dropped, fmt.Sprintf("%s (%s)", r.Identity(), module))
+			continue
+		}
+		kept = append(kept, raw)
+	}
+	if len(dropped) == 0 {
+		return own, nil
+	}
+	own.Resources = kept
+	return own, dropped
+}
+
+// droppedSaid is every set of the node's own resources left out that was said, so each is said once.
+var droppedSaid sync.Map
+
+// sayDroppedOnce says once which of the node's own resources were left out because a module's composition declares them.
+func sayDroppedOnce(dropped []string) {
+	if len(dropped) == 0 {
+		return
+	}
+	sort.Strings(dropped)
+	key := strings.Join(dropped, ", ")
+	if _, said := droppedSaid.LoadOrStore(key, true); said {
+		return
+	}
+	fmt.Fprintf(os.Stderr, "mesh-host: the node's own composition also declares %d resource(s) a module's composition "+
+		"declares, and they are left out of it: %s (issue 1945)\n", len(dropped), key)
+}
+
 func scopeName(c delivery.Composed) string {
 	if c.Module == "" {
 		return agent.NodeModule
@@ -233,40 +316,60 @@ func trustedKeys(opts options, controller ed25519.PublicKey) func() []ed25519.Pu
 	}
 }

-// keptByModule is the resources of the declaration the node kept before the switch, by the module they say; the node's
-// own are those that say none (ADR 0539 §9).
-func keptByModule(opts options, signer ed25519.PublicKey) func() map[string][]json.RawMessage {
+// keptOnce reads the kept declaration once and splits it by module: its resources (the node's own are those that say
+// none, ADR 0539 §9) and its filled paths.
+func keptOnce(opts options, signer ed25519.PublicKey) func() (map[string][]json.RawMessage, map[string][]string) {
 	var once sync.Once
 	var by map[string][]json.RawMessage
-	return func() map[string][]json.RawMessage {
+	var filled map[string][]string
+	return func() (map[string][]json.RawMessage, map[string][]string) {
 		once.Do(func() {
 			raw, err := store.LoadDeclared(store.DeclaredPath(opts.state), signer)
 			if err != nil {
 				return
 			}
-			var kept struct {
-				Resources []json.RawMessage `json:"resources"`
-			}
-			if json.Unmarshal(raw, &kept) != nil {
-				return
-			}
-			by = map[string][]json.RawMessage{}
-			for _, r := range kept.Resources {
-				var head struct {
-					Module string `json:"module"`
-				}
-				_ = json.Unmarshal(r, &head)
-				name := head.Module
-				if name == "" {
-					name = agent.NodeModule
-				}
-				by[name] = append(by[name], r)
-			}
+			by, filled = splitKept(raw)
 		})
-		return by
+		return by, filled
 	}
 }

+// splitKept is a kept declaration's resources by the module they say (the node's own under agent.NodeModule) and its
+// filled paths by the module whose file is at each path; nil for a declaration that does not read.
+func splitKept(raw []byte) (map[string][]json.RawMessage, map[string][]string) {
+	var kept struct {
+		Resources []json.RawMessage `json:"resources"`
+		Filled    []string          `json:"filled_files"`
+	}
+	if json.Unmarshal(raw, &kept) != nil {
+		return nil, nil
+	}
+	isFilled := make(map[string]bool, len(kept.Filled))
+	for _, p := range kept.Filled {
+		isFilled[p] = true
+	}
+	by, filled := map[string][]json.RawMessage{}, map[string][]string{}
+	named := map[string]bool{}
+	for _, r := range kept.Resources {
+		var head struct {
+			Module string `json:"module"`
+			Type   string `json:"type"`
+			Path   string `json:"path"`
+		}
+		_ = json.Unmarshal(r, &head)
+		name := head.Module
+		if name == "" {
+			name = agent.NodeModule
+		}
+		by[name] = append(by[name], r)
+		if head.Type == string(declaration.TypeFile) && isFilled[head.Path] && !named[name+"\x00"+head.Path] {
+			named[name+"\x00"+head.Path] = true
+			filled[name] = append(filled[name], head.Path)
+		}
+	}
+	return by, filled
+}
+
 // nodeAgent runs the node agent under the session's links for as long as ctx lasts: it waits for a link, follows the
 // node's wants and compositions on it, and begins again with the next link. A watch it cannot make (a bucket this node's
 // account may not read) is said, and tried again with a growing wait, as the link is.
@@ -287,13 +390,16 @@ func nodeAgent(ctx context.Context, opts options, signer ed25519.PublicKey, sess
 			return
 		}
 		var ag *agent.Agent
+		kept := keptOnce(opts, signer)
 		env := agent.Env{
 			Node:     node,
 			Wants:    bucketWatch{session, delivery.AssignerWantsBucket},
 			Composed: bucketWatch{session, composedBucket},
 			Haves:    sessionHaves{session},
-			Kept:     keptByModule(opts, signer),
-			Recorded: recordedIn(opts.state, say),
+			Kept:     func() map[string][]json.RawMessage { by, _ := kept(); return by },
+			// The same kept declaration's filled paths, so a module still running from it keeps its files filled.
+			KeptFilled: func() map[string][]string { _, filled := kept(); return filled },
+			Recorded:   recordedIn(opts.state, say),
 			Revisions: func(ctx context.Context, key string) ([]agent.Update, error) {
 				got, err := session.KVHistory(ctx, composedBucket, key)
 				if err != nil {
@@ -372,6 +478,12 @@ func (c *agentNow) get() *agent.Agent {
 func applyComposition(ctx context.Context, opts options, c delivery.Composed, others []delivery.Composed,
 	sched *apply.Scheduler, say link.Announce) agent.Result {
 	name := scopeName(c)
+	if c.Module == "" {
+		// What another module's composition declares is that module's, never the node's own apply's (issue 1945).
+		var dropped []string
+		c, dropped = ownWithout(c, others)
+		sayDroppedOnce(dropped)
+	}
 	d, err := declarationOf(c)
 	if err != nil {
 		return agent.Result{Failed: true, Detail: shorten(err.Error())}
@@ -4,10 +4,13 @@ import (
 	"crypto/ed25519"
 	"encoding/json"
 	"errors"
+	"strings"
 	"testing"

 	"git.novox.be/novox/mesh-sdk/go/declaration"
 	"git.novox.be/novox/mesh-sdk/go/delivery"
+
+	"github.com/novox/mesh-host/internal/agent"
 )

 func aFileOf(module, id string) json.RawMessage {
@@ -57,3 +60,80 @@ func TestDeclaredNowReadsWhatRunsWhenTheAgentIsRunning(t *testing.T) {
 		t.Fatalf("declaredNow read %+v (%v), not what runs", d, err)
 	}
 }
+
+// 1945/AC17 (unit): a resource the node's own composition shares with a running module's composition is declared once,
+// the module's: the declaration joined of both names each file once, so the filling writes it; the node's own resources
+// no module declares stay. (Live from 2026-10-25 12:43: "2 resources declare X" on every node.)
+func TestTheNodesOwnCopyOfAModulesResourceIsLeftOut(t *testing.T) {
+	own := delivery.Composed{Node: "n1", Resources: []json.RawMessage{aFileOf("zsh", "rc"), aFileOf("mesh-wireguard", "conf")},
+		Filled: []string{"/tmp/r6/zsh/rc"}}
+	zsh := delivery.Composed{Node: "n1", Module: "zsh", FP: "src2:aa", Resources: []json.RawMessage{aFileOf("zsh", "rc")},
+		Filled: []string{"/tmp/r6/zsh/rc"}}
+	d, err := declarationOf(own, zsh)
+	if err != nil {
+		t.Fatal(err)
+	}
+	at := map[string]int{}
+	for _, r := range d.Resources {
+		at[r.Target()]++
+	}
+	if at["/tmp/r6/zsh/rc"] != 1 || at["/tmp/r6/mesh-wireguard/conf"] != 1 || len(d.Resources) != 2 {
+		t.Errorf("the joined declaration declares %v, want each path once", at)
+	}
+	if !d.Fills("/tmp/r6/zsh/rc") {
+		t.Errorf("the filled path is no longer named")
+	}
+
+	// The same at the node's own apply: what the others declare is theirs.
+	trimmed, dropped := ownWithout(own, []delivery.Composed{zsh})
+	if len(trimmed.Resources) != 1 || len(dropped) != 1 || dropped[0] != "zsh.rc (zsh)" {
+		t.Errorf("the node's own without zsh's: %d resources, dropped %v", len(trimmed.Resources), dropped)
+	}
+	if len(own.Resources) != 2 {
+		t.Errorf("ownWithout changed its argument")
+	}
+	// A node's own composition alone is all of it.
+	if d, err := declarationOf(own); err != nil || len(d.Resources) != 2 {
+		t.Errorf("the node's own alone: %v (%v)", d, err)
+	}
+}
+
+// 1945/AC18 (unit): the kept declaration's filled paths go to the module whose file is at each path, the node's own to
+// the node's; and what runs from it, joined with the node's composition, names them filled.
+func TestTheKeptDeclarationsFilledPathsGoToTheirModules(t *testing.T) {
+	raw := []byte(`{"resources":[
+		{"id":"claude-code.settings","type":"file","module":"claude-code","path":"/etc/cc.json"},
+		{"id":"screen-lock.conf","type":"file","module":"screen-lock","path":"/etc/lock.conf"},
+		{"id":"host.motd","type":"file","path":"/etc/motd"},
+		{"id":"zsh.rc","type":"file","module":"zsh","path":"/etc/zshrc"}],
+		"filled_files":["/etc/cc.json","/etc/lock.conf","/etc/motd"]}`)
+	by, filled := splitKept(raw)
+	if len(by["claude-code"]) != 1 || len(by[agent.NodeModule]) != 1 {
+		t.Fatalf("resources by module: %v", by)
+	}
+	if strings.Join(filled["claude-code"], ",") != "/etc/cc.json" || strings.Join(filled["screen-lock"], ",") != "/etc/lock.conf" ||
+		strings.Join(filled[agent.NodeModule], ",") != "/etc/motd" || len(filled["zsh"]) != 0 {
+		t.Fatalf("filled by module: %v", filled)
+	}
+	cc := delivery.Composed{Node: "n1", Module: "claude-code", Resources: by["claude-code"], Filled: filled["claude-code"]}
+	node := delivery.Composed{Node: "n1"}
+	d, err := whatRuns([]delivery.Composed{cc, node}, true, func() (*declaration.Declaration, error) { return nil, errors.New("not read") })
+	if err != nil || !d.Fills("/etc/cc.json") {
+		t.Fatalf("what runs does not name claude-code's file filled: %v (%v)", d, err)
+	}
+	if splitBy, _ := splitKept([]byte("not json")); splitBy != nil {
+		t.Errorf("a kept declaration that does not read split into %v", splitBy)
+	}
+}
+
+// 1945/AC17 (unit, review): a resource of the node's own that shares only its target with a module's (another id: a
+// unit or a container of the same name) is not the module's, and is neither dropped nor left to be removed.
+func TestASharedTargetWithAnotherIDIsNotDropped(t *testing.T) {
+	ownFile, _ := json.Marshal(map[string]any{"id": "host.zshrc", "type": "file", "path": "/tmp/r6/zsh/rc", "content": "x"})
+	own := delivery.Composed{Node: "n1", Resources: []json.RawMessage{ownFile}}
+	zsh := delivery.Composed{Node: "n1", Module: "zsh", FP: "src2:aa", Resources: []json.RawMessage{aFileOf("zsh", "rc")}}
+	trimmed, dropped := ownWithout(own, []delivery.Composed{zsh})
+	if len(dropped) != 0 || len(trimmed.Resources) != 1 {
+		t.Errorf("a resource sharing only its target was dropped: %v", dropped)
+	}
+}
@@ -90,6 +90,10 @@ type Env struct {
 	// §9), as JSON; the node's own are under NodeModule. Nil when the node kept none. A module the agent does not hold yet
 	// still runs from it.
 	Kept func() map[string][]json.RawMessage
+	// KeptFilled is the paths the kept declaration names as filled by the provisioner, by the module whose file is at
+	// each (the node's own under NodeModule). A module that still runs from the kept declaration keeps its filled files
+	// filled: without them the filling refused each one as "not named as filled" (issue 1945). Nil names none.
+	KeptFilled func() map[string][]string
 	// Trusted is the public keys a composition may be signed by: the controller's and the operator's (ADR 0539 §3). A
 	// composition unsigned, signed by another key, or changed after it was signed is refused and nothing of it is
 	// applied. Nil trusts no key, so nothing is applied.
@@ -441,6 +445,9 @@ func (a *Agent) others(except string) []delivery.Composed {
 		sort.Strings(names)
 		for _, m := range names {
 			c := delivery.Composed{Node: a.env.Node, Module: m, Resources: kept[m]}
+			if a.env.KeptFilled != nil {
+				c.Filled = a.env.KeptFilled()[m]
+			}
 			if m == NodeModule {
 				c.Module = ""
 			}
@@ -835,3 +835,18 @@ func TestTheRevisionAWantNamesIsAppliedFromHistory(t *testing.T) {
 		t.Fatalf("the have names %s, want %s", h.Composed, oldDig)
 	}
 }
+
+// 1945/AC18 (unit): a module that still runs from the kept declaration carries the kept declaration's filled paths of
+// its files, so the filling writes them ("not named as filled" for claude-code and screen-lock, 2026-10-25).
+func TestAModuleRunningFromTheKeptDeclarationKeepsItsFilledPaths(t *testing.T) {
+	b := newBed(t)
+	b.kept = map[string][]json.RawMessage{"claude-code": {file("claude-code", "settings", "")}}
+	b.env.KeptFilled = func() map[string][]string {
+		return map[string][]string{"claude-code": {"/tmp/r6/claude-code/settings"}}
+	}
+	a := New(b.env)
+	got := a.Everything()
+	if len(got) != 1 || got[0].Module != "claude-code" || strings.Join(got[0].Filled, ",") != "/tmp/r6/claude-code/settings" {
+		t.Fatalf("what runs from the kept declaration: %+v", got)
+	}
+}