diff --git a/CHANGELOG.md b/CHANGELOG.md index 97e4809..70b35a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,32 @@ ## Changelog +### v0.158.1 — fix: the lifecycle methods broke every app detail page (2026-07-21) + +**Defect shipped in v0.158.0 and caught live within the hour. `/apps/` returned HTTP 500 for +EVERY app**, not just withdrawn ones. + +`EffectiveLifecycle` / `CanInstall` / `IsAbandoned` were declared with POINTER receivers. +`appDetailHandler` puts `data["Meta"] = found.Meta` — a `stacks.Metadata` VALUE inside a +`map[string]interface{}` — and html/template cannot call a pointer-receiver method on a +non-addressable value. So `{{if .Meta.IsAbandoned}}` failed at RENDER time: + +``` +executing "app_info" at <.Meta.IsAbandoned>: can't evaluate field IsAbandoned in type interface {} +``` + +Switched to value receivers, with the reason recorded at the declaration so it is not "tidied" back. + +**Why the tests missed it, which is the more useful lesson:** it compiles, `go vet` is silent, and +every v0.158.0 test passed — because none of them rendered `app_info`. The catalog-page tests +exercised the funcmap route (`lifecycleBadge .Meta`), which takes a value and works either way. A +template method call is only ever checked when the template actually runs. + +Added `TestAppInfoRendersForEveryLifecycle`, which renders the real `app_info` template through the +production tree with the handler's exact data shape — `"Meta"` as a VALUE in a +`map[string]interface{}`, deliberately not a pointer, because the pointer is what hides the bug. +Red-proof: restoring the pointer receiver reproduces the 500 for every lifecycle value including the +empty one. + ### v0.158.0 — apps get a lifecycle: available / hidden / abandoned (2026-07-21) No agent coupling; MinAgent unchanged. diff --git a/controller/internal/stacks/metadata.go b/controller/internal/stacks/metadata.go index c8a0017..13b9eb4 100644 --- a/controller/internal/stacks/metadata.go +++ b/controller/internal/stacks/metadata.go @@ -220,7 +220,7 @@ const ( // or a state added in a later release, silently pull a working app out of every customer's catalog. // The gate that actually protects against installing something is `CanInstall`, and it is fed by // this same function, so the two can never disagree. -func (m *Metadata) EffectiveLifecycle() string { +func (m Metadata) EffectiveLifecycle() string { switch m.Lifecycle { case "", LifecycleAvailable: return LifecycleAvailable @@ -233,13 +233,20 @@ func (m *Metadata) EffectiveLifecycle() string { } } +// VALUE receivers on all three, deliberately: html/template cannot call a POINTER-receiver method +// on a non-addressable value, and `.Meta` reaches the templates as a stacks.Metadata VALUE inside a +// map[string]interface{}. With a pointer receiver `{{if .Meta.IsAbandoned}}` fails at RENDER time +// with "can't evaluate field IsAbandoned in type interface {}" — which is a 500 on the page, not a +// compile error, so nothing catches it until someone loads the page. That is exactly how it shipped +// once (v0.158.0, caught live the same hour). Keep these value receivers. +// // CanInstall reports whether this app may be offered/installed. The catalog listing and the deploy // endpoint MUST both use this — a template excluded from the list but accepted by a direct POST // would be a gate in name only. -func (m *Metadata) CanInstall() bool { return m.EffectiveLifecycle() == LifecycleAvailable } +func (m Metadata) CanInstall() bool { return m.EffectiveLifecycle() == LifecycleAvailable } // IsAbandoned reports whether a DEPLOYED instance should carry the "no longer maintained" notice. -func (m *Metadata) IsAbandoned() bool { return m.EffectiveLifecycle() == LifecycleAbandoned } +func (m Metadata) IsAbandoned() bool { return m.EffectiveLifecycle() == LifecycleAbandoned } // LoadMetadata reads .felhom.yml from a stack directory. // Returns default metadata if the file doesn't exist. diff --git a/controller/internal/web/lifecycle_test.go b/controller/internal/web/lifecycle_test.go index 7b61c96..4ad1093 100644 --- a/controller/internal/web/lifecycle_test.go +++ b/controller/internal/web/lifecycle_test.go @@ -136,3 +136,61 @@ func TestAvailableAppHasNoBadgeAndKeepsInstallButton(t *testing.T) { t.Fatal("an available, undeployed app must still offer Telepítés") } } + +// appInfoData mirrors appDetailHandler's data map — CRUCIALLY including `"Meta": st.Meta` as a +// stacks.Metadata VALUE inside a map[string]interface{}, which is what the handler really does. +// That detail is the entire point of this test: html/template cannot call a POINTER-receiver method +// on a non-addressable value, so `{{if .Meta.IsAbandoned}}` renders fine in a unit test that passes +// a *Metadata and 500s in production. Do not "simplify" this to a pointer. +func appInfoData(st stacks.Stack) map[string]interface{} { + return map[string]interface{}{ + "Page": "stacks", "Title": st.Meta.DisplayName, + "Stack": &st, "Meta": st.Meta, "AppInfo": st.Meta.AppInfo, + "HasAppInfo": st.Meta.HasAppInfo(), "EffectiveSubdomain": st.Meta.Subdomain, + "Domain": "demo-felhom.eu", + } +} + +// TestAppInfoRendersForEveryLifecycle is the regression test for a defect that SHIPPED: v0.158.0's +// first cut used pointer receivers, so `{{if .Meta.IsAbandoned}}` failed at RENDER time with +// "can't evaluate field IsAbandoned in type interface {}" — a 500 on EVERY app detail page, +// abandoned or not. It compiled, and every other test passed, because nothing rendered app_info. +// +// A template method call is only checked when the template runs. Render the page. +func TestAppInfoRendersForEveryLifecycle(t *testing.T) { + for _, lc := range []string{"", "available", "hidden", "abandoned", "bogus"} { + t.Run("lifecycle="+lc, func(t *testing.T) { + st := lcStack("someapp", lc, true) + st.State = stacks.StateRunning + html := renderBackupPage(t, "app_info", appInfoData(st)) + if len(html) < 500 { + t.Fatalf("app_info rendered only %d bytes — the page is broken", len(html)) + } + wantNotice := lc == "abandoned" + gotNotice := strings.Contains(html, "felhagyott a fejlesztéssel") + if gotNotice != wantNotice { + t.Errorf("abandoned notice present=%v, want %v (lifecycle %q)", gotNotice, wantNotice, lc) + } + gotBadge := strings.Contains(html, "Nem karbantartott") + if gotBadge != wantNotice { + t.Errorf("badge present=%v, want %v (lifecycle %q)", gotBadge, wantNotice, lc) + } + }) + } +} + +// TestAppInfoHidesInstallButtonForWithdrawnApp — the undeployed view: a withdrawn app must not offer +// Telepítés, and an available one must. +func TestAppInfoHidesInstallButtonForWithdrawnApp(t *testing.T) { + for _, c := range []struct { + lifecycle string + wantBtn bool + }{{"", true}, {"available", true}, {"hidden", false}, {"abandoned", false}} { + st := lcStack("someapp", c.lifecycle, false) + html := renderBackupPage(t, "app_info", appInfoData(st)) + got := strings.Contains(html, "Telepítés") + if got != c.wantBtn { + t.Errorf("lifecycle %q: Telepítés button present=%v, want %v", c.lifecycle, got, c.wantBtn) + } + } +}