diff --git a/CHANGELOG.md b/CHANGELOG.md index f711601..c7c0f5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,9 +78,19 @@ test still green, because the two fields have the same type. The comparison now `Stack.CatalogImages`, taken from the syncer's git clone. **No readable catalog entry renders nothing.** No Hungarian string, badge state or partial changed. +### A fix must also refresh the stored definition — found by the LIVE validation, not by review + +The stored definition is written when the pin is written. A fix delivered afterwards landed in the +live compose file but **not** in the stored one — so the first time the catalog moved a version, the +freeze would have reverted every fix delivered since, **silently undoing the half of the ruling that +says fixes keep flowing**. The equal-images branch now refreshes the store as it delivers. The images +cannot move there by construction, so no version moves and no intent is rewritten. +`TestFixRefreshesTheStoredDefinition` asserts both halves: the fix reaches the store, and it survives +the subsequent freeze. + ### Tests -+16 (1729 → 1745). 28 packages green, 0 FAIL. New: `internal/sync/render_test.go`, ++17 (1729 → 1746). 28 packages green, 0 FAIL. New: `internal/sync/render_test.go`, `internal/stacks/pin_test.go`, Group G in `internal/web/updatebadge_test.go`. **Three companion red-proofs, each run, observed failing, and reverted (2026-09-06):** diff --git a/controller/internal/stacks/pin.go b/controller/internal/stacks/pin.go index 984874e..bebf08f 100644 --- a/controller/internal/stacks/pin.go +++ b/controller/internal/stacks/pin.go @@ -180,6 +180,11 @@ type RenderPlan struct { Protected bool Pinned map[string]string // service -> image ref; nil/empty means UNPINNED AppliedPath string // the stored definition; "" when none is stored + // StackDir lets the syncer REFRESH the stored definition when it copies the catalog verbatim + // into a pinned app whose images still match. See Syncer.renderSource — without that refresh the + // stored definition goes stale relative to the fixes that flowed after it, and the freeze would + // later undo them. Found by the live validation of v0.235.0, not by review. + StackDir string } // RenderPlanFor answers for one app by name. Unknown apps come back as an empty plan, which the @@ -198,6 +203,7 @@ func (m *Manager) RenderPlanFor(name string) RenderPlan { plan.Pinned = s.AppConfig.PinnedImages } stackDir := filepath.Dir(s.ComposePath) + plan.StackDir = stackDir if _, err := LoadAppliedDefinition(stackDir); err == nil { plan.AppliedPath = AppliedComposePath(stackDir) } diff --git a/controller/internal/sync/render_test.go b/controller/internal/sync/render_test.go index 18c055f..d18083f 100644 --- a/controller/internal/sync/render_test.go +++ b/controller/internal/sync/render_test.go @@ -265,3 +265,52 @@ func TestRenderTable_EmptyStoredDefinitionIsTreatedAsAbsent(t *testing.T) { t.Fatal("an empty compose file was written over a live app") } } + +// TestFixRefreshesTheStoredDefinition — found by the LIVE validation of v0.235.0, not by review. +// +// The stored definition is written when the pin is written. A fix delivered afterwards lands in the +// live compose file but NOT in the stored one — so the first time the catalog moves a version, the +// freeze reverts every fix delivered since, silently undoing the half of the ruling that says fixes +// keep flowing. The equal-images branch therefore refreshes the store as it delivers. +// +// The IMAGES cannot move here by construction: this branch only runs when the catalog's images equal +// the pin. No version moves and no intent is rewritten. +func TestFixRefreshesTheStoredDefinition(t *testing.T) { + s, stackDir, _ := renderFixture(t, tplOldFixed) // same version, fixed healthcheck + live := filepath.Join(stackDir, "docker-compose.yml") + write(t, live, tplOld) + write(t, stacks.AppliedComposePath(stackDir), tplOld) + s.SetRenderPlanFn(func(string) stacks.RenderPlan { + return stacks.RenderPlan{Deployed: true, StackDir: stackDir, + Pinned: map[string]string{"web": "nextcloud:31.0.14-apache"}, + AppliedPath: stacks.AppliedComposePath(stackDir)} + }) + + if _, _, err := s.copyTemplates(); err != nil { + t.Fatal(err) + } + stored, err := stacks.LoadAppliedDefinition(stackDir) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(stored), "status.php") { + t.Fatalf("the delivered fix must also become part of what the app is pinned TO:\n%s", stored) + } + if !strings.Contains(string(stored), "31.0.14-apache") { + t.Fatalf("refreshing the store must NOT move the version:\n%s", stored) + } + + // And now the catalog moves: the freeze must keep the fix, not revert to the pre-fix definition. + catDir := filepath.Join(filepath.Dir(filepath.Dir(stackDir)), "data", "catalog-cache", "templates", "nextcloud") + write(t, filepath.Join(catDir, "docker-compose.yml"), tplNew) + if _, _, err := s.copyTemplates(); err != nil { + t.Fatal(err) + } + got := readFile(t, live) + if strings.Contains(got, "34.0.1") { + t.Fatalf("the version must be frozen:\n%s", got) + } + if !strings.Contains(got, "status.php") { + t.Fatalf("the freeze must keep the fix that was delivered while the versions matched:\n%s", got) + } +} diff --git a/controller/internal/sync/sync.go b/controller/internal/sync/sync.go index 0e19274..c82842c 100644 --- a/controller/internal/sync/sync.go +++ b/controller/internal/sync/sync.go @@ -377,12 +377,13 @@ func (s *Syncer) copyTemplates() (newApps []string, updated []string, err error) // v0.235.0 — the RENDER. `.felhom.yml` is always copied verbatim (it holds no image); // only the compose file can be frozen. See renderSource for the whole table. + refreshAppliedIn := "" if filename == "docker-compose.yml" { - frozenSrc, skip := s.renderSource(appName, src) + renderedSrc, skip, refresh := s.renderSource(appName, src) if skip { continue } - src = frozenSrc + src, refreshAppliedIn = renderedSrc, refresh } changed, err := copyIfChanged(src, dst) @@ -393,6 +394,21 @@ func (s *Syncer) copyTemplates() (newApps []string, updated []string, err error) if changed { anyChanged = true s.logger.Printf("[INFO] [sync] Updated %s/%s", appName, filename) + // The fix we just delivered becomes part of what this app is pinned TO. Without + // this the stored definition stays as it was when the pin was written, and the + // first time the catalog moves the freeze reverts every fix delivered since — + // silently undoing the half of the ruling that says fixes keep flowing. + // The IMAGES are unchanged here by construction (this branch only runs when the + // catalog's images equal the pin), so no version moves and no intent is rewritten. + if refreshAppliedIn != "" { + if data, rerr := os.ReadFile(src); rerr != nil { + s.logger.Printf("[WARN] [sync] %s: could not re-read the template to refresh its stored definition: %v", appName, rerr) + } else if serr := stacks.StoreAppliedDefinition(refreshAppliedIn, data); serr != nil { + s.logger.Printf("[WARN] [sync] %s: could not refresh the stored definition: %v", appName, serr) + } else if s.isDebug() { + s.logger.Printf("[DEBUG] [sync] %s: stored definition refreshed with the delivered fix", appName) + } + } if s.isDebug() { s.logFileHashes(appName, filename, src, dst) } @@ -438,27 +454,27 @@ func (s *Syncer) copyTemplates() (newApps []string, updated []string, err error) // deploy fields reaching a deployed app, and it destroys the self-healing measured in // SPIKE-app-update-2026-09-01 §3 — a hand-broken compose file repaired itself within 15 minutes. // Both halves were worth keeping; only the version change was not. -func (s *Syncer) renderSource(appName, catalogSrc string) (src string, skip bool) { +func (s *Syncer) renderSource(appName, catalogSrc string) (src string, skip bool, refreshAppliedIn string) { if s.renderPlanFn == nil { - return catalogSrc, false // pre-v0.235.0 behaviour, byte for byte + return catalogSrc, false, "" // pre-v0.235.0 behaviour, byte for byte } plan := s.renderPlanFn(appName) if !plan.Deployed || plan.Protected { - return catalogSrc, false + return catalogSrc, false, "" } if plan.Deploying { // Do not race an in-flight deploy for its own compose file. if s.isDebug() { s.logger.Printf("[DEBUG] [sync] %s: mid-deploy, compose file left alone this cycle", appName) } - return "", true + return "", true, "" } if len(plan.Pinned) == 0 { if s.isDebug() { s.logger.Printf("[DEBUG] [sync] %s: deployed but UNPINNED — catalog copied verbatim (pre-v0.235.0 behaviour)", appName) } - return catalogSrc, false + return catalogSrc, false, "" } catalogImages, err := stacks.ParseComposeImages(catalogSrc) @@ -466,19 +482,20 @@ func (s *Syncer) renderSource(appName, catalogSrc string) (src string, skip bool // CANNOT TELL whether the catalog has moved. Leave the app's file alone rather than guess in // either direction — an unreadable catalog template must not be able to unfreeze an app. s.logger.Printf("[WARN] [sync] %s: cannot read the catalog template's images (%v) — compose file left alone this cycle", appName, err) - return "", true + return "", true, "" } if samePin(plan.Pinned, catalogImages) { // The catalog still offers what this app runs: everything else in the template is a FIX and // is delivered, exactly as before v0.235.0. This branch is why the feature is not a freeze. - return catalogSrc, false + // The delivered fix also REFRESHES the stored definition — see the call site. + return catalogSrc, false, plan.StackDir } if plan.AppliedPath == "" { // We cannot freeze what we do not have, and we must not invent it. s.logger.Printf("[WARN] [sync] %s: the catalog has moved past this app's pinned version, but no stored definition exists — copying the catalog verbatim (pre-v0.235.0 behaviour). The app will take the new version on its next start.", appName) - return catalogSrc, false + return catalogSrc, false, "" } // DEFENCE IN DEPTH, and this line was added because a test demanded it: the manager's // RenderPlanFor already refuses to hand over a path whose file is missing or empty, but the @@ -486,12 +503,12 @@ func (s *Syncer) renderSource(appName, catalogSrc string) (src string, skip bool // The cost of re-reading a small file once per app per cycle is nothing next to that. if data, err := os.ReadFile(plan.AppliedPath); err != nil || len(strings.TrimSpace(string(data))) == 0 { s.logger.Printf("[WARN] [sync] %s: the stored definition is missing or empty (%v) — copying the catalog verbatim rather than writing an empty compose file", appName, err) - return catalogSrc, false + return catalogSrc, false, "" } if s.isDebug() { s.logger.Printf("[DEBUG] [sync] %s: catalog has moved past the pin — rendering the stored applied definition", appName) } - return plan.AppliedPath, false + return plan.AppliedPath, false, "" } // samePin compares a pin against a template's images. Local to the syncer so this package needs