Merge pull request 'A module's directory is never shared code, held or not (hq issue 278)' (#93) from fix/a-module-directory-is-never-shared-code into main

cmd/mesh-controller/order_test.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"encoding/json"
 	"reflect"
 	"strings"
 	"testing"
@@ -243,3 +244,70 @@ func TestAModuleBuiltAtACommitStillFollowsItsBranch(t *testing.T) {
 		t.Error("a module following another branch was matched")
 	}
 }
+
+// novox/hq issue 278: whether a directory is a module is a fact of the repository at the merge commit,
+// and the announcer says it. A merge touching the code of a module the mesh does not hold — the
+// catalogue's reference module, whose manifest it left alone — read as shared and rebuilt every module
+// built from the repository (103 of them on 2026-10-06, 88 byte-identical). Said by the announcer, it
+// rebuilds nothing; a directory holding no manifest is shared as before; and an announcer that does not
+// say keeps the old rule.
+func TestAChangeInsideAModuleIsThatModulesHeldOrNot(t *testing.T) {
+	const repo = "http://forge.internal:20000/novox/mesh-catalog.git"
+	gitea := fromRepo("gitea", repo, "modules/gitea")
+	keycloak := fromRepo("keycloak", repo, "modules/keycloak")
+	known := []inventory.Entry{gitea, keycloak}
+	candidates := []inventory.Entry{gitea, keycloak}
+	merge := func(paths, modules []string, said bool) link.SourceMoved {
+		return link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Paths: paths,
+			ModuleDirs: modules, ModuleDirsSaid: said}
+	}
+	named := func(entries []inventory.Entry) string {
+		var names []string
+		for _, e := range entries {
+			names = append(names, e.Manifest.Module)
+		}
+		return strings.Join(names, ",")
+	}
+	showcase := []string{"modules/showcase/index.ts"}
+	for _, c := range []struct {
+		what string
+		m    link.SourceMoved
+		want string
+	}{
+		{"a module held by none, said", merge(showcase, []string{"modules/showcase"}, true), ""},
+		{"beside a held one's change", merge(append([]string{"modules/gitea/index.ts"}, showcase...),
+			[]string{"modules/gitea", "modules/showcase"}, true), "gitea"},
+		{"deeper inside it", merge([]string{"modules/showcase/daemon/index.ts"}, []string{"modules/showcase"}, true), ""},
+		{"a directory holding no manifest is still shared", merge([]string{"modules/lib/x.go"}, nil, true), "gitea,keycloak"},
+		{"one of two files in no module", merge([]string{"modules/showcase/index.ts", "modules/lib/x.go"},
+			[]string{"modules/showcase"}, true), "gitea,keycloak"},
+		{"the root is never a module directory", merge([]string{"tsconfig.json"}, []string{"", "/", "."}, true), "gitea,keycloak"},
+		{"not said: the old rule", merge(showcase, []string{"modules/showcase"}, false), "gitea,keycloak"},
+		{"an old announcer saying nothing", merge(showcase, nil, false), "gitea,keycloak"},
+		{"a manifest the merge removed, said or not", merge([]string{"modules/gone/module.json", "modules/gone/x.ts"}, nil, true), ""},
+	} {
+		if got := named(whatTheMergeTouched(candidates, known, c.m)); got != c.want {
+			t.Errorf("%s: rebuilt %q, wanted %q", c.what, got, c.want)
+		}
+	}
+}
+
+// What the announcer says reaches the controller as it is sent: the two fields, by their names on the
+// wire, and an older announcement without them reads as not said.
+func TestTheAnnouncerSaysWhichDirectoriesAreModules(t *testing.T) {
+	var m link.SourceMoved
+	if err := json.Unmarshal([]byte(`{"owner":"novox","repo":"mesh-catalog","merge_commit_sha":"abc",`+
+		`"paths":["modules/showcase/index.ts"],"module_dirs":["modules/showcase"],"module_dirs_said":true}`), &m); err != nil {
+		t.Fatal(err)
+	}
+	if !m.ModuleDirsSaid || !reflect.DeepEqual(m.ModuleDirs, []string{"modules/showcase"}) {
+		t.Fatalf("the announcer's word was lost: %+v", m)
+	}
+	var old link.SourceMoved
+	if err := json.Unmarshal([]byte(`{"owner":"novox","repo":"mesh-catalog","merge_commit_sha":"abc","paths":["x"]}`), &old); err != nil {
+		t.Fatal(err)
+	}
+	if old.ModuleDirsSaid || old.ModuleDirs != nil {
+		t.Fatalf("an older announcement says nothing about module directories: %+v", old)
+	}
+}

cmd/mesh-controller/release_plan.go

@@ -1062,6 +1062,8 @@ func plansCommand(ctx context.Context, args []string) error {
 	whatIf := set.String("what-if", "", "owner/repository: the plan a merge there would produce, saving nothing — with --paths or --modules")
 	paths := set.String("paths", "", "the files the merge would change, comma-separated, from the repository's root")
 	modules := set.String("modules", "", "or the modules it would change, comma-separated")
+	moduleDirs := set.String("module-dirs", "", "with --paths: the directories holding a module.json at the commit, "+
+		"comma-separated, as the forge's announcer says them (novox/hq issue 278)")
 	// Ending a plan by hand is a repair, and says why (novox/hq to-be 45 §7).
 	why := addHandActFlags(set)
 	positionals, err := parseAround(set, args)
@@ -1131,7 +1133,7 @@ func plansCommand(ctx context.Context, args []string) error {
 		return nil
 	}
 	if *whatIf != "" {
-		return planWhatIf(ctx, inv, *whatIf, splitList(*paths), splitList(*modules))
+		return planWhatIf(ctx, inv, *whatIf, splitList(*paths), splitList(*modules), moduleDirsOf(*moduleDirs))
 	}
 	// `retry` (novox/hq ADR 0219): a failed plan's failed builds asked again, and the plan goes on.
 	if len(positionals) == 2 && positionals[0] == "retry" {
@@ -1194,12 +1196,15 @@ func plansCommand(ctx context.Context, args []string) error {
 // planWhatIf is the plan a merge would produce, computed the way the merge handler computes one
 // and saved nowhere: the modules the repository's changed files touch (or the modules named), what
 // packages their source, everything reachable from them, in tiers. For reading before merging.
-func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string, paths, modules []string) error {
+func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string, paths, modules []string, moduleDirs *[]string) error {
 	owner, repo, found := strings.Cut(repository, "/")
 	if !found {
 		return fmt.Errorf("--what-if takes owner/repository, not %q", repository)
 	}
 	m := link.SourceMoved{Owner: owner, Repo: repo, Base: "main", Commit: "what-if", Paths: paths}
+	if moduleDirs != nil {
+		m.ModuleDirs, m.ModuleDirsSaid = *moduleDirs, true
+	}
 	entries, err := inv.Catalogued(ctx)
 	if err != nil {
 		return err
@@ -1275,6 +1280,16 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string
 	return nil
 }
 
+// moduleDirsOf is --module-dirs as an announcer would say it: nothing when not given, so the what-if
+// reads as an announcement from an announcer that does not say.
+func moduleDirsOf(s string) *[]string {
+	if strings.TrimSpace(s) == "" {
+		return nil
+	}
+	dirs := splitList(s)
+	return &dirs
+}
+
 func splitList(s string) []string {
 	var out []string
 	for _, part := range strings.Split(s, ",") {

cmd/mesh-controller/release_plan_test.go

@@ -212,3 +212,45 @@ func TestTheGateJudgesEachMachineFromItsOwnSend(t *testing.T) {
 		t.Fatal("a report from before the machine was sent opened the gate")
 	}
 }
+
+// novox/hq issue 278 and ADR 0236's open question: a change to the build agent — its manifest alone, as
+// the catalogue's data sections were, or its program — rebuilds the build agent and nothing it builds.
+// What it builds is ordered after it in a plan that holds both, never added to one for its sake. The
+// merge that rebuilt 103 modules with the agent in tier 0 rebuilt them for a file read as shared code;
+// the agent stood first only because everything else is built by it.
+func TestAChangeToTheBuildAgentRebuildsTheBuildAgentAlone(t *testing.T) {
+	const repo = "http://forge.internal:20000/novox/mesh-catalog.git"
+	agent := fromRepo("build-agent", repo, "modules/build-agent")
+	redis := fromRepo("redis", repo, "modules/redis")
+	postgres := fromRepo("postgres", repo, "modules/postgres")
+	entries := []inventory.Entry{agent, redis, postgres}
+	edges := []inventory.Edge{
+		{From: "redis", To: "mesh-tools", Kind: inventory.EdgeStandsOn},
+		{From: "redis", To: "build-agent", Kind: inventory.EdgeBuiltBy},
+		{From: "postgres", To: "build-agent", Kind: inventory.EdgeBuiltBy},
+		{From: "mesh-tools", To: "build-agent", Kind: inventory.EdgeBuiltBy},
+		{From: "mesh-controller", To: "build-agent", Kind: inventory.EdgeBuiltBy},
+		{From: "build-agent", To: "mesh-controller", Kind: inventory.EdgeWorkerOf},
+	}
+	for _, paths := range [][]string{
+		{"modules/build-agent/module.json"},                                          // its manifest alone
+		{"modules/build-agent/cmd/agent/main.go", "modules/build-agent/module.json"}, // its program too
+	} {
+		m := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "abc", Paths: paths,
+			ModuleDirs: []string{"modules/build-agent"}, ModuleDirsSaid: true}
+		touched := whatTheMergeTouched(entries, entries, m)
+		if len(touched) != 1 || touched[0].Manifest.Module != "build-agent" {
+			t.Fatalf("%v touched %v", paths, touched)
+		}
+		p := planOfMerge(m, []string{"build-agent"}, edges)
+		if len(p.Tiers) != 1 || len(p.Tiers[0]) != 1 || p.Tiers[0][0] != "build-agent" || len(p.Modules) != 1 {
+			t.Fatalf("%v planned %v: the build agent alone", paths, p.Tiers)
+		}
+	}
+	// With what it builds moved beside it, the agent comes first and they follow: an order, not a widening.
+	p := planOfMerge(link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Commit: "abc"},
+		[]string{"build-agent", "redis"}, edges)
+	if len(p.Tiers) != 2 || p.Tiers[0][0] != "build-agent" || p.Tiers[1][0] != "redis" || len(p.Modules) != 2 {
+		t.Fatalf("the agent, then what moved beside it: %v", p.Tiers)
+	}
+}

cmd/mesh-controller/seatverbs.go

@@ -284,6 +284,9 @@ func (a *verbArguments) commandLine() ([]string, error) {
 			if m := str("modules"); m != "" {
 				argv = append(argv, "--modules", m)
 			}
+			if d := str("module-dirs"); d != "" {
+				argv = append(argv, "--module-dirs", d)
+			}
 			return argv, nil
 		}
 		for _, act := range []string{"stop", "close", "retry"} {

cmd/mesh-controller/upgrades.go

@@ -640,6 +640,11 @@ func lastLookAt(entries []inventory.Entry, m link.SourceMoved) time.Time {
 // library (`modules/lib`), and that is still read as shared. Rebuilding too much remains the safe
 // direction: the fault this whole path exists for is a mesh that believes it is current and is not
 // (novox/hq 04-ISSUES/131). A file at the root, or directly in a parent, is shared as it always was.
+//
+// **Whether a directory is a module is a fact of the repository at the merge commit** (novox/hq issue
+// 278), and the announcer now says it: a changed file inside a directory holding a `module.json` there
+// is that module's, whatever the merge did to the manifest; only a file in no such directory is shared.
+// From an announcer that does not say, the rule above stands.
 func whatTheMergeTouched(candidates, known []inventory.Entry, m link.SourceMoved) []inventory.Entry {
 	// Nothing said about the files, or not all of them said: everything built from it is affected.
 	if len(m.Paths) == 0 || m.PathsTruncated {
@@ -660,8 +665,9 @@ func whatTheMergeTouched(candidates, known []inventory.Entry, m link.SourceMoved
 	for _, p := range m.Paths {
 		changed[strings.TrimPrefix(p, "/")] = true
 	}
+	modules := saidModuleDirs(m)
 	for _, p := range m.Paths {
-		if insideAny(p, dirs) || inAModuleOfItsOwn(p, parents, changed) {
+		if insideAny(p, dirs) || inAModuleOfItsOwn(p, parents, changed) || insideAny(p, modules) {
 			continue
 		}
 		return candidates
@@ -675,6 +681,25 @@ func whatTheMergeTouched(candidates, known []inventory.Entry, m link.SourceMoved
 	return out
 }
 
+// saidModuleDirs is the directories the announcer says hold a module at the merge commit (novox/hq
+// issue 278): a changed file in one of them is that module's business and nobody else's — the
+// catalogue's reference module, held by no machine, whose code a merge touched beside its manifest,
+// read as shared and rebuilt 103 modules. Nothing when the announcer did not look, and never the
+// repository's root: a module built from the root is handled as one built from it, and a root that
+// counted would make every file a module's.
+func saidModuleDirs(m link.SourceMoved) []string {
+	if !m.ModuleDirsSaid {
+		return nil
+	}
+	var out []string
+	for _, d := range m.ModuleDirs {
+		if d = strings.Trim(strings.TrimSpace(d), "/"); d != "" && d != "." {
+			out = append(out, d)
+		}
+	}
+	return out
+}
+
 // inAModuleOfItsOwn is whether a changed file is inside a module directory the mesh does not know —
 // `<parent>/<name>/…` under a directory known to hold modules, whose `module.json` the same merge
 // changed (novox/hq issue 252). Such a file is that module's business and nobody else's.