diff --git a/cmd/mesh-controller/order_test.go b/cmd/mesh-controller/order_test.go index f4a2c62..5fe7b12 100644 --- a/cmd/mesh-controller/order_test.go +++ b/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) + } +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 1adb2fa..dd9250e 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/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, ",") { diff --git a/cmd/mesh-controller/release_plan_test.go b/cmd/mesh-controller/release_plan_test.go index e42c214..066c602 100644 --- a/cmd/mesh-controller/release_plan_test.go +++ b/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) + } +} diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 3517e34..38ed024 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/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"} { diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index 515d311..b95a59e 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/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 — // `//…` 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. diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index d6020d2..727c4e5 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -109,9 +109,11 @@ var ControllerVerbs = []Verb{ "repository": "owner/repository: the plan a merge there would produce, saving nothing (what-if); with paths or modules", "paths": "with repository: the files the merge would change, comma-separated, from the repository's root", "modules": "with repository: or the modules it would change, comma-separated", - "limit": "how many plans to list (default 10); only when listing", - "why": "with stop or close: why it is ended by hand — required, and recorded in the hand-act log (novox/hq to-be 45 §7)", - "cause": "with stop or close: the cause in a word, or a condition's kind (optional)", + "module-dirs": "with repository and paths: the directories holding a module.json at the commit, comma-separated, " + + "as the forge's announcer says them (novox/hq issue 278); without it, a directory is a module only when the paths hold its manifest", + "limit": "how many plans to list (default 10); only when listing", + "why": "with stop or close: why it is ended by hand — required, and recorded in the hand-act log (novox/hq to-be 45 §7)", + "cause": "with stop or close: the cause in a word, or a condition's kind (optional)", }, nil)}, {Name: "plan", Description: "What one machine would run, and why: the declaration the mesh would send it — " + "or, with files, the files it would be given.", diff --git a/internal/inventory/dependencies_test.go b/internal/inventory/dependencies_test.go index 32c3615..bbf463e 100644 --- a/internal/inventory/dependencies_test.go +++ b/internal/inventory/dependencies_test.go @@ -120,3 +120,37 @@ func TestAToolchainStandingOnTheSDKFollowsIt(t *testing.T) { t.Errorf("no edge from the toolchain to the SDK: %v", edges) } } + +// novox/hq issue 278: nothing depends on the build agent but by being built by it. A plan is widened +// only along the other kinds (reachableFrom), so this is what keeps a change to the build agent — its +// manifest or its program — from rebuilding what it builds: the relation holds no edge to it that could. +func TestNothingStandsOnTheBuildAgentOnlyIsBuiltByIt(t *testing.T) { + const catalogueRepo = "http://forge/novox/mesh-catalog.git" + agent := Entry{Manifest: catalogue.Manifest{Module: "build-agent", + Claims: []catalogue.Claim{{Name: "node-build-agent", Scope: catalogue.ScopeNode}}}, + Source: Source{Repository: catalogueRepo, Path: "modules/build-agent"}} + bundle := Entry{Manifest: catalogue.Manifest{Module: "redis", Build: &catalogue.Build{ + Artifacts: []catalogue.Artifact{{Name: "code", Kind: catalogue.ArtifactBundle, Language: "typescript"}}}}, + Source: Source{Repository: catalogueRepo, Path: "modules/redis"}} + entries := []Entry{ + agent, bundle, + {Manifest: catalogue.Manifest{Module: "postgres"}, Source: Source{Repository: catalogueRepo, Path: "modules/postgres"}}, + {Manifest: catalogue.Manifest{Module: "mesh-tools"}, Source: Source{Repository: "http://forge/novox/mesh-tools.git"}}, + {Manifest: catalogue.Manifest{Module: TheControlPlane}, Source: Source{Repository: "http://forge/novox/mesh-controller.git"}}, + } + // A module packaging another repository depends on what is built from it, never on the agent. + read := map[string][]ReadRepository{"postgres": {{Repository: "http://forge/novox/mesh-tools.git"}}} + builtBy := 0 + for _, e := range dependenciesOf(entries, nil, read) { + if e.To != "build-agent" { + continue + } + if e.Kind != EdgeBuiltBy { + t.Errorf("%s depends on the build agent as %s: a change to the agent would rebuild it", e.From, e.Kind) + } + builtBy++ + } + if builtBy != 4 { + t.Errorf("everything source-built is built by the agent: %d edges", builtBy) + } +} diff --git a/internal/link/events.go b/internal/link/events.go index 84bb5b5..397503f 100644 --- a/internal/link/events.go +++ b/internal/link/events.go @@ -188,6 +188,19 @@ type SourceMoved struct { // deleted at its source: it is forgotten, or said, and never built (novox/hq ADR 0236). Empty from an // announcer that does not say which files went, and then a build that finds no manifest says it. Removed []string `json:"removed,omitempty"` + + // ModuleDirs are the directories holding the files the merge changed that hold a `module.json` at + // the merge commit, from the repository's root (novox/hq issue 278). A changed file inside one of + // them is that module's business, whether or not the mesh holds the module and whether or not the + // merge touched its manifest; only a file in no such directory is shared code. Without it, a + // change to a module the mesh does not hold — whose manifest the merge left alone — read as shared + // and rebuilt every module built from the repository. + ModuleDirs []string `json:"module_dirs,omitempty"` + + // ModuleDirsSaid says the announcer looked, so an empty ModuleDirs means "none of them is a + // module" rather than "not said". An announcer from before this says nothing, and the old rule + // stands: a directory is a module only when the merge changed its manifest. + ModuleDirsSaid bool `json:"module_dirs_said,omitempty"` } type Upgraded struct {