diff --git a/04-ISSUES/338-a-module-built-from-a-shared-repository-moves-on-every-merge-to-it/00-report.md b/04-ISSUES/338-a-module-built-from-a-shared-repository-moves-on-every-merge-to-it/00-report.md new file mode 100644 index 00000000..92ecced7 --- /dev/null +++ b/04-ISSUES/338-a-module-built-from-a-shared-repository-moves-on-every-merge-to-it/00-report.md @@ -0,0 +1,63 @@ +--- +status: located +opened: 2026-10-08 +located-in: [mesh-controller cmd/mesh-controller (upgrades.go, mergeCandidates and readsFrom), mesh-controller internal/inventory (dependencies.go, the packages edge), mesh-controller internal/builder (builder.go and source.go, a context's fingerprint), mesh-catalog modules/route-proxy and modules/build-agent (their recipes and build contexts)] +fixed-by: +amended-design: +--- + +# 338. A module built from a shared repository moves on every merge to it + +## What was observed + +On 2026-10-08 the controller's repository took eight merges. Every one of them produced the same +delivery plan: build `mesh-controller`, then `build-agent`, then `route-proxy`, in three tiers. Each +tier waited for its own first-node gate, about three and a half minutes each, so every merge of the +controller took three gates where one would have done. + +`build-agent` (the build seat's holder) and `route-proxy` (the public route proxy on the home server) +are packaged in the catalogue, but their recipes compile code from the controller's repository. Their +manifests say so with a build context: `build.artifacts[].context` names the git seat, the repository +`novox/mesh-controller` and the ref `main`. + +The plan rebuilt and moved them whatever the merge changed. Two examples: + +- Mesh-controller PR #159, merged as `ca09a07f`, changed files under `cmd/mesh-controller`, + `internal/broker`, `internal/link`, `internal/catalogue/verbs.go` and the root `module.json`. Nothing + under `examples/route-proxy` changed. Its walk said, on the home server, "recreates 1 of route-proxy's + 3 container(s) (with a new image: server)" and "recreates 1 of build-agent's 1 container(s)". +- Mesh-controller PR #163, merged as `9b028b42`, did the same. The route proxy's program at that commit + is byte for byte the program at the commit before it (measured in the diagnosis), and it was still + sent a new image and restarted. + +The controller's `plans` what-if shows that the files do not matter. A merge that changes only the +controller's `README.md` plans the same three modules in three tiers: + +> a merge of novox/mesh-controller would build 3 module(s) in 3 tier(s) … build-agent, route-proxy +> package source from novox/mesh-controller, so they are rebuilt without their own source moving + +## What the operator expects + +The operator stated the rule the delivery plan should follow: + +1. Changed files map to the module or modules whose declared build source contains them. One commit can + touch several modules. +2. The module graph's build dependencies add the modules that are built from a changed module. +3. Nothing else counts: not sharing a repository, and not a whole-repository build context. + +## Why it is an issue + +- **Deliveries are slow.** A controller merge walked for about fifteen minutes instead of about five, + and the walks of one repository must not overlap (issue 335), so every merge after it waits too. The + operator considers this a major bug and a main reason deliveries are slow. +- **A public service restarts for nothing.** The route proxy on the home server was recreated at + merges that did not change its program. +- **The planner says it is path-precise and is not.** ADR 0238 says a changed file touches exactly the + modules whose build reads it. That holds for a module's own directory and fails for a build context, + which is read whole. + +## How it is checked + +Not checked today. The diagnosis proposes the checks that would hold the fix: a planner test in which a +change to a file outside a module's declared source paths moves nothing, and the `plans` what-if above +answering one module for a `README.md`-only change. diff --git a/04-ISSUES/338-a-module-built-from-a-shared-repository-moves-on-every-merge-to-it/01-diagnosis.md b/04-ISSUES/338-a-module-built-from-a-shared-repository-moves-on-every-merge-to-it/01-diagnosis.md new file mode 100644 index 00000000..a92b0366 --- /dev/null +++ b/04-ISSUES/338-a-module-built-from-a-shared-repository-moves-on-every-merge-to-it/01-diagnosis.md @@ -0,0 +1,195 @@ +# 338. Diagnosis + +## 2026-10-08 — the record first + +`records_search` for "route-proxy rebuilt" found nothing. "build context" found +[ADR 0155](../../02-DECISIONS/0155-a-definition-names-no-installation-and-how-that-is-checked.md) (a build +context may live on the git seat) and issue 060's note that the route proxy's build context is the +controller's repository, not its own module directory. "fingerprint" found issue 280 and +[ADR 0238](../../02-DECISIONS/0238-a-commit-is-the-build-at-hand-one-commit-one-change-plan-checked-off-the-trunk-and-published-only-on-it.md). +Issue 280 fixed the wide rebuild for a module's own directory and left one sentence that matters here: +the source fingerprint hashes "the git tree of each other repository an artifact's context is cloned +from", which is the whole repository. + +## How a merge decides what it moves + +Read in mesh-controller at `8170fc58`. + +**Facts, from the code.** + +1. `mergeCandidates` (cmd/mesh-controller/upgrades.go) sorts every catalogued module into two kinds. + A module whose own source is the merged repository and branch is a candidate *built from* it. A + module whose last build read the merged repository as a context is a *packaging* module + (`readsFrom`). The test for the second kind is `sameRepository` and the ref, and nothing else. +2. `touchedBy`, the one function ADR 0238 names for mapping changed files onto modules, is applied + only to the first kind. A candidate is touched when a changed file lies inside its module directory, + or when it is built from the repository's root. The packaging modules never pass through it. +3. `reachOfMerge` and the merge handler `SourceMoved` both add every packaging module to what moves: + `moved := touched + packaging`. So for a packaging module the rule is **per repository**, not per + path. The changed paths and `module_dirs_said` of PR #159's delivery were read, and they decided + nothing for `route-proxy` and `build-agent`. +4. `mesh-controller` itself is built from the repository's root (its manifest's bundle compiles + `cmd/mesh-controller`; its source path is empty), so every changed file in the repository touches + it as well. +5. The tiers come from `inventory.Dependencies`. Two of its edge kinds make the walk serial here: + - a `packages` edge, from a module whose build read a repository to the module whose source that + repository is. So `route-proxy` and `build-agent` both depend on `mesh-controller`, because they + share its repository, not because either is built from the controller's artifacts; + - a `built-by` edge, from every source-built module to the build seat's holder. So `route-proxy` + waits for `build-agent`. + + That gives three tiers, each with its own first-node gate. +6. The manifests (mesh-catalog `modules/route-proxy/module.json` and `modules/build-agent/module.json`) + declare the context as the whole repository: `{seat: git, repository: novox/mesh-controller, ref: + main}`. Nothing in a manifest can say which paths of it a build reads. Their recipes copy the whole + context (`COPY . .`) and compile one package from it: `./examples/route-proxy` and + `./cmd/mesh-builder`. +7. These are the only two manifests in the catalogue with a build context. Both name the controller's + repository. That repository is also the source of `mesh-controller`, so three modules share it. + +**The operator's rule, against the code.** Rule 1 (a changed file maps to the modules whose declared +build source contains it) holds for a module's own directory in `touchedBy`, and is broken by fact 3: +a packaging module moves on any file. Rule 2 (the graph's build dependencies add what is built from a +changed module) is broken by fact 5: the `packages` edge treats sharing a repository as a build +dependency. Rule 3 (nothing else counts) is broken by both. A manifest has no way to declare the build +source inside a context (fact 6), so the planner could not obey rule 1 for these modules even if it +tried. + +## Why a rebuild of an unchanged program still moved + +Issue 280's fix should make a rebuild of an unchanged source no move. It does not here, for two +reasons. + +**The fingerprint reads the whole repository** (fact, internal/builder/builder.go and source.go). For +an artifact with a context, the builder clones the repository and records `git rev-parse HEAD^{tree}`, +the root tree. Any commit to the repository changes it, so two builds never share a fingerprint. + +**The program embeds the commit** (fact, measured). The builder clones the context with its `.git` +directory, the controller's repository has no `.dockerignore`, and the recipes run `COPY . .` and then +`go build -trimpath` without `-buildvcs=false`. Go's default then stamps the commit and its time into +the binary. Built on the laptop with the recipe's flags at `8170fc58`, `go version -m` on the route +proxy shows `vcs.revision=8170fc58…` and `vcs.time=2026-10-08T19:48:03Z`. So every commit gives a +different program even where no source it compiles changed. The golang base image carries git, so I +expect the same stamp inside the build; I did not inspect a built image. + +**And the image is not reproducible either.** Issue 280 already found that two `docker build`s of one +source give two digests. The build records confirm it: `build-agent` was built three times tonight +from catalogue commit `7b307a06` and got three different image digests. + +## What actually changed in the two programs + +To separate "the program changed" from "the plan moved it anyway", each program was built on the +laptop at each of tonight's merges, with `-trimpath -buildvcs=false`, and hashed. + +| merge | route proxy program | build seat program | +|---|---|---| +| `3f68a495` | `41b7e844` | `262ef27d` | +| `0cf5a3a5` | `41b7e844` (same) | `17ae6be8` | +| `ac91357a` | `41b7e844` (same) | `323e4388` | +| `e3ec15f7` | `41b7e844` (same) | `9e0b21b0` | +| `7f65e627` | `41b7e844` (same) | `8825b46e` | +| `9b028b42` (#163) | `41b7e844` (same) | `062d8c27` | +| `ca09a07f` (#159) | `f0a5dda5` | `87b9e34e` | +| `efcdd5dd` (#161) | `57669ce3` | `9c9fc316` | +| `8170fc58` (#165) | `57669ce3` (same) | `9c9fc316` (same) | + +The route proxy's program changed at two of nine merges. The build seat's program changed at eight of +nine. **This corrects part of the symptom as first reported**: PR #159 did change the route proxy's +program. The proxy imports `internal/broker`, `internal/lease` and `internal/envfile` from the +controller's repository, and #159 changed `internal/broker`. "Nothing under `examples/route-proxy` +changed" was true, and is the wrong test: a module's build source is its import closure, not its +directory. PR #163 (`9b028b42`) is the clean case: the route proxy's program was unchanged and it was +still recreated on the home server. + +So a path-precise rule that looked only at `examples/route-proxy` would have missed #159's real +change. The build source of the route proxy is `examples/route-proxy`, `internal/broker`, +`internal/lease`, `internal/envfile`, `go.mod`, `go.sum` and `vendor/`. + +## Measured: what the extra tiers cost + +From `mesh-controller.plans` (read only), the first-node gate of each tier of tonight's controller +walks: + +| walk | mesh-controller | build-agent | route-proxy | three gates | +|---|---|---|---|---| +| `3f68a495` | 3m40s | 3m43s | 3m46s | 11m09s | +| `0cf5a3a5` | 3m39s | 3m26s | 3m34s | 10m39s | +| `e3ec15f7` | 3m02s | 3m51s | 3m14s | 10m07s | +| `7f65e627` | 3m37s | 3m36s | 3m33s | 10m46s | +| `9b028b42` | 3m02s | 3m36s | 3m32s | 10m10s | +| `ca09a07f` | 3m43s | 3m46s | 3m40s | 11m09s | +| `efcdd5dd` | 3m38s | 3m40s | 3m37s | 10m55s | + +Two more walks (`a5f53ef9`, `ac91357a`) were superseded part-way, and `8170fc58` was still walking at +21:50. The gates alone are 10 to 11 minutes per walk, of which 7 minutes are the two packaging tiers. +Each tier also builds before it sends: the build seat's image took 53 seconds at 21:36. Estimated from +the plan's creation time and the build records, `ca09a07f` walked from 21:07 to about 21:23 and +`efcdd5dd` from 21:32 to about 21:47, about 15 minutes each. The controller's own tier, alone, is about +4 to 5 minutes. Of the seven completed walks, five recreated `route-proxy` on the home server +(`3f68a495`, `e3ec15f7`, `7f65e627`, `9b028b42`, `ca09a07f`), and its program had changed in one of +them. Four recreated `build-agent` (`e3ec15f7`, `7f65e627`, `9b028b42`, `ca09a07f`). + +## A second finding, not yet explained + +The deploy plan names a build of a packaging module by the module's own source commit, which is the +catalogue's. `moveFacts.identical` (cmd/mesh-controller/release.go) calls two builds identical when +their commits are equal. In `efcdd5dd`'s walk both modules went `7b307a06 → 7b307a06`, nothing was +said to be recreated, and the gate still took about 3.5 minutes for each. Yet the route proxy's +program did change at `efcdd5dd` (table above). **I do not know whether that program reached the home +server.** Either it was sent and the plan said nothing, or it was not sent and the route proxy runs the +program from before #161. Next step: compare the image digest the home server's route proxy runs with +the digest of `build-1791487124940030405` and of the build before it. The same identity by catalogue +commit would also make issue 280's "repeat" rule register a different program as the same build. + +## Ruled out + +- **The changed paths not reaching the planner.** PR #159's delivery record carries its paths and + `module_dirs_said: true`. They are read, and `touchedBy` uses them for the controller's own module. + They are never consulted for a packaging module. +- **The catalogue moving at the same time.** In some walks the catalogue's commit also changed + (`32307bd1 → 7b307a06`), but the README-only what-if shows the controller's repository alone is enough + to plan both modules. + +## Cause + +Located. A module whose build reads a second repository as a context moves on **every** merge to that +repository, because `mergeCandidates` adds it by repository and never maps the changed files onto it; +the dependency graph's `packages` edge then orders it after the repository's own module, and `built-by` +orders the route proxy after the build seat, giving three gated tiers. A rebuild that changed nothing +still moves, because the context's fingerprint is the whole repository's tree and the Go build stamps +the commit into the program. + +## Fix directions (not implemented) + +1. **A manifest declares its build's source paths within a repository**, and the planner uses only + those, plus the module graph's build edges. For a context this means a field beside `repository` + and `ref`, such as a list of paths. `touchedBy` then treats a context module like any other: it moves + when a changed file is inside one of its paths. The `packages` edge goes, or is drawn only when a + changed path is inside the declared set. The builder fingerprints the git trees of those paths, not + the root tree, so a rebuild with an unchanged source is no move. Manifests that must change: + `route-proxy` (paths: `examples/route-proxy`, `internal/broker`, `internal/lease`, + `internal/envfile`, `go.mod`, `go.sum`, `vendor`) and `build-agent` (`cmd/mesh-builder` and its + twelve internal packages, the same three root files). They are the only two with a context today. + The controller's own manifest builds from the root and so also claims `examples/` and + `cmd/mesh-builder`; it should declare its paths too, or a change to the route proxy alone moves the + controller. + - **Tradeoff:** a hand-written list goes stale. When the route proxy starts importing another + internal package and nobody adds it, a real change is missed, which is worse than a needless + rebuild. The list therefore needs a check: the merge gate or the repository's own check compares + the declared paths with the program's import closure (`go list -deps`) and fails when the closure + reaches outside them. A derived alternative is to compute the closure at build time and record it + with the build; that is exact for Go and needs a reader per language. +2. **Reproducible builds**: `-buildvcs=false` (or a `.dockerignore` that leaves `.git` out of the + context) in both recipes, so an unchanged source gives an identical program. This alone does not + stop the walk: the image digest still differs (issue 280), and the planner would still plan and gate + three tiers. It is cheap and worth doing beside 1, so the program's identity no longer depends on the + commit. +3. **Split the route proxy and the build seat into repositories of their own.** Then the existing + per-directory rule already applies, with no new manifest field. The tradeoff: the route proxy and + the build seat share `internal/broker`, `internal/catalogue` and more with the controller. Splitting + means a shared library in the SDK (where the mesh's rule says shared code belongs) or copies of it, + and a change to that library then moves all three again, through a real build edge. + +Recommendation: 1 with its check, and 2 beside it. A further question for the fix: whether `built-by` +should order a walk at all. It is a running-order edge for the build seat's worker (issue 206); the +operator's rule 2 counts only modules built *from* a changed module.