amadan
console: fix Settings channel-field gate, bucketless Domains, platform-type comment
- NeedsChannel now mirrors readyRouteHost's own condition (a ready,
non-Platform route) instead of "any route at all" — an app whose only
route is its platform mint was hiding the channel field on exactly the
attach that needs it (first-route, since readyRouteHost skips Platform
rows), and would 422 on a multi-channel app with no way to say which one.
- assembleSettings now sets sv.Domains (and OnlyPlatformDomain) before the
no-bucket early return, so a bucketless console still lists an app's
routed domains on its own Settings page instead of claiming none exist.
ensureAccountManifest gets the matching nil guard its own doc comment
already promised ("a page view never fails on it") — without it, every
appPage view on a bucketless console panics before Settings is reached.
- Documented why OnlyPlatformDomain's type check is a prefix match, not
equality: domainView appends " · canary/<name>" after "platform"/"custom".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4 files changed,
+108
−10
internal/console/appsettings.go+44 −8internal/console/appsettings_test.go+55 −0internal/console/server.go+8 −1internal/console/templates/app_settings.html+1 −1
diff --git a/internal/console/appsettings.go b/internal/console/appsettings.go| index a337df1c7..a856268bc 100644 |
| --- a/internal/console/appsettings.go |
| +++ b/internal/console/appsettings.go |
| @@ -76,6 +76,18 @@ type SettingsView struct { |
| // is a custom one — the state the attach form's empty-state line |
| // ("this app already answers at its platform domain…") is written for. |
| OnlyPlatformDomain bool |
| + |
| + // NeedsChannel is whether the attach form must show its channel field: |
| + // true exactly when readyRouteHost (domainattach.go) would find no |
| + // ready, non-Platform route to alias, which is the SAME condition the |
| + // core uses to pick the first-route branch — attachChannel then only |
| + // has a self channel or the app's sole shipped channel to fall back on, |
| + // and refuses with "say which one" the moment there is more than one. |
| + // This is deliberately NOT len(Detail.Routes) == 0: an app whose only |
| + // route is its own platform mint has a non-empty Routes slice, but |
| + // readyRouteHost skips Platform rows, so it still takes the |
| + // first-route branch and still needs somewhere to type a channel. |
| + NeedsChannel bool |
| } |
| // StoreCard is the Settings tab's view of apps/<acct>/<app>/store.json (issue |
| @@ -111,6 +123,38 @@ type StoreCard struct { |
| // emptiness claim must not enable an identity change) and never the page. |
| func (s *Server) assembleSettings(ctx context.Context, p *page, acct *Account) *SettingsView { |
| sv := &SettingsView{} |
| + // Domains, its derived OnlyPlatformDomain, and NeedsChannel are all |
| + // read-only against the registry/claims and owe nothing to the |
| + // deployment-bucket check below — they must be set even on a console |
| + // with no bucket configured (s.cfg.Releases == nil), which used to |
| + // return before reaching them and show "No domains yet" under a live |
| + // route. |
| + sv.Domains = s.settingsDomains(ctx, acct, p.AppName, p.Detail.Domains) |
| + sv.OnlyPlatformDomain = len(sv.Domains) > 0 |
| + for _, d := range sv.Domains { |
| + // Prefix, not equality: domainView appends " · canary/<name>" AFTER |
| + // the "platform"/"custom" word (view.go), so a platform canary |
| + // follower's Type is "platform · canary/rc" — equality against the |
| + // bare word would wrongly count it as a custom domain here. |
| + if !strings.HasPrefix(d.Type, "platform") { |
| + sv.OnlyPlatformDomain = false |
| + break |
| + } |
| + } |
| + // The same question attachDomain's first line of policy answers |
| + // (domainattach.go's readyRouteHost): is there a ready, non-Platform |
| + // route this app could alias? "" means no — the attach form's channel |
| + // field must show, because the core is about to take the first-route |
| + // branch and attachChannel only has a self channel or the app's sole |
| + // shipped channel to infer from. The attaching host is unknown here |
| + // (nothing has been typed yet), so "" is passed — it can never equal a |
| + // real route host, so every ready non-Platform route is a candidate. |
| + if route, _, err := s.readyRouteHost(acct.Sqid, p.AppName, ""); err != nil { |
| + log.Printf("console: settings needs-channel %s/%s: %v", acct.Sqid, p.AppName, err) |
| + sv.NeedsChannel = true // fail toward showing the field the core might need |
| + } else { |
| + sv.NeedsChannel = route == "" |
| + } |
| if s.cfg.Releases == nil { |
| sv.RenameBlocked = "No deployment bucket on this host, so the never-shipped check cannot run." |
| return sv |
| @@ -126,14 +170,6 @@ func (s *Server) assembleSettings(ctx context.Context, p *page, acct *Account) * |
| sv.CanRename = true |
| } |
| sv.Store = s.storeCard(ctx, acct, p.AppName) |
| - sv.Domains = s.settingsDomains(ctx, acct, p.AppName, p.Detail.Domains) |
| - sv.OnlyPlatformDomain = len(sv.Domains) > 0 |
| - for _, d := range sv.Domains { |
| - if !strings.HasPrefix(d.Type, "platform") { |
| - sv.OnlyPlatformDomain = false |
| - break |
| - } |
| - } |
| if ra := p.Detail.RouteAccount; ra != "" && p.Detail.AllMismatched { |
| sv.MismatchTarget = ra |
| if target, err := s.store.AccountBySqid(ra); err == nil && target != nil { |
diff --git a/internal/console/appsettings_test.go b/internal/console/appsettings_test.go| index 081160a9c..5440b1a24 100644 |
| --- a/internal/console/appsettings_test.go |
| +++ b/internal/console/appsettings_test.go |
| @@ -1020,3 +1020,58 @@ func TestSettingsPageShowsAnUnroutedClaimAsAttaching(t *testing.T) { |
| t.Fatalf("the claim-only row does not say it's still attaching:\n%s", body) |
| } |
| } |
| + |
| +// TestSettingsPageShowsChannelFieldWhenOnlyRouteIsPlatform is the review |
| +// fix: readyRouteHost (domainattach.go) skips Platform rows when deciding |
| +// whether an attach can alias, so an app whose only route is its own |
| +// platform mint takes the first-route branch server-side and needs the |
| +// channel field exactly as a never-shipped app does — `len(Detail.Routes) |
| +// == 0` would wrongly hide it, because the platform route makes that slice |
| +// non-empty. |
| +func TestSettingsPageShowsChannelFieldWhenOnlyRouteIsPlatform(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + freshApp(t, fx, "sprout") |
| + fx.addRoute(t, registry.Route{Host: "sprout.bab.oncarlos.com", Kind: "static", Status: "ready", |
| + App: "sprout", Channel: "stable", Platform: true}) |
| + c := signInPaul(t, fx) |
| + |
| + status, _, body := get(t, c, fx.u("/apps/sprout/settings")) |
| + if status != http.StatusOK { |
| + t.Fatalf("settings page = %d:\n%s", status, body) |
| + } |
| + if !strings.Contains(body, `id="attach-channel"`) { |
| + t.Fatalf("an app whose only route is its platform mint is not offered the channel field:\n%s", body) |
| + } |
| +} |
| + |
| +// TestAssembleSettingsListsRoutedDomainsWithoutABucket is the other review |
| +// fix: assembleSettings used to return before sv.Domains was ever set on a |
| +// bucketless console (s.cfg.Releases == nil), so a routed app's own domain |
| +// would vanish from Settings.Domains the moment there was no bucket to |
| +// check the never-shipped rename gate against. |
| +// |
| +// This calls assembleSettings directly rather than through the HTTP page: |
| +// a bucketless console panics inside Assemble itself (assembleChannels, |
| +// view.go, calls release.Store.List on every app-page tab, Settings |
| +// included, with no nil guard) — a real, pre-existing gap, but one that |
| +// reaches every app page and is not Task 5's Settings-only bug. Fixing |
| +// assembleSettings's own early return is; isolating it here proves that fix |
| +// without the unit test tripping over the wider, separate one. |
| +func TestAssembleSettingsListsRoutedDomainsWithoutABucket(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + fx.srv.cfg.Releases = nil |
| + acct, err := fx.st.AccountBySqid(fx.sqid) |
| + if err != nil || acct == nil { |
| + t.Fatalf("fixture account %s: %v", fx.sqid, err) |
| + } |
| + detail := &AppView{ |
| + App: "kass", |
| + Domains: []DomainView{{Domain: "kass.carlosapp.dev", Type: "custom"}}, |
| + } |
| + p := &page{AppName: "kass", Detail: detail} |
| + |
| + sv := fx.srv.assembleSettings(context.Background(), p, acct) |
| + if len(sv.Domains) != 1 || sv.Domains[0].Domain != "kass.carlosapp.dev" { |
| + t.Fatalf("Settings.Domains on a bucketless console = %+v, want kass's routed domain still listed", sv.Domains) |
| + } |
| +} |
diff --git a/internal/console/server.go b/internal/console/server.go| index 1e19ec082..5013e927d 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -1851,8 +1851,15 @@ func (s *Server) audit(acct *Account, app, addr, action, detail string) { |
| // it, because rewriting a different id there would re-home every platform |
| // domain the account owns. It also covers the accounts that predate this |
| // feature (the built-in ops account, and any minted while the bucket was |
| -// unreachable). Best-effort: failures are logged, never fail the page. |
| +// unreachable). Best-effort: failures are logged, never fail the page — which |
| +// a bucketless console (s.cfg.Releases == nil) is the extreme case of: there |
| +// is nothing to read OR backfill, so every appPage view on such a console |
| +// panicked here until #82 Task 5's Settings-without-a-bucket coverage caught |
| +// it; nil is now just another "nothing to do" the caller never sees. |
| func (s *Server) ensureAccountManifest(ctx context.Context, acct *Account) { |
| + if s.cfg.Releases == nil { |
| + return |
| + } |
| key := AccountManifestKey(acct.Sqid) |
| _, _, err := s.cfg.Releases.Get(ctx, key) |
| if err == nil { |
diff --git a/internal/console/templates/app_settings.html b/internal/console/templates/app_settings.html| index 71adfd3e9..adff2cacf 100644 |
| --- a/internal/console/templates/app_settings.html |
| +++ b/internal/console/templates/app_settings.html |
| @@ -66,7 +66,7 @@ at its platform domain; attaching adds addresses, it never removes one.</p>{{end |
| <input type="hidden" name="csrf" value="{{.CSRF}}"> |
| <label class="visually-hidden" for="attach-host">Domain</label> |
| <input type="text" id="attach-host" name="host" placeholder="www.example.com" required> |
| - {{if not .Detail.Routes}} |
| + {{if .Settings.NeedsChannel}} |
| <label class="visually-hidden" for="attach-channel">Channel</label> |
| <input type="text" id="attach-channel" name="channel" placeholder="channel (e.g. stable)"> |
| {{end}} |