From 7ade322bcb4d6b2bdfb07d0cd5ef3058b141e9b9 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 21:59:18 +0200 Subject: [PATCH] Register issue 338: a module built from a shared repository moves on every merge to it A controller merge walks three gated tiers because the route proxy and the build seat are planned by repository, not by the paths their builds read, and their programs carry the commit. Located; fix directions recorded. --- .../00-report.md | 63 ++++++ .../01-diagnosis.md | 195 ++++++++++++++++++ 2 files changed, 258 insertions(+) create mode 100644 04-ISSUES/338-a-module-built-from-a-shared-repository-moves-on-every-merge-to-it/00-report.md create mode 100644 04-ISSUES/338-a-module-built-from-a-shared-repository-moves-on-every-merge-to-it/01-diagnosis.md 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.