A module's directory is never shared code, held or not (hq issue 278)

The merge of mesh-catalog 7f99fb4a rebuilt 103 modules with the build agent in tier 0, and ADR
0236 recorded it as "a change to the build agent rebuilds most of the catalogue". The agent had
not changed: modules/showcase/index.ts had. showcase is the catalogue's reference module, held
by no machine, and its manifest was not in the merge, so whatTheMergeTouched read the file as
shared code and rebuilt everything built from the repository (88 came out byte-identical). The
agent stood first only because everything is built by it.

Whether a directory is a module is a fact of the repository at the merge commit, so the forge's
announcer now says it: module_dirs, the changed files' directories holding a module.json there,
with module_dirs_said. A changed file inside one is that module's business; only a file in no
such directory is shared. An announcer that does not say keeps the old rule. `plans` what-if
takes the same list as module-dirs.

And a regression for the open question: nothing depends on the build agent except by being
built by it, and built-by never widens a plan, so a change to the agent - manifest or program -
rebuilds the agent alone; what moved beside it is ordered after it.
This commit is contained in:
jochen
2026-10-06 19:55:25 +02:00
parent 4635341a9d
commit b1e02aca1b
8 changed files with 208 additions and 6 deletions
+68
View File
@@ -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)
}
}
+17 -2
View File
@@ -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, ",") {
+42
View File
@@ -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)
}
}
+3
View File
@@ -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"} {
+26 -1
View File
@@ -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.