amadan
console: pipeline UI fix round — flow diagram, rollback gate order, bake-window agreement
Review fixes (1 Critical, 5 Important): - CRITICAL: the Overview page's flow diagram (flow.go, app_overview.html) drew the frozen edge/beta/live path unconditionally, independent of the Releases tab's pipeline-aware ladder card. assembleFlow now reads view.Pipeline: a one-channel pipeline hides the diagram entirely, a multi-channel one draws its own declared channels in order, reusing the same holdWindow resolver the ladder card already shares rather than a second implementation. - IMPORTANT: handleRollback's legacy-stable owner gate ran after the confirm card rendered — hoisted above it, mirroring handlePromote's own documented placement; the Roll back button is now hidden from non-owners on a legacy stable row too. - IMPORTANT: ChannelView.BakedUntil was computed from the legacy 72h-table in Assemble, while the ladder card recomputed its own correct answer via holdWindow — two implementations free to disagree. Fixed at the source: a new applyPipelineView (server.go), called from appPage, corrects BakedUntil once and rebuilds Ladder from it; assemblePipelineLadder no longer recomputes it. - IMPORTANT: the legacy Safety-delay line on the Overview page dangled for pipelined apps (pointed at a toggle the ladder card no longer offers) — suppressed for pipelined apps, matching ladder.html's own choice. - IMPORTANT: the pipeline read moved out of Assemble (which the account home page also calls, for a whole app list, never reading Ladder or Pipeline) into appPage — zero new bucket reads on the home page. - IMPORTANT: added TestPipelineSettingsSavePOST, covering requireFreshAuth enforcement, an unguarded save landing, and a guarded edit proposing. - Minors: "1 member" singular; pending card shows Kind; a hidden save button leads the settings form's tree order so Enter doesn't trigger row 0's Remove. New tests: TestOneChannelAppOverviewHidesFlowDiagram, TestMultiChannelFlowDrawsPipelineChannels, TestChannelsTableAndPipelineCardAgreeOnBakeWindow, TestPipelineSettingsSavePOST; TestLegacyStableRollbackGatedByOwnerAndStepUp extended for card-not-shown.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
10 files changed,
+440
−71
internal/console/flow.go+61 −15internal/console/ladder.go+21 −22internal/console/pipelinepage_test.go+235 −0internal/console/server.go+70 −12internal/console/server_test.go+18 −3internal/console/templates/app_overview.html+7 −0internal/console/templates/app_releases.html+1 −1internal/console/templates/pending.html+1 −1internal/console/templates/pipeline.html+4 −0internal/console/view.go+22 −17
diff --git a/internal/console/flow.go b/internal/console/flow.go| index c667ee980..55bd0b4f7 100644 |
| --- a/internal/console/flow.go |
| +++ b/internal/console/flow.go |
| @@ -153,11 +153,19 @@ func needsAttention(health string) bool { |
| return false |
| } |
| -// flowLine is the release path, in the order versions travel it, with the |
| -// label each box carries. "live" rather than "stable" is deliberate and |
| -// matches the Versions tab's strip: stable is the channel's name, live is |
| -// what it means. |
| -var flowLine = []struct{ name, channel string }{ |
| +// flowLineEntry is one box's name/channel pair on the release line — the |
| +// legacy frozen three (flowLine below), or a pipelined app's own declared |
| +// channels, each named for itself (assembleFlow builds that list from |
| +// view.Pipeline.Channels). |
| +type flowLineEntry struct{ name, channel string } |
| + |
| +// flowLine is the LEGACY release path, in the order versions travel it, |
| +// with the label each box carries. "live" rather than "stable" is |
| +// deliberate and matches the Versions tab's strip: stable is the channel's |
| +// name, live is what it means. Used only when view.Pipeline is nil — |
| +// legacy-path-only, exactly as the ladder card and the CLI's promote gate |
| +// already are (spec §7). |
| +var flowLine = []flowLineEntry{ |
| {"edge", "edge"}, |
| {"beta", "beta"}, |
| {"live", release.RungStable}, |
| @@ -167,7 +175,18 @@ var flowLine = []struct{ name, channel string }{ |
| // into the diagram. latest/latestAt are the newest shipped version and its |
| // build time ("" when nothing has been shipped). instancesPath is the app's |
| // Running tab URL, used as MoreURL on any stage that rolls up. |
| +// |
| +// Pipeline-aware (Task 9 review, the CRITICAL finding): a one-channel |
| +// pipeline hides this diagram entirely, same as the ladder card and the |
| +// promote form (spec §5 — "the app page shows a Releases list and a live |
| +// marker", not a picture of a ladder that does not exist); a multi-channel |
| +// pipeline draws its OWN declared channels, in order, rather than the |
| +// frozen edge/beta/live boxes a pipelined app may not even have. Legacy |
| +// (view.Pipeline == nil) is byte-identical to before this awareness existed. |
| func assembleFlow(view *AppView, rows []InstanceRow, latest, latestAt, instancesPath string) FlowView { |
| + if view.OneChannelPipeline() { |
| + return FlowView{} |
| + } |
| f := FlowView{Shipped: latest} |
| if t, err := time.Parse(time.RFC3339, latestAt); err == nil { |
| f.ShippedAt = t |
| @@ -189,10 +208,39 @@ func assembleFlow(view *AppView, rows []InstanceRow, latest, latestAt, instances |
| for _, c := range view.Channels { |
| chans[c.Name] = c |
| } |
| - // An unreadable production flag counts as ON, because that is what the |
| - // boxes do with it (they keep baking) — the diagram must not tell an |
| - // owner their delay is off while every box is still holding. |
| - held := view.Production.On || view.Production.Err != "" |
| + |
| + // line is the release path this app actually has, and holdFor is how |
| + // each box on it resolves its general "waits ~24h here" line — the two |
| + // things that differ between a legacy app and a pipelined one. Neither |
| + // duplicates the ladder card's own logic: line is read straight off |
| + // view.Pipeline.Channels (the same order assemblePipelineLadder reads), |
| + // and holdFor calls the SAME holdWindow resolver the ladder card and the |
| + // promote/fast-track gates already share — this is the one new call |
| + // site, not a second implementation. |
| + var line []flowLineEntry |
| + var holdFor func(channel string) time.Duration |
| + if view.Pipeline != nil { |
| + line = make([]flowLineEntry, len(view.Pipeline.Channels)) |
| + for i, c := range view.Pipeline.Channels { |
| + line[i] = flowLineEntry{c.Name, c.Name} |
| + } |
| + holdFor = func(channel string) time.Duration { return holdWindow(view.Pipeline, channel) } |
| + } else { |
| + line = flowLine |
| + // An unreadable production flag counts as ON, because that is what |
| + // the boxes do with it (they keep baking) — the diagram must not |
| + // tell an owner their delay is off while every box is still |
| + // holding. Legacy-path-only: a pipelined app's bake policy comes |
| + // solely from its own pipeline and never consults this flag at all |
| + // (spec §3) — holdFor above already reflects that. |
| + held := view.Production.On || view.Production.Err != "" |
| + holdFor = func(channel string) time.Duration { |
| + if !held { |
| + return 0 |
| + } |
| + return release.HoldFor(release.DefaultHolds, channel) |
| + } |
| + } |
| stage := func(name, channel string) FlowStage { |
| c := chans[channel] |
| @@ -202,10 +250,8 @@ func assembleFlow(view *AppView, rows []InstanceRow, latest, latestAt, instances |
| BakedUntil: c.BakedUntil, FastTrack: c.FastTrack, |
| Addresses: byChannel[channel], |
| } |
| - if held { |
| - if d := release.HoldFor(release.DefaultHolds, channel); d > 0 { |
| - s.Hold = holdText(d) |
| - } |
| + if d := holdFor(channel); d > 0 { |
| + s.Hold = holdText(d) |
| } |
| // A channel with addresses but nothing ever published to it is the |
| // silent failure the ladder was built to surface: hosts serving |
| @@ -238,7 +284,7 @@ func assembleFlow(view *AppView, rows []InstanceRow, latest, latestAt, instances |
| // share a version string ("") without any version having travelled |
| // between them, and drawing that as a solid arrow would be the diagram's |
| // one chance to lie outright. |
| - for _, ls := range flowLine { |
| + for _, ls := range line { |
| f.Line = append(f.Line, stage(ls.name, ls.channel)) |
| } |
| prev := latest |
| @@ -250,7 +296,7 @@ func assembleFlow(view *AppView, rows []InstanceRow, latest, latestAt, instances |
| // Canaries from their pointers first, then any canary channel an address |
| // follows that has no pointer at all. |
| drawn := map[string]bool{} |
| - for _, ls := range flowLine { |
| + for _, ls := range line { |
| drawn[ls.channel] = true |
| } |
| for _, c := range view.Channels { |
diff --git a/internal/console/ladder.go b/internal/console/ladder.go| index 439952e86..883628ef2 100644 |
| --- a/internal/console/ladder.go |
| +++ b/internal/console/ladder.go |
| @@ -142,7 +142,7 @@ var gates = map[string]string{ |
| // section exists to stop being invisible. |
| func assembleLadder(channels []ChannelView, routes []RouteView, pipeline *release.Pipeline, now time.Time) []LadderRung { |
| if pipeline != nil { |
| - return assemblePipelineLadder(pipeline, channels, routes, now) |
| + return assemblePipelineLadder(pipeline, channels, routes) |
| } |
| prod := prodWiredCanaryHosts(routes) |
| expiredFor := canaryExpiry(channels, now) |
| @@ -237,7 +237,7 @@ func stepUp(from, to string, chans []LadderChannel) *LadderStep { |
| // AppView.OneChannelPipeline() and skip the ladder card, the promote form |
| // and the channel table outright — this function simply never hands them |
| // anything to draw in that case. |
| -func assemblePipelineLadder(p *release.Pipeline, channels []ChannelView, routes []RouteView, now time.Time) []LadderRung { |
| +func assemblePipelineLadder(p *release.Pipeline, channels []ChannelView, routes []RouteView) []LadderRung { |
| if len(p.Channels) < 2 { |
| return nil |
| } |
| @@ -257,26 +257,21 @@ func assemblePipelineLadder(p *release.Pipeline, channels []ChannelView, routes |
| for i, c := range p.Channels { |
| var chans []LadderChannel |
| if cv, ok := byChannel[c.Name]; ok { |
| - lc := LadderChannel{ |
| + // BakedUntil is copied verbatim from cv, exactly as the legacy |
| + // branch above does — it is NOT recomputed here. The caller |
| + // (applyPipelineView, server.go) is the one place a pipelined |
| + // app's Channels get their BakedUntil corrected to THIS |
| + // channel's own window (holdWindow) in place of the legacy |
| + // per-app table, precisely so this card and the Channels table |
| + // reading the same Channels slice can never disagree (Task 9 |
| + // review) — recomputing it again here would be a second |
| + // implementation of the same answer, free to drift from the |
| + // first. |
| + chans = []LadderChannel{{ |
| Channel: cv.Name, Version: cv.Version, PromotedAt: cv.PromotedAt, |
| Actor: cv.Actor, Hotfix: cv.Hotfix, Err: cv.Err, |
| - FastTrack: cv.FastTrack, Routes: byRoute[c.Name], |
| - } |
| - // The window is THIS channel's own (holdWindow, apipipeline.go) — |
| - // never the legacy per-app BakedUntil the shared ChannelView |
| - // slice may already carry, which was computed from the legacy |
| - // production flag and the frozen HoldFor table (view.go's |
| - // assembleChannels) and means nothing for a name the legacy |
| - // table has never heard of (spec §3: a pipelined app's bake |
| - // policy comes solely from its own pipeline). |
| - if cv.Err == "" && !cv.FastTrack { |
| - if d := holdWindow(p, c.Name); d > 0 { |
| - if until := cv.PromotedAt.Add(d); now.Before(until) { |
| - lc.BakedUntil = until |
| - } |
| - } |
| - } |
| - chans = []LadderChannel{lc} |
| + FastTrack: cv.FastTrack, BakedUntil: cv.BakedUntil, Routes: byRoute[c.Name], |
| + }} |
| } else if rts := byRoute[c.Name]; len(rts) > 0 { |
| chans = []LadderChannel{{Channel: c.Name, Routes: rts, |
| Err: "nothing has been published here yet — these addresses are waiting for a version"}} |
| @@ -300,8 +295,12 @@ func gateSentence(rules release.ChannelRules) string { |
| if rules.Passkey { |
| parts = append(parts, "Needs a passkey.") |
| } |
| - if rules.PromoteApprovals > 0 { |
| - parts = append(parts, fmt.Sprintf("Needs %d members to confirm.", rules.PromoteApprovals)) |
| + if n := rules.PromoteApprovals; n > 0 { |
| + member := "members" |
| + if n == 1 { |
| + member = "member" |
| + } |
| + parts = append(parts, fmt.Sprintf("Needs %d %s to confirm.", n, member)) |
| } |
| if len(parts) == 0 { |
| return "Anyone on the team promotes here." |
diff --git a/internal/console/pipelinepage_test.go b/internal/console/pipelinepage_test.go| index 6fdcea77d..fc6e2fa44 100644 |
| --- a/internal/console/pipelinepage_test.go |
| +++ b/internal/console/pipelinepage_test.go |
| @@ -10,11 +10,14 @@ package console |
| // pending-changes card shows real quorum progress with working controls. |
| import ( |
| + "context" |
| "net/http" |
| "net/url" |
| "strings" |
| "testing" |
| "time" |
| + |
| + "github.com/carlosframework/platform/internal/release" |
| ) |
| func TestOneChannelAppHidesChannelVocabulary(t *testing.T) { |
| @@ -67,6 +70,144 @@ func assertNoChannelVocabulary(t *testing.T, body string) { |
| } |
| } |
| +// TestOneChannelAppOverviewHidesFlowDiagram pins the CRITICAL finding from |
| +// the Task 9 review: the OVERVIEW page (/a/{sqid}/apps/{app}, what spec §5 |
| +// calls "the app page") draws its own release-flow diagram |
| +// (app_overview.html's {{template "flow" .}}, flow.go), completely |
| +// independent of the Releases tab's ladder card — and flow.go hardcoded the |
| +// legacy edge/beta/live path unconditionally. A one-channel pipeline must |
| +// hide it exactly as it hides the ladder card, and with real content on the |
| +// entry channel (a fresh, truly empty app never shows the diagram at all, |
| +// legacy or not — Show only goes true once something is actually held or |
| +// addressed — so this only proves the fix with something to leak). |
| +func TestOneChannelAppOverviewHidesFlowDiagram(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-hide-flow") |
| + if status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/new", |
| + map[string]string{"name": "soloflow"}, token); status != http.StatusOK { |
| + t.Fatalf("create app = %d %v", status, out) |
| + } |
| + shipVersion(t, fx, "soloflow", "v1") |
| + seedChannel(t, fx, "soloflow", "edge", "v1") |
| + |
| + c := signInPaul(t, fx) |
| + status, _, body := get(t, c, fx.u("/apps/soloflow")) |
| + if status != http.StatusOK { |
| + t.Fatalf("app overview page = %d", status) |
| + } |
| + assertNoChannelVocabulary(t, body) |
| + if strings.Contains(body, "Promoting is what moves it") { |
| + t.Errorf("one-channel app overview page still draws the flow diagram:\n%s", body) |
| + } |
| +} |
| + |
| +// TestMultiChannelFlowDrawsPipelineChannels pins the other half of the same |
| +// CRITICAL finding: a MULTI-channel pipelined app's flow diagram must draw |
| +// the app's OWN declared channels, in order — not the frozen edge/beta/live |
| +// path, which a pipeline named edge/production simply does not have. |
| +func TestMultiChannelFlowDrawsPipelineChannels(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-flow-multi") |
| + if status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/new", |
| + map[string]string{"name": "multiflow"}, token); status != http.StatusOK { |
| + t.Fatalf("create app = %d %v", status, out) |
| + } |
| + if status, out := putJSON(t, fx.ts.URL+"/api/cli/apps/bab/multiflow/pipeline", |
| + map[string]any{"channels": []map[string]any{ |
| + {"name": "edge"}, |
| + {"name": "production", "bake": "24h"}, |
| + }}, token); status != http.StatusOK { |
| + t.Fatalf("pipeline put = %d %v", status, out) |
| + } |
| + shipVersion(t, fx, "multiflow", "v1") |
| + seedChannel(t, fx, "multiflow", "edge", "v1") |
| + |
| + c := signInPaul(t, fx) |
| + _, _, body := get(t, c, fx.u("/apps/multiflow")) |
| + edgeLabel, prodLabel := `<p class="flow-label">edge</p>`, `<p class="flow-label">production</p>` |
| + if !strings.Contains(body, edgeLabel) { |
| + t.Fatalf("flow diagram does not draw the entry channel:\n%s", body) |
| + } |
| + if !strings.Contains(body, prodLabel) { |
| + t.Fatalf("flow diagram does not draw the app's own second channel:\n%s", body) |
| + } |
| + if strings.Contains(body, `<p class="flow-label">beta</p>`) || strings.Contains(body, `<p class="flow-label">live</p>`) { |
| + t.Fatalf("flow diagram still draws the legacy edge/beta/live path for a pipelined app:\n%s", body) |
| + } |
| + if i, j := strings.Index(body, edgeLabel), strings.Index(body, prodLabel); i < 0 || j < 0 || i > j { |
| + t.Fatalf("channels are not drawn in pipeline order (edge before production):\n%s", body) |
| + } |
| +} |
| + |
| +// TestChannelsTableAndPipelineCardAgreeOnBakeWindow pins IMPORTANT 2 from |
| +// the Task 9 review: the Channels table and the pipeline card read the SAME |
| +// AppView.Channels slice, and used to disagree about how long an |
| +// app-defined channel bakes for — the table read the legacy 72h fallback |
| +// (release.HoldFor(DefaultHolds, "production") — an unrecognized name gets |
| +// the longest standard window, fail closed), the card read the pipeline's |
| +// own declared window via holdWindow. A 1h bake makes the two answers |
| +// impossible to confuse in their rendered form: the correct one is |
| +// minutes-scale ("~59m", no "d"), the legacy-table bug would show a |
| +// multi-day figure ("~2d23h..."). |
| +func TestChannelsTableAndPipelineCardAgreeOnBakeWindow(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-bake-agree") |
| + if status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/new", |
| + map[string]string{"name": "bakeapp"}, token); status != http.StatusOK { |
| + t.Fatalf("create app = %d %v", status, out) |
| + } |
| + if status, out := putJSON(t, fx.ts.URL+"/api/cli/apps/bab/bakeapp/pipeline", |
| + map[string]any{"channels": []map[string]any{ |
| + {"name": "edge"}, |
| + {"name": "production", "bake": "1h"}, |
| + }}, token); status != http.StatusOK { |
| + t.Fatalf("pipeline put = %d %v", status, out) |
| + } |
| + shipVersion(t, fx, "bakeapp", "v1") |
| + seedChannel(t, fx, "bakeapp", "edge", "v1") |
| + seedChannel(t, fx, "bakeapp", "production", "v1") |
| + |
| + c := signInPaul(t, fx) |
| + _, _, body := get(t, c, fx.u("/apps/bakeapp/releases")) |
| + |
| + // The Channels table's own "Goes live" cell for production. |
| + _, after, ok := strings.Cut(body, "<td>production</td>") |
| + if !ok { |
| + t.Fatalf("no Channels-table row for production:\n%s", body) |
| + } |
| + rowEnd := strings.Index(after, "</tr>") |
| + if rowEnd < 0 { |
| + t.Fatalf("malformed Channels table row:\n%s", after) |
| + } |
| + row := after[:rowEnd] |
| + i := strings.Index(row, "in ~") |
| + if i < 0 { |
| + t.Fatalf("Channels table shows no live bake countdown for production:\n%s", row) |
| + } |
| + cell := row[i:] |
| + if k := strings.Index(cell, "<"); k >= 0 { |
| + cell = cell[:k] |
| + } |
| + if strings.Contains(cell, "d") { |
| + t.Errorf("Channels table shows a multi-day bake window for a 1h pipeline channel — it read the legacy 72h table, not the pipeline's own window: %q", cell) |
| + } |
| + |
| + // The pipeline card's own rung for production tells the same story. |
| + prodBlock := rungBlock(t, body, "production") |
| + if !strings.Contains(prodBlock, "goes live ~") { |
| + t.Fatalf("pipeline card shows no live bake countdown for production:\n%s", prodBlock) |
| + } |
| + if i := strings.Index(prodBlock, "goes live ~"); i >= 0 { |
| + line := prodBlock[i:] |
| + if j := strings.Index(line, "<"); j >= 0 { |
| + line = line[:j] |
| + } |
| + if strings.Contains(line, "d") { |
| + t.Errorf("pipeline card shows a multi-day bake window for a 1h pipeline channel:\n%s", line) |
| + } |
| + } |
| +} |
| + |
| func TestPipelineCardAndSettingsTemplates(t *testing.T) { |
| fx := newConsoleFixture(t) |
| token := cliToken(t, fx, "paul@keymail.dev", "code-pipeline-card") |
| @@ -209,3 +350,97 @@ func pendingIDFromBody(t *testing.T, body string) string { |
| } |
| return rest[:j] |
| } |
| + |
| +// TestPipelineSettingsSavePOST is IMPORTANT 5 from the Task 9 review: the |
| +// settings page's own write door had no test. Three properties, in the |
| +// order they'd hurt if lost: |
| +// - requireFreshAuth is enforced even for an ordinary, unguarded edit — |
| +// a plain POST with no step-up redirects into Keymail rather than |
| +// landing anything (postGated walks the round trip that DOES land it). |
| +// - An unguarded save writes the pipeline directly once stepped up. |
| +// - A GUARDED edit (touching a channel the STORED pipeline already |
| +// protects with change_approvals) becomes a pending change instead of |
| +// landing — the same propose flow the API PUT takes, reused rather |
| +// than duplicated (pipelinepage.go's own doc comment). |
| +func TestPipelineSettingsSavePOST(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-settings-post") |
| + if status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/new", |
| + map[string]string{"name": "settle"}, token); status != http.StatusOK { |
| + t.Fatalf("create app = %d %v", status, out) |
| + } |
| + c := signInPaul(t, fx) |
| + csrf := sessionToken(t, fx, c) |
| + |
| + saveForm := url.Values{ |
| + "csrf": {csrf}, "action": {"save"}, |
| + "ch.0.name": {"edge"}, |
| + "ch.1.name": {"production"}, "ch.1.bake": {"24h"}, |
| + } |
| + |
| + // Un-stepped-up: the request is redirected into Keymail, and nothing |
| + // changes yet — the fixture's default pipeline (one channel, "edge") |
| + // is still exactly what it was. |
| + status, loc, _ := postForm(t, c, fx.u("/apps/settle/pipeline"), saveForm) |
| + if status != http.StatusSeeOther || !strings.HasPrefix(loc, fx.fk.srv.URL) { |
| + t.Fatalf("un-stepped-up pipeline save = %d -> %q, want a redirect into Keymail", status, loc) |
| + } |
| + if pipe, err := release.GetPipeline(context.Background(), fx.rel, fx.pub, release.OpsAccount, "settle"); err != nil { |
| + t.Fatal(err) |
| + } else if pipe == nil || len(pipe.Channels) != 1 { |
| + t.Fatalf("pipeline changed before step-up completed: %+v", pipe) |
| + } |
| + |
| + // Completing the step-up carries the unguarded edit straight through. |
| + if status, _, body := completeStepUp(t, fx, c, loc); status != http.StatusSeeOther { |
| + t.Fatalf("resumed pipeline save = %d:\n%s", status, body) |
| + } |
| + pipe, err := release.GetPipeline(context.Background(), fx.rel, fx.pub, release.OpsAccount, "settle") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if pipe == nil || len(pipe.Channels) != 2 || pipe.Channels[1].Name != "production" || pipe.Channels[1].Bake != "24h" { |
| + t.Fatalf("pipeline = %+v, want edge+production(24h)", pipe) |
| + } |
| + |
| + // Guard production with change_approvals: 2 — via the API PUT, standing |
| + // in for a previous, separately-confirmed console edit — so the NEXT |
| + // edit that touches it must propose rather than land. 2, not 1: a |
| + // quorum of 1 is met by the proposer's own step-up alone (spec §4 — it |
| + // degenerates to a passkey rule and executes in the same request), which |
| + // would prove nothing about the propose PATH specifically; a second |
| + // member (sam) makes 2 a real quorum this account can still reach. |
| + if err := fx.st.AddMember(OpsAccountID, "sam@keymail.dev", RoleMember, time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + if status, out := putJSON(t, fx.ts.URL+"/api/cli/apps/bab/settle/pipeline", |
| + map[string]any{"channels": []map[string]any{ |
| + {"name": "edge"}, |
| + {"name": "production", "bake": "24h", "rules": map[string]any{"change_approvals": 2}}, |
| + }}, token); status != http.StatusOK { |
| + t.Fatalf("pipeline put (add change_approvals) = %d %v", status, out) |
| + } |
| + |
| + guardedForm := url.Values{ |
| + "csrf": {csrf}, "action": {"save"}, |
| + "ch.0.name": {"edge"}, |
| + "ch.1.name": {"production"}, "ch.1.bake": {"48h"}, "ch.1.change_approvals": {"2"}, |
| + } |
| + status, _, body := postGated(t, fx, c, fx.u("/apps/settle/pipeline"), guardedForm) |
| + if status != http.StatusSeeOther { |
| + t.Fatalf("guarded save = %d:\n%s", status, body) |
| + } |
| + // Nothing landed — the STORED pipeline still says 24h. |
| + if pipe, err := release.GetPipeline(context.Background(), fx.rel, fx.pub, release.OpsAccount, "settle"); err != nil { |
| + t.Fatal(err) |
| + } else if pipe == nil || pipe.Channels[1].Bake != "24h" { |
| + t.Fatalf("pipeline = %+v, want the guarded edit to have PROPOSED, not landed", pipe) |
| + } |
| + pending, err := fx.st.PendingChangesFor(OpsAccountID, "settle", time.Now()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(pending) != 1 || pending[0].Kind != kindPolicy { |
| + t.Fatalf("pending changes = %+v, want exactly one policy proposal", pending) |
| + } |
| +} |
diff --git a/internal/console/server.go b/internal/console/server.go| index 758caa93a..46f0826ae 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -2096,9 +2096,52 @@ func (s *Server) appPage(w http.ResponseWriter, r *http.Request, tab string) (pa |
| p.ActiveTab = tab |
| p.ActiveNav = "apps" |
| p.Detail = &views[0] |
| + s.applyPipelineView(r.Context(), p.Detail, acct) |
| return p, acct, true |
| } |
| +// applyPipelineView is the one place a single app's view becomes |
| +// pipeline-aware (release pipelines v2 §2, §5) — deliberately not inside |
| +// Assemble (AppView.Pipeline's own doc comment: the account home page calls |
| +// Assemble for a whole app list and never reads Ladder or Pipeline, so a |
| +// pipeline GET per app there would be a bucket read nobody looks at; appPage |
| +// is the one place every app-scoped handler funnels through). |
| +// |
| +// It corrects each ChannelView's BakedUntil from the pipeline's OWN window |
| +// (holdWindow) in place of the legacy per-app table Assemble already |
| +// computed — the fix for the Task 9 review's disagreement between the |
| +// Channels table and the pipeline card, both of which read Channels — and |
| +// then rebuilds Ladder from those corrected channels, so the two can never |
| +// disagree again by construction. Best-effort and view-only, like every |
| +// other read in this family: an unreadable pipeline degrades the render to |
| +// the legacy ladder rather than failing the page. |
| +func (s *Server) applyPipelineView(ctx context.Context, view *AppView, acct *Account) { |
| + ctx, cancel := context.WithTimeout(ctx, pipelineStoreTimeout) |
| + defer cancel() |
| + pipe, err := release.GetPipeline(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, view.App) |
| + if err != nil { |
| + log.Printf("console: app page %s/%s: read pipeline: %v", acct.Sqid, view.App, err) |
| + return |
| + } |
| + if pipe == nil { |
| + return |
| + } |
| + view.Pipeline = pipe |
| + now := time.Now() |
| + for i := range view.Channels { |
| + view.Channels[i].BakedUntil = time.Time{} |
| + if view.Channels[i].Err != "" || view.Channels[i].FastTrack { |
| + continue |
| + } |
| + if d := holdWindow(pipe, view.Channels[i].Name); d > 0 { |
| + if until := view.Channels[i].PromotedAt.Add(d); now.Before(until) { |
| + view.Channels[i].BakedUntil = until |
| + } |
| + } |
| + } |
| + view.Ladder = assembleLadder(view.Channels, view.Routes, pipe, now) |
| +} |
| + |
| // appResourceMetrics sweeps the app's own instances only. The allowlist is |
| // built HERE, from the routes this account's app owns and nothing else — |
| // that slice is the entire tenant boundary on the metrics read, so it must |
| @@ -2882,6 +2925,26 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { |
| } |
| ctx, cancel := context.WithTimeout(r.Context(), 15*time.Second) |
| defer cancel() |
| + // Which gate this rollback meets is the pipeline's answer, so it is read |
| + // before the confirm card — the same reasoning, and the same order, as |
| + // handlePromote. |
| + pipe, perr := release.GetPipeline(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app) |
| + if perr != nil { |
| + log.Printf("console: rollback %s/%s: read pipeline: %v", acct.Sqid, app, perr) |
| + back("err", "Could not read the pipeline for "+app+". Nothing has been changed.") |
| + return |
| + } |
| + // The stable rung's owner gate fires BEFORE the confirm card (Task 9 |
| + // review, IMPORTANT 1 — mirroring handlePromote's own comment on this |
| + // exact point): a non-owner member probing a legacy stable rollback must |
| + // meet the byte-identical 404, never a card that concedes the action |
| + // would otherwise proceed. Legacy path only — a PIPELINED app's channel |
| + // named "stable" is governed by its own declared rules below, not this |
| + // rung table (pre-existing gap fix, Task 8/9 review). |
| + if rung, _ := release.Rung(channel); pipe == nil && rung == release.RungStable && role != RoleOwner { |
| + http.NotFound(w, r) |
| + return |
| + } |
| cur, err := release.Channel(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app, channel) |
| if err != nil { |
| back("err", "Could not read channel "+channel+".") |
| @@ -2910,12 +2973,6 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { |
| // channel's rules exist to govern. requireFreshAuth stays LAST, after the |
| // confirm card and the pin — and a channel with no rules, or an app with |
| // no pipeline, behaves exactly as it did. |
| - pipe, perr := release.GetPipeline(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app) |
| - if perr != nil { |
| - log.Printf("console: rollback %s/%s: read pipeline: %v", acct.Sqid, app, perr) |
| - back("err", "Could not read the pipeline for "+app+". Nothing has been changed.") |
| - return |
| - } |
| if rules := channelRules(pipe, channel); rules.Passkey || rules.PromoteApprovals >= 1 { |
| if !s.requireFreshAuth(w, r, addr, raw) { |
| return |
| @@ -2940,13 +2997,14 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { |
| // owner+passkey for a PROMOTION onto it and the API door 403s a |
| // rollback of it outright — a rollback lands the same pointer write |
| // in front of the same users. Mirror promoteGate's legacy-path shape |
| - // exactly: byte-identical 404 for a non-owner, requireFreshAuth last. |
| - // Every other legacy rung is unchanged — membership alone, as before. |
| + // exactly: byte-identical 404 for a non-owner (already refused |
| + // above, before the confirm card), requireFreshAuth last here — the |
| + // re-check below is deliberate duplication of the owner condition |
| + // (handlePromote's own comment: the confirm step must sit strictly |
| + // between owner 404 and requireFreshAuth without weakening either), |
| + // not a second gate. Every other legacy rung is unchanged — |
| + // membership alone, as before. |
| if rung, _ := release.Rung(channel); rung == release.RungStable { |
| - if role != RoleOwner { |
| - http.NotFound(w, r) |
| - return |
| - } |
| if !s.requireFreshAuth(w, r, addr, raw) { |
| return |
| } |
diff --git a/internal/console/server_test.go b/internal/console/server_test.go| index 0813f6b7e..bb10877bd 100644 |
| --- a/internal/console/server_test.go |
| +++ b/internal/console/server_test.go |
| @@ -983,17 +983,32 @@ func TestLegacyStableRollbackGatedByOwnerAndStepUp(t *testing.T) { |
| c := signInPaul(t, fx) |
| csrf := sessionToken(t, fx, c) |
| - // A member: the byte-identical 404, same as a promotion onto stable. |
| + // A member: the byte-identical 404, same as a promotion onto stable — |
| + // and never the confirm card first (IMPORTANT 1, Task 9 review): the |
| + // owner gate must fire BEFORE confirmRollback, not after a card that |
| + // already conceded the action would otherwise proceed. |
| fx.setUser("sam@keymail.dev") |
| cm := newBrowser(t) |
| signUp(t, fx, cm, "code-sam-rollback-stable") |
| + csrfSam := sessionToken(t, fx, cm) |
| + if status, _, body := postForm(t, cm, fx.u("/apps/kass/rollback"), |
| + url.Values{"csrf": {csrfSam}, "channel": {"stable"}}); status != http.StatusNotFound { |
| + t.Fatalf("member stable rollback (unconfirmed) = %d, want 404", status) |
| + } else if body != "404 page not found\n" { |
| + t.Fatalf("member stable rollback body = %q, want the stdlib http.NotFound output — not a confirm card", body) |
| + } |
| if status, _, _ := postForm(t, cm, fx.u("/apps/kass/rollback"), |
| - url.Values{"csrf": {sessionToken(t, fx, cm)}, "channel": {"stable"}, "confirmed": {"1"}}); status != http.StatusNotFound { |
| - t.Fatalf("member stable rollback = %d, want 404", status) |
| + url.Values{"csrf": {csrfSam}, "channel": {"stable"}, "confirmed": {"1"}}); status != http.StatusNotFound { |
| + t.Fatalf("member stable rollback (confirmed) = %d, want 404", status) |
| } |
| if p := fx.channelVersion(t, "kass", "stable"); p.Version != "v2" { |
| t.Fatalf("stable moved to %s on a refused member rollback", p.Version) |
| } |
| + // The button itself is hidden from a member on the releases page — not |
| + // merely refused on click. |
| + if _, _, mbody := get(t, cm, fx.u("/apps/kass/releases")); strings.Contains(mbody, `name="channel" value="stable"`) { |
| + t.Fatalf("member sees a Roll back control for stable, want it hidden:\n%s", mbody) |
| + } |
| // The owner: a plain POST with no step-up does not move the pointer — it |
| // is redirected into Keymail's re-authentication, same as a stable |
diff --git a/internal/console/templates/app_overview.html b/internal/console/templates/app_overview.html| index 5cbf9123a..07a415522 100644 |
| --- a/internal/console/templates/app_overview.html |
| +++ b/internal/console/templates/app_overview.html |
| @@ -67,6 +67,12 @@ |
| </a> |
| </section> |
| {{end}} |
| +{{/* The legacy safety-delay flag: legacy-path-only, exactly as ladder.html |
| + skips this same block in pipeline mode. A pipelined app's bake story is |
| + its own per-channel windows (spec §3) — this global on/off line would |
| + dangle or actively mislead once ladder.html stopped offering the |
| + control it points at. */}} |
| +{{if not .Detail.Pipeline}} |
| {{with .Detail.Production}} |
| {{if .Err}} |
| <p class="small error">Safety delay: this app's setting can't be read right now, so the delay stays on. Repair it on the <a href="{{$.Nav}}/apps/{{$.AppName}}/releases">Versions</a> tab.</p> |
| @@ -78,6 +84,7 @@ |
| <p class="small">Safety delay: off — new versions go live as soon as they're promoted.{{if .Nudge}} {{icon "triangle-alert"}} This app looks like it's serving real users — consider turning it on, on <a href="{{$.Nav}}/apps/{{$.AppName}}/releases">Versions</a>.{{end}}</p> |
| {{end}} |
| {{end}} |
| +{{end}} |
| {{template "pending" .}} |
| {{if .Events}} |
| <h2>Recent activity</h2> |
diff --git a/internal/console/templates/app_releases.html b/internal/console/templates/app_releases.html| index eb6cbbb4c..4078cfb31 100644 |
| --- a/internal/console/templates/app_releases.html |
| +++ b/internal/console/templates/app_releases.html |
| @@ -116,7 +116,7 @@ carlos ship -app {{.Detail.App}} -kind static -version v1 <site-dir></code |
| <td><span title="{{fmtTime .PromotedAt}}">{{ago .PromotedAt}}</span></td> |
| <td>{{if .BakedUntil.IsZero}}live now{{else}}in {{until .BakedUntil}}{{end}}{{if .FastTrack}} · delay skipped{{end}}</td> |
| {{end}} |
| - <td>{{if and $.CanPromote .HasHistory (not .Err)}} |
| + <td>{{if and $.CanPromote .HasHistory (not .Err) (or $.Detail.Pipeline $.IsOwner (ne .Name "stable"))}} |
| <form method="post" action="{{$.Base}}/apps/{{$.Detail.App}}/rollback"> |
| <input type="hidden" name="csrf" value="{{$.CSRF}}"> |
| <input type="hidden" name="channel" value="{{.Name}}"> |
diff --git a/internal/console/templates/pending.html b/internal/console/templates/pending.html| index 28eff25ca..4074cecec 100644 |
| --- a/internal/console/templates/pending.html |
| +++ b/internal/console/templates/pending.html |
| @@ -6,7 +6,7 @@ withdraw it. A change nobody acts on expires on its own after 7 days.</p> |
| <ul class="feed"> |
| {{range .Pending}} |
| <li> |
| - <p>{{.Summary}} <span class="small">— {{.Confirmed}}/{{.Needed}} confirmed</span></p> |
| + <p><span class="chip chip-env">{{.Kind}}</span> {{.Summary}} <span class="small">— {{.Confirmed}}/{{.Needed}} confirmed</span></p> |
| <form method="post" action="{{$.Base}}/apps/{{$.AppName}}/pending/{{.ID}}/confirm" class="inline"> |
| <input type="hidden" name="csrf" value="{{$.CSRF}}"> |
| <button type="submit" class="btn-sm">Confirm</button> |
diff --git a/internal/console/templates/pipeline.html b/internal/console/templates/pipeline.html| index e45c45329..1fd9f6a38 100644 |
| --- a/internal/console/templates/pipeline.html |
| +++ b/internal/console/templates/pipeline.html |
| @@ -26,6 +26,10 @@ from scratch with the row below. Nothing is written until you press Save.</p> |
| <form method="post" action="{{.Base}}/apps/{{.AppName}}/pipeline"> |
| <input type="hidden" name="csrf" value="{{.CSRF}}"> |
| + {{/* First submit button in tree order, so pressing Enter in a row's text |
| + field submits Save — implicit form submission uses the first submit |
| + button, and without this a row's own "Remove" button would be it. */}} |
| + <button type="submit" name="action" value="save" class="visually-hidden" tabindex="-1" aria-hidden="true"></button> |
| <div class="tablewrap"> |
| <table> |
| <tr><th>Channel</th><th>Bake</th><th>Passkey</th><th>Promote approvals</th><th>Change approvals</th><th></th></tr> |
diff --git a/internal/console/view.go b/internal/console/view.go| index f3f7717da..2957ee65a 100644 |
| --- a/internal/console/view.go |
| +++ b/internal/console/view.go |
| @@ -53,6 +53,17 @@ type AppView struct { |
| // degrades to the legacy ladder rather than failing outright; a WRITE |
| // door such as handlePromote reads its own copy and refuses on error |
| // instead, which is the place that distinction is load-bearing). |
| + // |
| + // Deliberately NOT populated by Assemble (view.go's own tagByRelease |
| + // precedent: VersionView.Tag is never read there either — every caller |
| + // that wants it joins separately). Assemble builds a whole account's app |
| + // list for the account home page, which never renders Ladder or |
| + // Pipeline at all; a pipeline read per app there would be a bucket GET |
| + // nobody looks at. appPage (server.go) is the one place every APP page |
| + // funnels through, so it is the one place that reads this, and rebuilds |
| + // Ladder — and corrects each ChannelView's BakedUntil, which the legacy |
| + // per-app table below computes and which means nothing for a pipelined |
| + // channel's own window (Task 9 review) — once it has one. |
| Pipeline *release.Pipeline |
| } |
| @@ -460,16 +471,6 @@ func Assemble(regPath string, s release.Store, pub ed25519.PublicKey, acct strin |
| } |
| seen[app] = true |
| channels := assembleChannels(ctx, s, pub, acct, app, now) |
| - // Best-effort, view-only (see AppView.Pipeline's doc comment): an |
| - // unreadable pipeline degrades this render to the legacy ladder |
| - // rather than failing the whole page, matching every other |
| - // best-effort read in this loop (ensureAccountManifest's sibling |
| - // posture, assembleProduction's own Err-carrying fallback). |
| - pipe, perr := release.GetPipeline(ctx, s, pub, acct, app) |
| - if perr != nil { |
| - log.Printf("console: assemble %s/%s: read pipeline: %v", acct, app, perr) |
| - pipe = nil |
| - } |
| mm := mismatchedByApp[app] |
| mismatch := "" |
| if len(mm) > 0 { |
| @@ -489,17 +490,21 @@ func Assemble(regPath string, s release.Store, pub ed25519.PublicKey, acct strin |
| } |
| } |
| out = append(out, AppView{ |
| - App: app, |
| - Routes: byApp[app], // List orders by host already |
| - Domains: domainsByApp[app], |
| - Channels: channels, |
| - Canaries: assembleCanaries(channels, byApp[app]), |
| - Ladder: assembleLadder(channels, byApp[app], pipe, now), |
| + App: app, |
| + Routes: byApp[app], // List orders by host already |
| + Domains: domainsByApp[app], |
| + Channels: channels, |
| + Canaries: assembleCanaries(channels, byApp[app]), |
| + // nil pipeline here, always: this Ladder is the legacy shape. |
| + // Pipeline itself is deliberately NOT read here — see |
| + // AppView.Pipeline's doc comment — so appPage (server.go) is |
| + // where a single app's view gets pipeline-aware, and rebuilds |
| + // Ladder from the corrected Channels once it has one. |
| + Ladder: assembleLadder(channels, byApp[app], nil, now), |
| Production: pv, |
| RouteAccount: mismatch, |
| Mismatched: mm, |
| AllMismatched: len(mm) > 0 && len(mm) == routableByApp[app], |
| - Pipeline: pipe, |
| }) |
| } |
| return out, nil |