amadan
Merge remote-tracking branch 'origin/main' into custom-domains-managed
26 files changed,
+2189
−180
docs/superpowers/specs/2026-08-19-fleet-architecture-diagram-design.md+27 −13internal/console/apicli_test.go+36 −0internal/console/archgraph.go+50 −13internal/console/archgraph_test.go+28 −30internal/console/archlayout.go+102 −15internal/console/archlayout_test.go+17 −19internal/console/archlegibility_test.go+149 −0internal/console/pending.go+52 −3internal/console/pending_test.go+69 −0internal/console/pendingflow_test.go+286 −0internal/console/pendinghandlers.go+180 −20internal/console/pipelinepage.go+14 −2internal/console/pipelinepage_test.go+114 −4internal/console/server.go+4 −2internal/console/templates/app_releases.html+48 −13internal/console/templates/layout.html+5 −0internal/console/view.go+29 −0internal/mail/walk.go+96 −3internal/mail/walk_enable_test.go+84 −0internal/release/adopt.go+85 −23internal/release/adopt_test.go+378 −0internal/release/bake.go+141 −7internal/release/bake_test.go+90 −5internal/release/pipeline_test.go+80 −0internal/release/release.go+23 −7mail_cli.go+2 −1
CI passed — run details
diff --git a/docs/superpowers/specs/2026-08-19-fleet-architecture-diagram-design.md b/docs/superpowers/specs/2026-08-19-fleet-architecture-diagram-design.md| index 0598d14c5..81ce6b4a3 100644 |
| --- a/docs/superpowers/specs/2026-08-19-fleet-architecture-diagram-design.md |
| +++ b/docs/superpowers/specs/2026-08-19-fleet-architecture-diagram-design.md |
| @@ -229,7 +229,12 @@ three inputs and owns every join: |
| box label (falling back to the `""` default spec, exactly as |
| `AllocateBox` does), priced `HourUSD × 730 + EBSGB × EBSGBMonthUSD`. |
| The discovered `machine.type` and summed `disk.size_gb` are shown |
| - beside it. |
| + beside it. A box with **no cloud machine behind it** — a customer's |
| + fleet box, a Pi on a desk — is not priced at all (amended 2026-08-27): |
| + the rate card prices what this platform pays for, so a monthly figure |
| + there would be a claim about someone else's bill, and a "priced by the |
| + default spec" chip would warn about a pricing that should not be |
| + happening. The card still says plainly that there is no cloud machine. |
| - **box → bucket**: every box that reports edges to the deployment bucket |
| the console itself is configured with (`s.cfg.Releases`) — the one |
| bucket relationship the console can state as fact. Per-box replication |
| @@ -259,18 +264,27 @@ A deterministic layered pass in Go, no JavaScript library, no CDN |
| (the vendored-icons rule): |
| 1. **Layer by kind** — left to right: `zone`, `record`, `ip`, |
| - `router`/`external`/`alarm`, `box`/`machine`, `apps`, `bucket`. Of the |
| - satellites, `bucket` and `alarm` are DRAWN and `identity` is not: |
| - `alarm` sits in the column immediately left of the box or machine it |
| - watches — few enough per fleet (one auto-recover alarm per box) that an |
| - armed alarm beside its box is worth the ink, and left of it so the |
| - `watches` edge reads in the same direction as every other edge. |
| - `identity`, and likewise `firewall`, `network`, `disk` and |
| - `certificate`, are listed in the resource tables below the diagram |
| - instead: a graph that draws every IAM role is a hairball. "Anchored to |
| - its owner" in a layered layout means ordered by the row of the thing it |
| - watches, which is what the barycentre sweep in (2) does — not pinned to |
| - the same Y. |
| + `router`/`external`, `box`/`machine`, `apps`, `bucket`. `bucket` is the |
| + one satellite drawn as a node; `identity`, `firewall`, `network`, |
| + `disk` and `certificate` are listed in the resource tables below the |
| + diagram instead, because a graph that draws every IAM role is a |
| + hairball. |
| + |
| + **Alarms are drawn ON the node they watch, not as nodes of their own** |
| + (amended 2026-08-27, after seeing the page rendered). They were first |
| + given the column immediately left of the boxes — the only gap in the |
| + flow — and that column is the one every IP-to-box edge crosses. Nodes |
| + paint after edges, so each alarm covered the line running beneath it |
| + and the picture read as a request reaching its box *through* an alarm. |
| + The deeper problem was the modelling: an alarm is not a hop a request |
| + takes, it is a property of the thing it watches. Each alarm is now a |
| + mark on its subject's node, coloured when firing, with its name and |
| + state in the node's hover title — which is what "anchored to its owner" |
| + should have meant all along, and which no layer can put in the way of a |
| + line. The Alarms table still lists every one in full. |
| + |
| + The invariant this cost us is now enforced directly: **no node may sit |
| + on an edge's path** unless it is one of that edge's endpoints. |
| 2. **Order within a layer** to reduce crossings: one barycentre sweep |
| down and one up, then a stable sort by name so the same fleet always |
| produces the same picture. A diagram that reshuffles between refreshes |
diff --git a/internal/console/apicli_test.go b/internal/console/apicli_test.go| index 145074d34..d005c9594 100644 |
| --- a/internal/console/apicli_test.go |
| +++ b/internal/console/apicli_test.go |
| @@ -597,6 +597,42 @@ func TestAPIPromoteHappy(t *testing.T) { |
| } |
| } |
| +// TestAPIPromotePipelinedChannelNamesItselfInTo: the promote door's "to" |
| +// field is PromoteResult.To, which was the target channel's RUNG — and an |
| +// app-defined channel is not a rung, so a pipelined promotion answered with |
| +// "to": "". The CLI does not print it (it prints what it asked for), but the |
| +// field is the API's own record of where the build went, and "" is a worse |
| +// answer than the channel's name for every other client. Wire-additive: a |
| +// fuller string where an empty one used to sit. |
| +func TestAPIPromotePipelinedChannelNamesItselfInTo(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-promote-pipelined") |
| + if status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/new", |
| + map[string]string{"name": "piped"}, token); status != http.StatusOK { |
| + t.Fatalf("create app = %d %v", status, out) |
| + } |
| + if status, out := putJSON(t, fx.ts.URL+"/api/cli/apps/bab/piped/pipeline", |
| + map[string]any{"channels": []map[string]any{{"name": "edge"}, {"name": "production"}}}, token); status != http.StatusOK { |
| + t.Fatalf("pipeline put = %d %v", status, out) |
| + } |
| + shipVersion(t, fx, "piped", "abc1234") |
| + if status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/piped/promote", |
| + map[string]string{"version": "abc1234", "channel": "edge"}, token); status != http.StatusOK { |
| + t.Fatalf("promote to edge = %d %v", status, out) |
| + } |
| + status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/piped/promote", |
| + map[string]string{"version": "abc1234", "channel": "production"}, token) |
| + if status != http.StatusOK { |
| + t.Fatalf("promote to production = %d %v", status, out) |
| + } |
| + if got := str(t, out, "to"); got != "production" { |
| + t.Errorf("promote body to = %q, want %q: %v", got, "production", out) |
| + } |
| + if got := str(t, out, "from"); got != "edge" { |
| + t.Errorf("promote body from = %q, want %q: %v", got, "edge", out) |
| + } |
| +} |
| + |
| // TestAPIPromoteStableRefused: stable cannot be reached over the API at all — |
| // there is no passkey to step up to — and the refusal is byte-identical for a |
| // member token and an owner token, so a token holder learns nothing about |
diff --git a/internal/console/archgraph.go b/internal/console/archgraph.go| index 64fdd186e..754921af5 100644 |
| --- a/internal/console/archgraph.go |
| +++ b/internal/console/archgraph.go |
| @@ -64,7 +64,25 @@ type KV struct{ K, V string } |
| type Node struct { |
| ID, Kind, Title, Subtitle string |
| Detail []KV |
| - X, Y, W, H int |
| + // Alarms are the alarms watching THIS node's subject. They ride the |
| + // node rather than standing as nodes of their own, for two reasons an |
| + // earlier draft learned the hard way: a satellite column placed |
| + // between the IP and the box sits underneath every IP-to-box edge, and |
| + // nodes paint after edges, so the line vanished behind it and the |
| + // picture read as a request flowing THROUGH the alarm; and an alarm is |
| + // not a hop a request takes, it is a property of the thing it watches. |
| + // The Alarms resource table still lists every one of them in full. |
| + Alarms []NodeAlarm |
| + X, Y, W, H int |
| +} |
| + |
| +// NodeAlarm is one alarm as its subject wears it: enough to say which |
| +// alarm and whether it is firing, and nothing else — the table below the |
| +// diagram carries the metric, the region and the ARN. |
| +type NodeAlarm struct { |
| + Name string |
| + State string |
| + Firing bool |
| } |
| // Edge is one relationship the console can state as fact. Kind is the |
| @@ -152,7 +170,7 @@ func BuildArch(in ArchInput) ArchView { |
| addDNSNodes(&v, in.Inv, claimed) |
| addBoxNodes(&v, in) |
| addMachineNodes(&v, in.Inv, claimed) |
| - addAlarmNodes(&v, in.Inv, claimed) |
| + addAlarmBadges(&v, in.Inv, claimed) |
| v.Groups = resourceGroups(in.Inv) |
| // AFTER layout, not before: layout is what makes the node set final (it |
| // drops the satellite kinds the diagram lists in tables instead), so an |
| @@ -353,10 +371,19 @@ func buildBoxCards(in ArchInput) []BoxCard { |
| card.DiscoveredDiskGB = diskGBFor(in.Inv, m.ID) |
| } |
| + // A box with no cloud machine behind it is somebody else's |
| + // hardware — a customer's fleet box, the Pi on a desk. The rate |
| + // card prices what THIS platform pays for, so putting a monthly |
| + // figure on one of those is a claim about a bill we do not pay, |
| + // and "priced by the default spec" is a warning about a pricing |
| + // that should not be happening at all. Both are withheld; the |
| + // card still says plainly that there is no cloud machine. |
| usd, spec, explicit := boxCost(in.Rates, label) |
| - card.USDMonth = usd |
| - card.Cores, card.MemGB, card.EBSGB = spec.Cores, spec.MemGB, spec.EBSGB |
| - card.Drift = driftFor(card, spec, explicit) |
| + if card.CloudID != "" { |
| + card.USDMonth = usd |
| + card.Cores, card.MemGB, card.EBSGB = spec.Cores, spec.MemGB, spec.EBSGB |
| + card.Drift = driftFor(card, spec, explicit) |
| + } |
| cards = append(cards, card) |
| } |
| return cards |
| @@ -574,24 +601,34 @@ func addMachineNodes(v *ArchView, inv *infraview.Inventory, claimed map[string]s |
| } |
| } |
| -// addAlarmNodes hangs each alarm off whatever it watches — the box when a |
| +// addAlarmBadges hangs each alarm on whatever it watches — the box when a |
| // box reports that machine, the bare machine otherwise. An alarm naming |
| -// nothing this page draws is still listed in the resource groups. |
| -func addAlarmNodes(v *ArchView, inv *infraview.Inventory, claimed map[string]string) { |
| +// nothing this page draws is still listed in the resource groups, and one |
| +// naming something the page DOES draw becomes a mark on that node rather |
| +// than a node in the flow (see Node.Alarms for why). |
| +func addAlarmBadges(v *ArchView, inv *infraview.Inventory, claimed map[string]string) { |
| if inv == nil { |
| return |
| } |
| + at := map[string]int{} |
| + for i := range v.Nodes { |
| + at[v.Nodes[i].ID] = i |
| + } |
| for _, a := range inv.ByKind(infraview.KindAlarm) { |
| to := targetOf(a.Attr("target"), claimed) |
| if to == "" { |
| continue |
| } |
| - id := "alarm:" + a.ID |
| - v.Nodes = append(v.Nodes, Node{ |
| - ID: id, Kind: "alarm", Title: firstNonEmpty(a.Name, a.ID), |
| - Subtitle: strings.TrimSpace(a.Attr("metric") + " " + a.Attr("state")), |
| + i, drawn := at[to] |
| + if !drawn { |
| + continue |
| + } |
| + state := a.Attr("state") |
| + v.Nodes[i].Alarms = append(v.Nodes[i].Alarms, NodeAlarm{ |
| + Name: a.Name, |
| + State: state, |
| + Firing: state == "ALARM", |
| }) |
| - v.Edges = append(v.Edges, Edge{From: id, To: to, Kind: "watches"}) |
| } |
| } |
diff --git a/internal/console/archgraph_test.go b/internal/console/archgraph_test.go| index f127d9015..56a90122a 100644 |
| --- a/internal/console/archgraph_test.go |
| +++ b/internal/console/archgraph_test.go |
| @@ -189,16 +189,29 @@ func TestArchCostAndDrift(t *testing.T) { |
| t.Error("a 30 GB rate-card entry against a 40 GB discovered disk must raise drift") |
| } |
| - // A reporting box with no rate-card entry of its own is also drift: |
| - // it is being priced by the default spec, which is a guess. |
| - var pi BoxCard |
| - for _, b := range v.Boxes { |
| - if b.Label == "pi-dublin" { |
| - pi = b |
| + // A box WE pay for that has no rate-card entry of its own is also |
| + // drift: it is being priced by the default spec, which is a guess. |
| + // Machine-backed on purpose — a bring-your-own box is not priced at |
| + // all, so it has no pricing to be wrong about (TestByoBoxIsNotPriced). |
| + noEntry := archFixture() |
| + delete(noEntry.Rates.Boxes, "carlos-eu-west-1") |
| + var defaulted BoxCard |
| + for _, b := range BuildArch(noEntry).Boxes { |
| + if b.Label == "carlos-eu-west-1" { |
| + defaulted = b |
| + } |
| + } |
| + if defaulted.CloudID == "" { |
| + t.Fatal("fixture drift: carlos-eu-west-1 must be machine-backed for this case") |
| + } |
| + var said bool |
| + for _, d := range defaulted.Drift { |
| + if strings.Contains(d, "default spec") { |
| + said = true |
| } |
| } |
| - if len(pi.Drift) == 0 { |
| - t.Error("a box priced by the default spec must say so") |
| + if !said { |
| + t.Errorf("a box we pay for, priced by the default spec, must say so: %v", defaulted.Drift) |
| } |
| } |
| @@ -379,36 +392,21 @@ func TestArchDrawsNoEdgeToAnUndrawnNode(t *testing.T) { |
| } |
| } |
| -// An alarm watching a machine is an edge to the BOX running on it — the |
| -// operator's question is "is this box watched?", not "is i-123 watched?". |
| -// |
| -// Asserted on the RENDERED page, not merely on the join: an armed |
| -// auto-recover alarm beside the box it watches is the operator-facing fact |
| -// this page exists to surface, and a build that produced the edge while the |
| -// layout dropped the node would satisfy a join-only assertion and show the |
| -// operator nothing. |
| -func TestArchAlarmWatchesItsBox(t *testing.T) { |
| +// Every edge on a built view must reach the page. RenderArchSVG skips an |
| +// edge whose endpoints it cannot place, and that skip is the second net, |
| +// not the mechanism: BuildArch prunes dangling edges after layout, so by |
| +// the time the renderer runs there is nothing left for the skip to catch. |
| +// A build that quietly lost an edge to the skip would otherwise look fine. |
| +func TestEveryEdgeReachesThePage(t *testing.T) { |
| in := archFixture() |
| in.Inv.Resources = append(in.Inv.Resources, infraview.Resource{ |
| Kind: infraview.KindAlarm, ID: "auto-recover-i-123", Name: "auto-recover", |
| Attrs: map[string]string{"target": "i-123", "state": "OK", "metric": "StatusCheckFailed"}, |
| }) |
| v := BuildArch(in) |
| - if !hasEdge(v, "alarm:auto-recover-i-123", "box:carlos-eu-west-1") { |
| - t.Errorf("an alarm naming a box's machine must edge to that box: %+v", v.Edges) |
| - } |
| - if !hasNode(v, "alarm:auto-recover-i-123") { |
| - t.Fatalf("the alarm must survive layout and be drawn: %+v", v.Nodes) |
| - } |
| out := string(RenderArchSVG(v)) |
| - if !strings.Contains(out, "archnode-alarm") || !strings.Contains(out, "auto-recover") { |
| - t.Errorf("the alarm never reaches the SVG:\n%s", out) |
| - } |
| - // RenderArchSVG skips an edge whose endpoints it cannot place. That skip |
| - // is the second net, not the mechanism: every edge on a built view must |
| - // reach the page. |
| if got := strings.Count(out, `<path class="archedge"`); got != len(v.Edges) { |
| - t.Errorf("%d edge paths drawn for %d edges — the renderer is silently dropping edges", got, len(v.Edges)) |
| + t.Errorf("page draws %d edges, view carries %d", got, len(v.Edges)) |
| } |
| } |
diff --git a/internal/console/archlayout.go b/internal/console/archlayout.go| index 860e441f4..1bc88f76c 100644 |
| --- a/internal/console/archlayout.go |
| +++ b/internal/console/archlayout.go |
| @@ -21,16 +21,15 @@ import ( |
| // so the layer is otherwise empty, and an address leaving the estate must |
| // sit to the RIGHT of the record that names it, not in the zone column. |
| // |
| -// "alarm" is the one satellite that is DRAWN (spec §6.4). It shares that |
| -// same layer 3, which puts it in the column immediately LEFT of the boxes |
| -// and machines: its `watches` edge then reads left-to-right like every |
| -// other edge on the page, it costs no extra width because that column is |
| -// reserved anyway, and the barycentre sweep below orders it by the row of |
| -// the thing it watches — which is what "anchored to its owner" amounts to |
| -// in a layered layout. Non-overlap is structural: every node in a layer |
| -// gets its own row index. |
| +// Alarms are deliberately absent from this map. They were briefly given |
| +// layer 3 — the column between the IPs and the boxes — and that column is |
| +// the one every IP-to-box edge crosses. Nodes paint after edges, so each |
| +// alarm covered the line running under it and the diagram read as a |
| +// request reaching its box THROUGH an alarm. An alarm is not a hop; it is |
| +// a property of the thing it watches, so it now rides that node as a mark |
| +// (Node.Alarms) and no layer can put it in the way of a line. |
| var archLayer = map[string]int{ |
| - "zone": 0, "record": 1, "ip": 2, "router": 3, "external": 3, "alarm": 3, |
| + "zone": 0, "record": 1, "ip": 2, "router": 3, "external": 3, |
| "box": 4, "machine": 4, "apps": 5, "bucket": 6, |
| } |
| @@ -47,10 +46,8 @@ func layout(v *ArchView) { |
| // Identities, firewalls, networks, disks and certificates are listed in |
| // the tables below the diagram rather than drawn: a graph that draws |
| // every IAM role is a hairball, and the resource groups already carry |
| - // them in full. Alarms are the exception and are drawn — they are few |
| - // (one auto-recover alarm per box on this fleet) and an armed alarm |
| - // beside the box it watches is exactly the operator-facing fact this |
| - // page exists to surface. |
| + // them in full. Alarms are not in this list because they are no longer |
| + // nodes at all — they ride the node they watch (see archLayer above). |
| kept := v.Nodes[:0] |
| for _, n := range v.Nodes { |
| if strings.Contains(satelliteKinds, n.Kind) { |
| @@ -189,6 +186,79 @@ func layout(v *ArchView) { |
| // viewport is narrower — legible everywhere, at the cost of a sideways |
| // scroll on the widest graphs, which is the right trade for a diagram whose |
| // labels are the content. |
| +// RenderArchSVG emits the diagram. Every string that came from a hostname, |
| +// a tag or a label goes through template.HTMLEscapeString — these are |
| +// operator-supplied values from a registry and a cloud account, and the one |
| +// thing this page must never do is execute one of them. |
| +// |
| +// width/height are emitted ALONGSIDE the viewBox, and they are what make the |
| +// picture readable. An <svg> carrying only a viewBox has no intrinsic size, |
| +// so a browser sizes it at 100% of its container and scales the drawing to |
| +// fit: a full estate is 1918 units wide and the console's main column is |
| +// ~1024px, which renders 13px node titles at ~7px and no scrollbar ever |
| +// appears, because the SVG never overflows anything. With an intrinsic size |
| +// the drawing renders 1:1 and .archwrap (overflow-x: auto) scrolls when the |
| +// viewport is narrower — legible everywhere, at the cost of a sideways |
| +// scroll on the widest graphs, which is the right trade for a diagram whose |
| +// labels are the content. |
| +// The text-fitting budget. There are no font metrics in a server-rendered |
| +// SVG, so the width of a string is estimated: for the ui-sans stack these |
| +// nodes use, an average glyph runs a little over half its point size, and |
| +// 0.58 is the ratio that keeps the longest real strings (a five-app list, |
| +// a full instance-count line) inside the box without cutting short ones |
| +// that would have fit. It is deliberately a little pessimistic — a string |
| +// cut one character early costs nothing, a string one character too long |
| +// bleeds across the neighbouring node. |
| +const ( |
| + titlePx = 13 |
| + subPx = 11 |
| + glyphRatio = 0.58 |
| + textPad = 12 |
| + alarmR = 5 |
| + alarmInset = 14 |
| + alarmStep = 14 |
| +) |
| + |
| +// fitText cuts s to the pixels available, appending an ellipsis when it |
| +// has to. The full string is never lost — RenderArchSVG puts it in the |
| +// node's hover title. |
| +func fitText(s string, px, avail int) string { |
| + if avail <= 0 || s == "" { |
| + return "" |
| + } |
| + max := int(float64(avail) / (float64(px) * glyphRatio)) |
| + r := []rune(s) |
| + if max < 1 { |
| + return "…" |
| + } |
| + if len(r) <= max { |
| + return s |
| + } |
| + return string(r[:max-1]) + "…" |
| +} |
| + |
| +// alarmRoom is the width the alarm marks occupy on the title line, so a |
| +// title stops before running underneath them. |
| +func alarmRoom(n Node) int { |
| + if len(n.Alarms) == 0 { |
| + return 0 |
| + } |
| + return len(n.Alarms)*alarmStep + alarmR |
| +} |
| + |
| +// nodeTitleText is the node in full for the hover title: what the boxes |
| +// say, plus every alarm riding it, none of it truncated. |
| +func nodeTitleText(n Node) string { |
| + out := n.Title |
| + if n.Subtitle != "" { |
| + out += " — " + n.Subtitle |
| + } |
| + for _, a := range n.Alarms { |
| + out += " · " + a.Name + " " + a.State |
| + } |
| + return out |
| +} |
| + |
| func RenderArchSVG(v ArchView) template.HTML { |
| var b strings.Builder |
| fmt.Fprintf(&b, `<svg class="archsvg" width="%d" height="%d" viewBox="0 0 %d %d" role="img" aria-label="Deployment architecture">`, |
| @@ -215,12 +285,29 @@ func RenderArchSVG(v ArchView) template.HTML { |
| } |
| for _, n := range v.Nodes { |
| fmt.Fprintf(&b, `<g class="archnode archnode-%s">`, template.HTMLEscapeString(n.Kind)) |
| + // The hover title carries the node in full — every string below is |
| + // cut to the rect, and this is where the untruncated version lives |
| + // (it is also what a screen reader reads for the group). |
| + fmt.Fprintf(&b, `<title>%s</title>`, template.HTMLEscapeString(nodeTitleText(n))) |
| fmt.Fprintf(&b, `<rect x="%d" y="%d" width="%d" height="%d" rx="8" />`, n.X, n.Y, n.W, n.H) |
| fmt.Fprintf(&b, `<text x="%d" y="%d" class="archtitle">%s</text>`, |
| - n.X+12, n.Y+24, template.HTMLEscapeString(n.Title)) |
| + n.X+textPad, n.Y+24, template.HTMLEscapeString(fitText(n.Title, titlePx, n.W-2*textPad-alarmRoom(n)))) |
| if n.Subtitle != "" { |
| fmt.Fprintf(&b, `<text x="%d" y="%d" class="archsub">%s</text>`, |
| - n.X+12, n.Y+44, template.HTMLEscapeString(n.Subtitle)) |
| + n.X+textPad, n.Y+44, template.HTMLEscapeString(fitText(n.Subtitle, subPx, n.W-2*textPad))) |
| + } |
| + // One mark per alarm, top-right, filling leftwards. A firing alarm |
| + // gets its own class so it reads as different at a glance rather |
| + // than only in the words of its hover title. |
| + for i, a := range n.Alarms { |
| + cls := "archalarm" |
| + if a.Firing { |
| + cls = "archalarm archalarm-firing" |
| + } |
| + cx := n.X + n.W - alarmInset - i*alarmStep |
| + fmt.Fprintf(&b, `<circle class="%s" cx="%d" cy="%d" r="%d"><title>%s</title></circle>`, |
| + cls, cx, n.Y+alarmInset, alarmR, |
| + template.HTMLEscapeString(a.Name+" "+a.State)) |
| } |
| b.WriteString(`</g>`) |
| } |
diff --git a/internal/console/archlayout_test.go b/internal/console/archlayout_test.go| index 1e2bde30d..a011c6bb8 100644 |
| --- a/internal/console/archlayout_test.go |
| +++ b/internal/console/archlayout_test.go |
| @@ -128,30 +128,28 @@ func archAlarmFixture() ArchInput { |
| return in |
| } |
| -// Alarms are DRAWN, as satellites in the column immediately left of the |
| -// thing they watch — an armed auto-recover alarm beside its box is the |
| -// operator-facing fact this page exists to surface. Left of, not right of, |
| -// so the `watches` edge reads in the same direction as every other edge. |
| -func TestLayoutDrawsAlarmSatellites(t *testing.T) { |
| +// An alarm-bearing estate stays clean: the alarms ride the nodes they |
| +// watch instead of standing in the flow (which is what put them under the |
| +// IP-to-box edges), and adding them changes neither the overlap-free |
| +// property nor determinism. |
| +func TestLayoutAlarmBearingEstateStaysClean(t *testing.T) { |
| v := BuildArch(archAlarmFixture()) |
| - x := map[string]int{} |
| + |
| for _, n := range v.Nodes { |
| - x[n.ID] = n.X |
| - } |
| - for _, c := range []struct{ alarm, watched string }{ |
| - {"alarm:a-1", "box:carlos-eu-west-1"}, |
| - {"alarm:a-2", "machine:i-999"}, |
| - } { |
| - if _, ok := x[c.alarm]; !ok { |
| - t.Fatalf("%s was dropped by layout: %+v", c.alarm, v.Nodes) |
| - } |
| - if !hasEdge(v, c.alarm, c.watched) { |
| - t.Fatalf("%s must watch %s: %+v", c.alarm, c.watched, v.Edges) |
| + if n.Kind == "alarm" { |
| + t.Fatalf("an alarm must not be a node in the flow: %+v", n) |
| } |
| - if x[c.alarm] >= x[c.watched] { |
| - t.Errorf("%s at x=%d must sit left of %s at x=%d", c.alarm, x[c.alarm], c.watched, x[c.watched]) |
| + } |
| + worn := map[string]int{} |
| + for _, n := range v.Nodes { |
| + worn[n.ID] = len(n.Alarms) |
| + } |
| + for _, id := range []string{"box:carlos-eu-west-1", "machine:i-999"} { |
| + if worn[id] == 0 { |
| + t.Errorf("%s should wear the alarm that watches it: %+v", id, v.Nodes) |
| } |
| } |
| + |
| for i := range v.Nodes { |
| for j := i + 1; j < len(v.Nodes); j++ { |
| a, b := v.Nodes[i], v.Nodes[j] |
diff --git a/internal/console/archlegibility_test.go b/internal/console/archlegibility_test.go| new file mode 100644 |
| index 000000000..039536fa9 |
| --- /dev/null |
| +++ b/internal/console/archlegibility_test.go |
| @@ -0,0 +1,149 @@ |
| +package console |
| + |
| +import ( |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/infraview" |
| +) |
| + |
| +// TestNoNodeSitsOnAnEdgePath is the invariant the alarm column broke: a |
| +// node that is not an endpoint of an edge must not sit ON that edge's |
| +// path. Nodes paint after edges, so a node standing in the middle of a |
| +// line hides it and the picture reads as a connection that does not |
| +// exist — an IP appearing to reach its box THROUGH an alarm. |
| +// |
| +// The edge is drawn as three segments (out, across, in), so the check is |
| +// the two horizontal runs at the endpoints' own y values plus the vertical |
| +// hop between them. |
| +func TestNoNodeSitsOnAnEdgePath(t *testing.T) { |
| + v := BuildArch(archFixture()) |
| + pos := map[string]Node{} |
| + for _, n := range v.Nodes { |
| + pos[n.ID] = n |
| + } |
| + for _, e := range v.Edges { |
| + from, to := pos[e.From], pos[e.To] |
| + x1, y1 := from.X+from.W, from.Y+from.H/2 |
| + x2, y2 := to.X, to.Y+to.H/2 |
| + mid := (x1 + x2) / 2 |
| + for _, n := range v.Nodes { |
| + if n.ID == e.From || n.ID == e.To { |
| + continue |
| + } |
| + if segmentHitsNode(x1, mid, y1, n) || segmentHitsNode(mid, x2, y2, n) || |
| + verticalHitsNode(mid, y1, y2, n) { |
| + t.Errorf("edge %s -> %s passes through node %s (%d,%d %dx%d)", |
| + e.From, e.To, n.ID, n.X, n.Y, n.W, n.H) |
| + } |
| + } |
| + } |
| +} |
| + |
| +func segmentHitsNode(xa, xb, y int, n Node) bool { |
| + if xa > xb { |
| + xa, xb = xb, xa |
| + } |
| + return y > n.Y && y < n.Y+n.H && xb > n.X && xa < n.X+n.W |
| +} |
| + |
| +func verticalHitsNode(x, ya, yb int, n Node) bool { |
| + if ya > yb { |
| + ya, yb = yb, ya |
| + } |
| + return x > n.X && x < n.X+n.W && yb > n.Y && ya < n.Y+n.H |
| +} |
| + |
| +// TestAlarmsRideTheNodeTheyWatch: an alarm is a property OF the thing it |
| +// watches, so it is drawn on that node rather than as a node of its own in |
| +// the flow. A firing alarm must be distinguishable from a healthy one by |
| +// something other than its wording. |
| +func TestAlarmsRideTheNodeTheyWatch(t *testing.T) { |
| + in := archFixture() |
| + in.Inv.Resources = append(in.Inv.Resources, |
| + infraview.Resource{Provider: "aws", Kind: infraview.KindAlarm, NativeType: "cloudwatch:alarm", |
| + ID: "arn:alarm/ok", Name: "carlos-eu-west-1-auto-recover", Region: "eu-west-1", |
| + Attrs: map[string]string{"state": "OK", "metric": "StatusCheckFailed_System", "target": "i-123"}}, |
| + infraview.Resource{Provider: "aws", Kind: infraview.KindAlarm, NativeType: "cloudwatch:alarm", |
| + ID: "arn:alarm/firing", Name: "carlos-eu-west-1-cpu", Region: "eu-west-1", |
| + Attrs: map[string]string{"state": "ALARM", "metric": "CPUUtilization", "target": "i-123"}}, |
| + ) |
| + v := BuildArch(in) |
| + |
| + if n := nodeByID(v, "alarm:arn:alarm/ok"); n != nil { |
| + t.Fatalf("an alarm must not be drawn as its own node in the flow: %+v", n) |
| + } |
| + box := nodeByID(v, "box:carlos-eu-west-1") |
| + if box == nil { |
| + t.Fatal("no box node") |
| + } |
| + if len(box.Alarms) != 2 { |
| + t.Fatalf("box carries %d alarms, want 2: %+v", len(box.Alarms), box.Alarms) |
| + } |
| + var firing, ok bool |
| + for _, a := range box.Alarms { |
| + if a.Name == "carlos-eu-west-1-cpu" && a.Firing { |
| + firing = true |
| + } |
| + if a.Name == "carlos-eu-west-1-auto-recover" && !a.Firing { |
| + ok = true |
| + } |
| + } |
| + if !firing || !ok { |
| + t.Errorf("firing state not carried per alarm: %+v", box.Alarms) |
| + } |
| + |
| + out := string(RenderArchSVG(v)) |
| + if !strings.Contains(out, "archalarm-firing") { |
| + t.Error("a firing alarm must render differently from a healthy one") |
| + } |
| + if !strings.Contains(out, "carlos-eu-west-1-cpu ALARM") { |
| + t.Error("the alarm's name and state must be reachable on the node (hover title)") |
| + } |
| +} |
| + |
| +// TestSVGTextIsFittedToItsBox: nothing may bleed past the rect it belongs |
| +// to. The full string stays reachable as the node's hover title, so |
| +// truncation costs nothing but the overflow. |
| +func TestSVGTextIsFittedToItsBox(t *testing.T) { |
| + in := archFixture() |
| + v := BuildArch(in) |
| + long := "console, kass, keymail, platform-site, woodstar, status, carloku-site" |
| + for i := range v.Nodes { |
| + if v.Nodes[i].Kind == "apps" { |
| + v.Nodes[i].Title = long |
| + v.Nodes[i].Subtitle = "static 3 · resident 4 · hibernating 2 · awake 6 · asleep 3" |
| + } |
| + } |
| + out := string(RenderArchSVG(v)) |
| + if strings.Contains(out, ">"+long+"<") { |
| + t.Error("a title wider than its rect must be truncated, not drawn past the box") |
| + } |
| + if !strings.Contains(out, "…") { |
| + t.Error("truncated text must show an ellipsis") |
| + } |
| + if !strings.Contains(out, long) { |
| + t.Error("the full string must survive as the node's hover title") |
| + } |
| +} |
| + |
| +// TestByoBoxIsNotPriced: the platform does not pay for a box it does not |
| +// own, so a bring-your-own machine gets no monthly figure and no |
| +// "priced by the default spec" chip — a price there would be a claim about |
| +// someone else's bill. |
| +func TestByoBoxIsNotPriced(t *testing.T) { |
| + v := BuildArch(archFixture()) |
| + for _, c := range v.Boxes { |
| + if c.CloudID != "" { |
| + continue |
| + } |
| + if c.USDMonth != 0 { |
| + t.Errorf("BYO box %s priced at %v", c.Label, c.USDMonth) |
| + } |
| + for _, d := range c.Drift { |
| + if strings.Contains(d, "default spec") { |
| + t.Errorf("BYO box %s carries a pricing-drift chip: %q", c.Label, d) |
| + } |
| + } |
| + } |
| +} |
diff --git a/internal/console/pending.go b/internal/console/pending.go| index 66dc5b326..30b0ee883 100644 |
| --- a/internal/console/pending.go |
| +++ b/internal/console/pending.go |
| @@ -19,6 +19,7 @@ import ( |
| "crypto/rand" |
| "database/sql" |
| "encoding/hex" |
| + "errors" |
| "fmt" |
| "time" |
| ) |
| @@ -27,6 +28,21 @@ import ( |
| // it can no longer be confirmed or executed. |
| const PendingChangeTTL = 7 * 24 * time.Hour |
| +// ConfirmChange's three refusals, as values. The console maps them onto the |
| +// wire (refuseConfirm: 404 / 410 / 409), and it does so with errors.Is — the |
| +// substring matching this replaced meant a reworded message silently became a |
| +// 500 telling a member to "try again" about a change that can never succeed. |
| +// A refusal that has to be recognised by another package's grep is not a |
| +// contract; these are. |
| +// |
| +// Each is wrapped with the change's id at the raise site, so the log and the |
| +// operator keep the detail while callers keep the value. |
| +var ( |
| + ErrNoPendingChange = errors.New("console: no such pending change") |
| + ErrPendingChangeExpired = errors.New("console: pending change has expired") |
| + ErrPendingChangeExecuted = errors.New("console: pending change has already been executed") |
| +) |
| + |
| // PendingChange is one proposed, quorum-gated action. |
| type PendingChange struct { |
| ID string |
| @@ -152,13 +168,13 @@ func (s *Store) ConfirmChange(id, address string, now time.Time) (*PendingChange |
| return nil, false, err |
| } |
| if pc == nil { |
| - return nil, false, fmt.Errorf("console: no such pending change %q", id) |
| + return nil, false, fmt.Errorf("%w: %q", ErrNoPendingChange, id) |
| } |
| if now.Unix() >= pc.ExpiresAt.Unix() { |
| - return nil, false, fmt.Errorf("console: pending change %q has expired", id) |
| + return nil, false, fmt.Errorf("%w: %q", ErrPendingChangeExpired, id) |
| } |
| if !executedAtZero(tx, id) { |
| - return nil, false, fmt.Errorf("console: pending change %q has already been executed", id) |
| + return nil, false, fmt.Errorf("%w: %q", ErrPendingChangeExecuted, id) |
| } |
| if _, err := tx.Exec(`INSERT OR IGNORE INTO pending_confirms (change_id, address, confirmed_at) VALUES (?, ?, ?)`, |
| id, address, now.Unix()); err != nil { |
| @@ -254,6 +270,39 @@ func (s *Store) PendingChange(id string, now time.Time) (*PendingChange, error) |
| return pc, nil |
| } |
| +// MemberConfirms is a change's confirming addresses filtered to those that |
| +// are STILL members of the account — the tally EXECUTION must count (spec §4: |
| +// "quorum is N distinct MEMBERS"). A confirmation is an act of standing |
| +// authority, and an address that has left the account has none: without this |
| +// filter, removing somebody from an account would leave their approvals |
| +// working on their way out, and an account could be brought to quorum by |
| +// people who are no longer in it. |
| +// |
| +// The rows are KEPT, not deleted. Who confirmed what is a true fact about the |
| +// past and the trail says so; what changes is only whether it still counts. |
| +// Confirms (the display tally on the card and the CLI list) deliberately |
| +// keeps showing every row — the gap between the two numbers is the honest |
| +// picture, and a card that quietly dropped a name would make a stuck change |
| +// unexplainable. |
| +func (s *Store) MemberConfirms(accountID int64, id string) ([]string, error) { |
| + rows, err := s.db.Query(`SELECT c.address FROM pending_confirms c |
| +JOIN memberships m ON m.address = c.address AND m.account_id = ? |
| +WHERE c.change_id = ? ORDER BY c.confirmed_at, c.address`, accountID, id) |
| + if err != nil { |
| + return nil, err |
| + } |
| + defer rows.Close() |
| + var out []string |
| + for rows.Next() { |
| + var a string |
| + if err := rows.Scan(&a); err != nil { |
| + return nil, err |
| + } |
| + out = append(out, a) |
| + } |
| + return out, rows.Err() |
| +} |
| + |
| // MarkExecuted stamps a change as carried out, so it stops appearing as |
| // pending and can no longer be confirmed. |
| func (s *Store) MarkExecuted(id string, now time.Time) error { |
diff --git a/internal/console/pending_test.go b/internal/console/pending_test.go| index 5c5ddcb4d..ca29f6189 100644 |
| --- a/internal/console/pending_test.go |
| +++ b/internal/console/pending_test.go |
| @@ -1,6 +1,10 @@ |
| package console |
| import ( |
| + "errors" |
| + "fmt" |
| + "net/http" |
| + "net/http/httptest" |
| "reflect" |
| "sort" |
| "strings" |
| @@ -183,3 +187,68 @@ func TestPendingChangeExpiryAndExecution(t *testing.T) { |
| t.Fatalf("PendingChange after DeleteChange = %v, %v, want nil, nil", got, err) |
| } |
| } |
| + |
| +// TestConfirmChangeRefusalsAreSentinels: ConfirmChange's three refusals are |
| +// values a caller can test for, not prose it has to grep. The HTTP mapping |
| +// (refuseConfirm) turns them into 404 / 410 / 409, and a reworded message |
| +// must not be able to silently downgrade any of those to a 500 that tells a |
| +// member to "try again" about something that can never succeed. |
| +func TestConfirmChangeRefusalsAreSentinels(t *testing.T) { |
| + s := openTestStore(t) |
| + now := time.Now() |
| + accountID := testAccountID() |
| + |
| + if _, _, err := s.ConfirmChange("nosuchid", "paul@keymail.dev", now); !errors.Is(err, ErrNoPendingChange) { |
| + t.Errorf("ConfirmChange on an unknown id = %v, want ErrNoPendingChange", err) |
| + } |
| + |
| + expiring, err := s.ProposeChange(accountID, "kass", "promote", []byte(`{}`), 2, "paul@keymail.dev", true, now) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + _, _, err = s.ConfirmChange(expiring.ID, "bob@keymail.dev", expiring.ExpiresAt.Add(time.Second)) |
| + if !errors.Is(err, ErrPendingChangeExpired) { |
| + t.Errorf("ConfirmChange past expiry = %v, want ErrPendingChangeExpired", err) |
| + } |
| + // The id stays in the message: a sentinel is for the caller, the text is |
| + // for the log. |
| + if err == nil || !strings.Contains(err.Error(), expiring.ID) { |
| + t.Errorf("expiry error %v does not name the change", err) |
| + } |
| + |
| + done, err := s.ProposeChange(accountID, "kass", "promote", []byte(`{}`), 2, "paul@keymail.dev", true, now) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := s.MarkExecuted(done.ID, now); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, _, err := s.ConfirmChange(done.ID, "bob@keymail.dev", now); !errors.Is(err, ErrPendingChangeExecuted) { |
| + t.Errorf("ConfirmChange on an executed change = %v, want ErrPendingChangeExecuted", err) |
| + } |
| +} |
| + |
| +// TestRefuseConfirmMapsSentinels: the status each sentinel earns, pinned |
| +// against errors.Is rather than the wording — the wording here is deliberately |
| +// nothing like the store's, which is exactly the case substring matching used |
| +// to get wrong. |
| +func TestRefuseConfirmMapsSentinels(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + for _, tc := range []struct { |
| + name string |
| + err error |
| + want int |
| + }{ |
| + {"unknown", fmt.Errorf("wrapped in something else entirely: %w", ErrNoPendingChange), http.StatusNotFound}, |
| + {"expired", fmt.Errorf("wrapped in something else entirely: %w", ErrPendingChangeExpired), http.StatusGone}, |
| + {"executed", fmt.Errorf("wrapped in something else entirely: %w", ErrPendingChangeExecuted), http.StatusConflict}, |
| + {"other", errors.New("database is locked"), http.StatusInternalServerError}, |
| + } { |
| + rec := httptest.NewRecorder() |
| + req := httptest.NewRequest(http.MethodPost, "/a/bab/apps/widget/pending/x/confirm", nil) |
| + fx.srv.refuseConfirm(rec, req, tc.err) |
| + if rec.Code != tc.want { |
| + t.Errorf("refuseConfirm(%s) = %d, want %d", tc.name, rec.Code, tc.want) |
| + } |
| + } |
| +} |
diff --git a/internal/console/pendingflow_test.go b/internal/console/pendingflow_test.go| index 78c137977..a300fcf63 100644 |
| --- a/internal/console/pendingflow_test.go |
| +++ b/internal/console/pendingflow_test.go |
| @@ -941,3 +941,289 @@ func TestHotfixBypassesOrderNotRules(t *testing.T) { |
| t.Fatalf("production serves %q, want abc1234", got.Version) |
| } |
| } |
| + |
| +// TestExecutionMeetsTheCURRENTQuorum: a proposal carries the quorum that was |
| +// in force when it was made, and a week can pass before it is confirmed. If |
| +// the channel's rule was TIGHTENED in between — 2 approvals to 3 — executing |
| +// at the stored 2 would let a stale proposal be the way past the new rule. |
| +// Execution therefore re-reads the pipeline and requires max(stored, current). |
| +// The refusal is an error, not a consumption: the change stays listed and |
| +// stays retryable, so the third member can still meet it. |
| +func TestExecutionMeetsTheCURRENTQuorum(t *testing.T) { |
| + fx, c, csrf := memberFixture(t) // paul owns, kim is a member |
| + if err := fx.st.AddMember(OpsAccountID, "alice@keymail.dev", RoleMember, time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + pipelinedApp(t, fx, "widget", "abc1234", |
| + release.PipelineChannel{Name: "edge"}, |
| + release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 2}}) |
| + |
| + status, loc, body := postGated(t, fx, c, fx.u("/apps/widget/promote"), |
| + url.Values{"csrf": {csrf}, "channel": {"production"}, "version": {"abc1234"}, "confirmed": {"1"}}) |
| + if status != http.StatusSeeOther { |
| + t.Fatalf("owner proposal = %d -> %q:\n%s", status, loc, body) |
| + } |
| + pending, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(pending) != 1 || pending[0].Quorum != 2 { |
| + t.Fatalf("pending = %+v, want one promote needing 2", pending) |
| + } |
| + pc := pending[0] |
| + |
| + // The rule is raised while the proposal waits. |
| + setPipeline(t, fx, "widget", |
| + release.PipelineChannel{Name: "edge"}, |
| + release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 3}}) |
| + |
| + kim := newBrowser(t) |
| + fx.setUser("kim@keymail.dev") |
| + signUp(t, fx, kim, "code-kim-tightened") |
| + kimCSRF := sessionToken(t, fx, kim) |
| + before := serial(t, fx) |
| + status, loc, body = postGated(t, fx, kim, |
| + fx.u("/apps/widget/pending/"+pc.ID+"/confirm"), url.Values{"csrf": {kimCSRF}}) |
| + if status != http.StatusSeeOther { |
| + t.Fatalf("second confirmation = %d -> %q:\n%s", status, loc, body) |
| + } |
| + channelAbsent(t, fx, "widget", "production") |
| + if got := serial(t, fx); got != before { |
| + t.Fatalf("serial = %d, want it unmoved at %d — nothing may be written under a quorum that is no longer enough", got, before) |
| + } |
| + left, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(left) != 1 || left[0].ID != pc.ID { |
| + t.Fatalf("pending after the refusal = %+v, want the change still listed and retryable", left) |
| + } |
| + if len(left[0].Confirms) != 2 { |
| + t.Fatalf("confirmations = %v, want both kept — a refusal must not discard them", left[0].Confirms) |
| + } |
| + |
| + // The third member meets the new rule, and THAT lands it. |
| + alice := newBrowser(t) |
| + fx.setUser("alice@keymail.dev") |
| + signUp(t, fx, alice, "code-alice-tightened") |
| + aliceCSRF := sessionToken(t, fx, alice) |
| + status, loc, body = postGated(t, fx, alice, |
| + fx.u("/apps/widget/pending/"+pc.ID+"/confirm"), url.Values{"csrf": {aliceCSRF}}) |
| + if status != http.StatusSeeOther { |
| + t.Fatalf("third confirmation = %d -> %q:\n%s", status, loc, body) |
| + } |
| + if got := fx.channelVersion(t, "widget", "production"); got.Version != "abc1234" { |
| + t.Fatalf("production serves %q after the raised quorum was met, want abc1234", got.Version) |
| + } |
| +} |
| + |
| +// TestPendingTalliesShowWhatExecutionRequires: the card, the CLI list and the |
| +// confirm flash must not promise a quorum execution will refuse. With the |
| +// rule raised from 2 to 3 underneath a live proposal, every surface has to |
| +// read 2 of 3 — a change sitting at "2 of 2" beside a Confirm button that |
| +// refuses every time it is pressed is worse than no card at all (PR #258 |
| +// review, I3). And a confirmation from a departed member has to stop counting |
| +// on those same surfaces, for the same reason. |
| +func TestPendingTalliesShowWhatExecutionRequires(t *testing.T) { |
| + fx, c, csrf := memberFixture(t) // paul owns, kim is a member |
| + if err := fx.st.AddMember(OpsAccountID, "alice@keymail.dev", RoleMember, time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + pipelinedApp(t, fx, "widget", "abc1234", |
| + release.PipelineChannel{Name: "edge"}, |
| + release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 2}}) |
| + |
| + if status, loc, body := postGated(t, fx, c, fx.u("/apps/widget/promote"), |
| + url.Values{"csrf": {csrf}, "channel": {"production"}, "version": {"abc1234"}, "confirmed": {"1"}}); status != http.StatusSeeOther { |
| + t.Fatalf("owner proposal = %d -> %q:\n%s", status, loc, body) |
| + } |
| + // Before the tightening the card reads 1/2 — pending.html's own format, |
| + // asserted here so the checks after the tightening are known to be |
| + // looking at the card and not at some flash that happens to say "of". |
| + _, _, body := get(t, c, fx.u("/apps/widget")) |
| + if !strings.Contains(body, "1/2 confirmed") { |
| + t.Fatalf("card does not show 1/2 confirmed before the tightening:\n%s", body) |
| + } |
| + |
| + setPipeline(t, fx, "widget", |
| + release.PipelineChannel{Name: "edge"}, |
| + release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 3}}) |
| + |
| + // The card. |
| + _, _, body = get(t, c, fx.u("/apps/widget")) |
| + if !strings.Contains(body, "1/3 confirmed") { |
| + t.Errorf("card shows the stored quorum, not the one execution now requires:\n%s", body) |
| + } |
| + if strings.Contains(body, "1/2 confirmed") { |
| + t.Errorf("card still shows the superseded 1/2:\n%s", body) |
| + } |
| + // The CLI list. |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-tally-list") |
| + status, out := getJSON(t, fx.ts.URL+"/api/cli/apps/bab/widget/pending", token) |
| + if status != http.StatusOK { |
| + t.Fatalf("pending list = %d %v", status, out) |
| + } |
| + list, _ := out["pending"].([]any) |
| + if len(list) != 1 { |
| + t.Fatalf("pending list = %v, want one row", out["pending"]) |
| + } |
| + row, _ := list[0].(map[string]any) |
| + if num(t, row, "needed") != 3 || num(t, row, "confirmed") != 1 { |
| + t.Errorf("CLI row = %v, want 1 of 3", row) |
| + } |
| + |
| + // The confirm flash, and the audit row behind it: kim's confirmation |
| + // takes it to 2 of 3, not "2 of 2, executed". |
| + pending, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + kim := newBrowser(t) |
| + fx.setUser("kim@keymail.dev") |
| + signUp(t, fx, kim, "code-kim-tally") |
| + kimCSRF := sessionToken(t, fx, kim) |
| + if status, loc, body := postGated(t, fx, kim, |
| + fx.u("/apps/widget/pending/"+pending[0].ID+"/confirm"), url.Values{"csrf": {kimCSRF}}); status != http.StatusSeeOther { |
| + t.Fatalf("second confirmation = %d -> %q:\n%s", status, loc, body) |
| + } |
| + _, _, kimBody := get(t, kim, fx.u("/apps/widget")) |
| + if !strings.Contains(kimBody, "2/3 confirmed") { |
| + t.Errorf("after the second confirmation the card does not read 2/3:\n%s", kimBody) |
| + } |
| + events, err := fx.st.Events(OpsAccountID, "widget", 10) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + found := false |
| + for _, e := range events { |
| + if e.Action == "confirmed" && strings.Contains(e.Detail, "2 of 3 confirmations") { |
| + found = true |
| + } |
| + } |
| + if !found { |
| + t.Errorf("no audit row recording 2 of 3 confirmations: %+v", events) |
| + } |
| + |
| + // And a departed member's row stops counting on the surfaces too. |
| + if err := fx.st.RemoveMember(OpsAccountID, "kim@keymail.dev"); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, _, body = get(t, c, fx.u("/apps/widget")) |
| + if !strings.Contains(body, "1/3 confirmed") { |
| + t.Errorf("card counts a departed member's confirmation:\n%s", body) |
| + } |
| +} |
| + |
| +// TestLoosenedRuleDoesNotLowerAStoredQuorum: the ratchet only goes one way. |
| +// A proposal made at 3 approvals is not executable at 2 because somebody |
| +// lowered the rule afterwards — the confirmers agreed to a change that needed |
| +// three of them, and lowering the rule is not their confirmation. |
| +func TestLoosenedRuleDoesNotLowerAStoredQuorum(t *testing.T) { |
| + fx, c, csrf := memberFixture(t) // paul owns, kim is a member |
| + if err := fx.st.AddMember(OpsAccountID, "alice@keymail.dev", RoleMember, time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + pipelinedApp(t, fx, "widget", "abc1234", |
| + release.PipelineChannel{Name: "edge"}, |
| + release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 3}}) |
| + |
| + if status, loc, body := postGated(t, fx, c, fx.u("/apps/widget/promote"), |
| + url.Values{"csrf": {csrf}, "channel": {"production"}, "version": {"abc1234"}, "confirmed": {"1"}}); status != http.StatusSeeOther { |
| + t.Fatalf("owner proposal = %d -> %q:\n%s", status, loc, body) |
| + } |
| + pending, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(pending) != 1 || pending[0].Quorum != 3 { |
| + t.Fatalf("pending = %+v, want one promote needing 3", pending) |
| + } |
| + pc := pending[0] |
| + |
| + setPipeline(t, fx, "widget", |
| + release.PipelineChannel{Name: "edge"}, |
| + release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 2}}) |
| + |
| + kim := newBrowser(t) |
| + fx.setUser("kim@keymail.dev") |
| + signUp(t, fx, kim, "code-kim-loosened") |
| + kimCSRF := sessionToken(t, fx, kim) |
| + if status, loc, body := postGated(t, fx, kim, |
| + fx.u("/apps/widget/pending/"+pc.ID+"/confirm"), url.Values{"csrf": {kimCSRF}}); status != http.StatusSeeOther { |
| + t.Fatalf("second confirmation = %d -> %q:\n%s", status, loc, body) |
| + } |
| + channelAbsent(t, fx, "widget", "production") |
| + left, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(left) != 1 { |
| + t.Fatalf("pending after the loosened rule = %+v, want the change still waiting on its own quorum of 3", left) |
| + } |
| +} |
| + |
| +// TestDepartedMemberConfirmDoesNotCount: a quorum is N distinct MEMBERS. An |
| +// address that has since left the account is not one, so its row must not |
| +// carry a change over the line — otherwise removing somebody from an account |
| +// would leave their standing approvals working on their way out. The row is |
| +// kept (the trail of who confirmed what stays true); it simply stops counting. |
| +func TestDepartedMemberConfirmDoesNotCount(t *testing.T) { |
| + fx, c, csrf := memberFixture(t) // paul owns, kim is a member |
| + if err := fx.st.AddMember(OpsAccountID, "alice@keymail.dev", RoleMember, time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + pipelinedApp(t, fx, "widget", "abc1234", |
| + release.PipelineChannel{Name: "edge"}, |
| + release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 3}}) |
| + |
| + if status, loc, body := postGated(t, fx, c, fx.u("/apps/widget/promote"), |
| + url.Values{"csrf": {csrf}, "channel": {"production"}, "version": {"abc1234"}, "confirmed": {"1"}}); status != http.StatusSeeOther { |
| + t.Fatalf("owner proposal = %d -> %q:\n%s", status, loc, body) |
| + } |
| + pending, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(pending) != 1 { |
| + t.Fatalf("pending = %+v, want one", pending) |
| + } |
| + pc := pending[0] |
| + |
| + kim := newBrowser(t) |
| + fx.setUser("kim@keymail.dev") |
| + signUp(t, fx, kim, "code-kim-departed") |
| + kimCSRF := sessionToken(t, fx, kim) |
| + if status, loc, body := postGated(t, fx, kim, |
| + fx.u("/apps/widget/pending/"+pc.ID+"/confirm"), url.Values{"csrf": {kimCSRF}}); status != http.StatusSeeOther { |
| + t.Fatalf("kim's confirmation = %d -> %q:\n%s", status, loc, body) |
| + } |
| + |
| + // Kim leaves the account, with her confirmation still on the row. |
| + if err := fx.st.RemoveMember(OpsAccountID, "kim@keymail.dev"); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + alice := newBrowser(t) |
| + fx.setUser("alice@keymail.dev") |
| + signUp(t, fx, alice, "code-alice-departed") |
| + aliceCSRF := sessionToken(t, fx, alice) |
| + before := serial(t, fx) |
| + if status, loc, body := postGated(t, fx, alice, |
| + fx.u("/apps/widget/pending/"+pc.ID+"/confirm"), url.Values{"csrf": {aliceCSRF}}); status != http.StatusSeeOther { |
| + t.Fatalf("alice's confirmation = %d -> %q:\n%s", status, loc, body) |
| + } |
| + channelAbsent(t, fx, "widget", "production") |
| + if got := serial(t, fx); got != before { |
| + t.Fatalf("serial = %d, want it unmoved at %d — a departed member's row must not carry a change over the line", got, before) |
| + } |
| + left, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(left) != 1 || left[0].ID != pc.ID { |
| + t.Fatalf("pending after the refusal = %+v, want the change still listed", left) |
| + } |
| + if len(left[0].Confirms) != 3 { |
| + t.Fatalf("confirmations = %v, want all three rows KEPT — they simply do not all count", left[0].Confirms) |
| + } |
| +} |
diff --git a/internal/console/pendinghandlers.go b/internal/console/pendinghandlers.go| index 16f0bb5f7..7d6b94fca 100644 |
| --- a/internal/console/pendinghandlers.go |
| +++ b/internal/console/pendinghandlers.go |
| @@ -286,30 +286,65 @@ func policyProposal(cur *release.Pipeline, next []release.PipelineChannel) polic |
| // caller must not mark it executed, and the member who confirmed it is told |
| // why rather than left believing it went through. |
| // |
| -// The first two checks are this function's OWN preconditions, restated rather |
| +// The first checks are this function's OWN preconditions, restated rather |
| // than assumed. Every caller today establishes them — quorum via |
| // ConfirmChange's `met`, the app via pendingScoped's 404 — but this is billed |
| // as the one place a gated write happens, and a future caller (the console's |
| // own pipeline page, Task 9) that got either wrong would otherwise turn a |
| // mistake into an unapproved signed write. They are errors, never panics: a |
| // bug here must refuse, not take the console down. |
| +// |
| +// The quorum precondition is not a restatement, though: ConfirmChange counts |
| +// ROWS against the number STORED on the proposal, and both of those can be |
| +// stale by the time anyone confirms. This is where the CURRENT rule and the |
| +// CURRENT membership are what the change has to meet — see the comment on the |
| +// check itself. |
| func (s *Server) executePending(ctx context.Context, acct *Account, app, actor string, pc *PendingChange) (string, error) { |
| if pc.AccountID != acct.ID || pc.App != app { |
| log.Printf("console: execute %s %s: change belongs to %d/%q, not %d/%q", |
| pc.Kind, pc.ID, pc.AccountID, pc.App, acct.ID, app) |
| return "", errors.New("that change belongs to a different app") |
| } |
| - if len(pc.Confirms) < pc.Quorum { |
| - log.Printf("console: execute %s %s: %d of %d confirmations", pc.Kind, pc.ID, len(pc.Confirms), pc.Quorum) |
| - return "", fmt.Errorf("that change has %d of %d confirmations", len(pc.Confirms), pc.Quorum) |
| + // The quorum this change must meet NOW, not merely the one it was |
| + // proposed under. A proposal can wait a week (PendingChangeTTL), and both |
| + // halves of "N distinct members" can move underneath it: |
| + // |
| + // - the RULE can be tightened. Executing at the stored number would make |
| + // a stale proposal the way past a raised rule — propose at 2, wait for |
| + // somebody to raise it to 3, land it with 2. Required is therefore |
| + // max(stored, current). Loosening never lowers the stored number: the |
| + // confirmers agreed to something that needed N of them, and one person |
| + // lowering the rule afterwards is not the others' confirmation. |
| + // - the MEMBERSHIP can change. A confirmation is standing authority, and |
| + // an address that has left the account has none, so its row stops |
| + // counting (MemberConfirms) while staying on the record. |
| + // |
| + // Both are refusals, never consumptions: the change stays unexecuted, |
| + // listed and retryable, and the remaining members can still meet it. |
| + required, err := s.currentQuorum(ctx, acct, app, pc) |
| + if err != nil { |
| + return "", err |
| } |
| + counted, err := s.store.MemberConfirms(pc.AccountID, pc.ID) |
| + if err != nil { |
| + log.Printf("console: execute %s %s: member confirms: %v", pc.Kind, pc.ID, err) |
| + return "", errors.New("could not check who has confirmed this change") |
| + } |
| + if len(counted) < required { |
| + log.Printf("console: execute %s %s: %d of %d confirmations counted (%d rows, quorum stored %d)", |
| + pc.Kind, pc.ID, len(counted), required, len(pc.Confirms), pc.Quorum) |
| + return "", fmt.Errorf("that change has %d of %d confirmations", len(counted), required) |
| + } |
| + // From the gate onwards, this change's confirmations ARE the counted |
| + // ones: every audit row below reports "%d confirmations" from here, and a |
| + // trail that credited a departed member's row would record a quorum that |
| + // was never met (PR #258 review, I3). The rows themselves are untouched |
| + // in the database — this narrows only what the EXECUTION claims it had. |
| + pc.Confirms = counted |
| if s.cfg.Key == nil { |
| return "", errors.New("no signing key on this host — the console is running read-only") |
| } |
| - var ( |
| - msg string |
| - err error |
| - ) |
| + var msg string |
| switch pc.Kind { |
| case kindPromote: |
| msg, err = s.executePendingPromote(ctx, acct, app, actor, pc) |
| @@ -335,6 +370,108 @@ func (s *Server) executePending(ctx context.Context, acct *Account, app, actor s |
| return msg, nil |
| } |
| +// currentQuorum is how many confirmations this change must have RIGHT NOW: |
| +// the number it was proposed under, raised to the target channel's current |
| +// rule if that rule has since been tightened. Never lowered — see |
| +// executePending's comment for why a loosening is not a confirmation. |
| +// |
| +// It costs one pipeline read that the promote path then makes again a moment |
| +// later. That duplication is deliberate: this is the gate, and a gate that |
| +// depends on a value threaded down through three kind-specific functions is a |
| +// gate that a fourth kind can be added without. A gated execution is a rare, |
| +// human-paced action; the read is not a cost worth trading the shape for. |
| +// |
| +// Every kind pays the read, INCLUDING a policy edit — the pipeline is fetched |
| +// before the kind is looked at, so an unreadable one fails the execution |
| +// closed whatever it is. What a policy edit does not do is raise its number |
| +// from a channel rule: its quorum comes from the channels its own edit |
| +// TOUCHES (guardedEdit), and executePendingPolicy refuses outright unless the |
| +// stored pipeline is byte-for-byte the one the proposal was composed against |
| +// (the SetAt pin). A rule that moved at all invalidates the proposal before |
| +// any quorum question is asked, so for that kind requiredQuorum returns the |
| +// stored number and max(stored, current) could only ever equal it. |
| +func (s *Server) currentQuorum(ctx context.Context, acct *Account, app string, pc *PendingChange) (int, error) { |
| + pipe, err := release.GetPipeline(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app) |
| + if err != nil { |
| + // Fail closed, like every other unreadable-pipeline path: a proposal |
| + // whose governing rules cannot be read is not executable. |
| + log.Printf("console: execute %s %s/%s: read pipeline: %v", pc.Kind, acct.Sqid, app, err) |
| + return 0, errors.New("could not read the pipeline") |
| + } |
| + return requiredQuorum(pc, pipe) |
| +} |
| + |
| +// requiredQuorum is currentQuorum's pure half: max(the number stored on the |
| +// proposal, the target channel's rule in pipe). Shared with the DISPLAY |
| +// surfaces (pendingChangeViews, handleAPIPending) so a card can never promise |
| +// a quorum execution will not accept — one rule, one place. |
| +// |
| +// A nil pipe (legacy app, or a display caller whose best-effort read failed) |
| +// carries no rules, so the answer is the stored quorum: the pre-existing |
| +// behaviour, and the safe direction for a display. |
| +func requiredQuorum(pc *PendingChange, pipe *release.Pipeline) (int, error) { |
| + var channel string |
| + var rule func(release.ChannelRules) int |
| + switch pc.Kind { |
| + case kindPromote: |
| + var p promotePayload |
| + if err := json.Unmarshal(pc.Payload, &p); err != nil { |
| + return 0, errors.New("this proposal's payload cannot be read — reject it and propose again") |
| + } |
| + channel, rule = p.Channel, func(r release.ChannelRules) int { return r.PromoteApprovals } |
| + case kindFastTrack: |
| + var p fastTrackPayload |
| + if err := json.Unmarshal(pc.Payload, &p); err != nil { |
| + return 0, errors.New("this proposal's payload cannot be read — reject it and propose again") |
| + } |
| + // A fast-track is gated by change_approvals, not promote_approvals |
| + // (spec §2: one field covers editing a channel's entry AND overriding |
| + // its bake), so it must be re-checked against that same field. |
| + channel, rule = p.Channel, func(r release.ChannelRules) int { return r.ChangeApprovals } |
| + default: |
| + return pc.Quorum, nil |
| + } |
| + if n := rule(channelRules(pipe, channel)); n > pc.Quorum { |
| + return n, nil |
| + } |
| + return pc.Quorum, nil |
| +} |
| + |
| +// pendingTally is the two numbers every SURFACE shows for one pending change |
| +// — the card, the CLI list, the confirm flash and the audit row — computed |
| +// the way execution counts them: confirmations from current members, against |
| +// max(stored quorum, current rule). Best-effort by construction: a failed |
| +// membership read or an unreadable payload degrades to the raw row rather |
| +// than costing the page, and execution re-checks both for real anyway. |
| +// |
| +// The gap between this and len(pc.Confirms) is deliberately not hidden: a |
| +// change stuck because someone left the account shows fewer confirmations |
| +// than it has rows, which is the honest explanation for why it is stuck. |
| +func (s *Server) pendingTally(pc PendingChange, pipe *release.Pipeline) (confirmed, needed int) { |
| + confirmed, needed = len(pc.Confirms), pc.Quorum |
| + if counted, err := s.store.MemberConfirms(pc.AccountID, pc.ID); err != nil { |
| + log.Printf("console: pending tally %s: member confirms: %v", pc.ID, err) |
| + } else { |
| + confirmed = len(counted) |
| + } |
| + if n, err := requiredQuorum(&pc, pipe); err == nil && n > needed { |
| + needed = n |
| + } |
| + return confirmed, needed |
| +} |
| + |
| +// pendingPipeline is the display surfaces' best-effort pipeline read: one per |
| +// listing, never fatal. A WRITE door reads its own copy and refuses on error |
| +// (currentQuorum); a card that cannot read the pipeline still renders. |
| +func (s *Server) pendingPipeline(ctx context.Context, acct *Account, app string) *release.Pipeline { |
| + pipe, err := release.GetPipeline(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app) |
| + if err != nil { |
| + log.Printf("console: pending tally %s/%s: read pipeline: %v", acct.Sqid, app, err) |
| + return nil |
| + } |
| + return pipe |
| +} |
| + |
| // staleProposal is the one refusal every kind shares: the world the proposal |
| // pinned is not the world any more. |
| func staleProposal(detail string) error { |
| @@ -546,20 +683,29 @@ func (s *Server) handlePendingConfirm(w http.ResponseWriter, r *http.Request) { |
| s.setFlash(w, kind, msg) |
| http.Redirect(w, r, appPath(acct, app), http.StatusSeeOther) |
| } |
| - pc, met, err := s.store.ConfirmChange(pc.ID, addr, time.Now()) |
| + pc, _, err := s.store.ConfirmChange(pc.ID, addr, time.Now()) |
| if err != nil { |
| s.refuseConfirm(w, r, err) |
| return |
| } |
| - if !met { |
| + ctx, cancel := context.WithTimeout(r.Context(), pendingTimeout) |
| + defer cancel() |
| + // ConfirmChange's own `met` counts ROWS against the STORED quorum, which |
| + // is not what execution requires (see executePending): a change whose |
| + // rule was raised, or whose confirmations include somebody who has since |
| + // left the account, is not met however many rows it carries. The tally |
| + // shown and audited here is the one that decides, so the flash a member |
| + // gets and the row the trail keeps say the same thing the gate does. |
| + // Deciding on it also keeps the not-met path a plain progress message |
| + // rather than an execution that refuses a beat later. |
| + confirmed, needed := s.pendingTally(*pc, s.pendingPipeline(ctx, acct, app)) |
| + if confirmed < needed { |
| s.audit(acct, app, addr, "confirmed", fmt.Sprintf("%s (%d of %d confirmations)", |
| - pendingSummary(*pc), len(pc.Confirms), pc.Quorum)) |
| + pendingSummary(*pc), confirmed, needed)) |
| back("ok", fmt.Sprintf("Confirmed: %s. %d of %d confirmations.", |
| - pendingSummary(*pc), len(pc.Confirms), pc.Quorum)) |
| + pendingSummary(*pc), confirmed, needed)) |
| return |
| } |
| - ctx, cancel := context.WithTimeout(r.Context(), pendingTimeout) |
| - defer cancel() |
| msg, err := s.executePending(ctx, acct, app, addr, pc) |
| if err != nil { |
| back("err", "Could not carry that out: "+err.Error()) |
| @@ -570,15 +716,20 @@ func (s *Server) handlePendingConfirm(w http.ResponseWriter, r *http.Request) { |
| // refuseConfirm maps ConfirmChange's own refusals onto the wire. See |
| // handlePendingConfirm's doc comment for why this cannot be an oracle. |
| +// |
| +// errors.Is, never the message text: these are sentinel values (pending.go) |
| +// precisely so that rewording a refusal cannot silently turn a 410 or a 409 |
| +// into the 500 fallback — "please try again" is the wrong thing to tell |
| +// somebody about a change that expired, and a wrong status is the kind of |
| +// regression no test catches unless the mapping is pinned to values. |
| func (s *Server) refuseConfirm(w http.ResponseWriter, r *http.Request, err error) { |
| - msg := err.Error() |
| switch { |
| - case strings.Contains(msg, "no such pending change"): |
| + case errors.Is(err, ErrNoPendingChange): |
| http.NotFound(w, r) |
| - case strings.Contains(msg, "has expired"): |
| + case errors.Is(err, ErrPendingChangeExpired): |
| s.renderMessage(w, r, http.StatusGone, "Expired", |
| "That change expired before it collected enough confirmations. Nothing has been changed — propose it again.") |
| - case strings.Contains(msg, "already been executed"): |
| + case errors.Is(err, ErrPendingChangeExecuted): |
| s.renderMessage(w, r, http.StatusConflict, "Already done", |
| "That change was carried out while you were confirming it. Nothing else has been changed.") |
| default: |
| @@ -641,14 +792,23 @@ func (s *Server) handleAPIPending(w http.ResponseWriter, r *http.Request) { |
| jsonError(w, http.StatusInternalServerError, "could not read pending changes") |
| return |
| } |
| + // The same tally the card shows, from one pipeline read: "confirmed" and |
| + // "needed" are what EXECUTION will count, so `carlos pipeline` cannot |
| + // print a change as ready when confirming it would refuse (PR #258 |
| + // review, I3). |
| + var pipe *release.Pipeline |
| + if len(list) > 0 { |
| + pipe = s.pendingPipeline(r.Context(), acct, app) |
| + } |
| out := make([]map[string]any, 0, len(list)) |
| for _, pc := range list { |
| + confirmed, needed := s.pendingTally(pc, pipe) |
| out = append(out, map[string]any{ |
| "id": pc.ID, |
| "kind": pc.Kind, |
| "summary": pendingSummary(pc), |
| - "confirmed": len(pc.Confirms), |
| - "needed": pc.Quorum, |
| + "confirmed": confirmed, |
| + "needed": needed, |
| }) |
| } |
| writeJSON(w, http.StatusOK, map[string]any{"pending": out}) |
diff --git a/internal/console/pipelinepage.go b/internal/console/pipelinepage.go| index 402334976..06be92230 100644 |
| --- a/internal/console/pipelinepage.go |
| +++ b/internal/console/pipelinepage.go |
| @@ -46,12 +46,24 @@ type PendingChangeView struct { |
| // pendingChangeViews folds a store listing into the card's rows, oldest |
| // first — PendingChangesFor's own order, unchanged, so the card and the |
| // CLI's `carlos pipeline` listing (Task 10) read the same queue the same way. |
| -func pendingChangeViews(pcs []PendingChange) []PendingChangeView { |
| +// |
| +// The two numbers are the ones EXECUTION will use, not the ones the row was |
| +// written with (PR #258 review, I3): Needed is max(stored quorum, the target |
| +// channel's current rule) and Confirmed counts only confirmations from |
| +// current members. Showing the raw row instead let a change sit at "2 of 2" |
| +// with a live Confirm button and refuse every time it was pressed — a card |
| +// that says a change is ready when it is not is worse than no card. |
| +// |
| +// pipe is the app's pipeline, read ONCE by the caller for the whole listing |
| +// (nil is fine — a legacy app, or a read that failed: the tally then degrades |
| +// to the stored quorum, which is exactly what it was before this existed). |
| +func (s *Server) pendingChangeViews(pcs []PendingChange, pipe *release.Pipeline) []PendingChangeView { |
| out := make([]PendingChangeView, len(pcs)) |
| for i, pc := range pcs { |
| + confirmed, needed := s.pendingTally(pc, pipe) |
| out[i] = PendingChangeView{ |
| ID: pc.ID, Kind: pc.Kind, Summary: pendingSummary(pc), |
| - Confirmed: len(pc.Confirms), Needed: pc.Quorum, |
| + Confirmed: confirmed, Needed: needed, |
| } |
| } |
| return out |
diff --git a/internal/console/pipelinepage_test.go b/internal/console/pipelinepage_test.go| index fc6e2fa44..d97816abe 100644 |
| --- a/internal/console/pipelinepage_test.go |
| +++ b/internal/console/pipelinepage_test.go |
| @@ -43,11 +43,20 @@ func TestOneChannelAppHidesChannelVocabulary(t *testing.T) { |
| t.Errorf("one-channel app page does not show the live marker:\n%s", body) |
| } |
| - // Ship a build and land it on the entry channel — still no vocabulary, |
| - // now that there's actually something to promote. |
| + // Ship TWO builds and land both on the entry channel — still no |
| + // vocabulary. Two promotions, not one, deliberately: the second gives the |
| + // pointer a history, which is the state in which the rollback control |
| + // renders (PR #258 review, I2). Tested with one promotion, this assertion |
| + // passed only because the page had no rollback form to put a channel name |
| + // into, so it never exercised the page a real one-channel app shows. |
| shipVersion(t, fx, "solo", "v1") |
| + shipVersion(t, fx, "solo", "v2") |
| seedChannel(t, fx, "solo", "edge", "v1") |
| + seedChannel(t, fx, "solo", "edge", "v2") |
| _, _, body2 := get(t, c, fx.u("/apps/solo/releases")) |
| + if !strings.Contains(body2, "Roll back") { |
| + t.Fatalf("this app has a rollback control by now — without it the check below proves nothing:\n%s", body2) |
| + } |
| assertNoChannelVocabulary(t, body2) |
| } |
| @@ -57,15 +66,116 @@ func TestOneChannelAppHidesChannelVocabulary(t *testing.T) { |
| // "top") names a `.ladder` CSS class and a code comment describing it, on |
| // EVERY page regardless of this app's pipeline; that is chrome, not content, |
| // and is not what spec §5's "hide the vocabulary entirely" is about. |
| +// |
| +// It checks the VISIBLE-TEXT projection, not the markup: spec §5 hides the |
| +// vocabulary from the READER, and a form field the browser never renders is |
| +// not something anybody is shown. The rollback control has to name the |
| +// channel it posts to somewhere — that is the door's parameter, not a word on |
| +// the page — and a raw-markup check would force the page to choose between |
| +// showing a rollback button and passing this test (PR #258 review, I2). |
| func assertNoChannelVocabulary(t *testing.T, body string) { |
| t.Helper() |
| _, main, ok := strings.Cut(body, "<main>") |
| if !ok { |
| t.Fatalf("page has no <main> to scope the check to:\n%s", body) |
| } |
| + text := visibleText(main) |
| for _, forbidden := range []string{"Promote", "edge", "ladder"} { |
| - if strings.Contains(main, forbidden) { |
| - t.Errorf("one-channel app page contains %q, want the vocabulary hidden entirely:\n%s", forbidden, main) |
| + if strings.Contains(text, forbidden) { |
| + t.Errorf("one-channel app page SHOWS %q, want the vocabulary hidden entirely.\nvisible text:\n%s", forbidden, text) |
| + } |
| + } |
| +} |
| + |
| +// visibleText is the crude tag stripper the assertion above needs: everything |
| +// outside <...> and nothing inside one, so attributes, hidden input values and |
| +// form actions drop out while every word a reader can actually see stays. Good |
| +// enough for a rendered console page (no CDATA, no raw "<" in text — the |
| +// templates escape it), and deliberately not a parser: a dependency-free |
| +// assertion helper that anybody can check by eye is worth more here than |
| +// fidelity to malformed markup. |
| +// |
| +// Known blind spot, named so it is a choice rather than an oversight: text |
| +// that reaches a reader THROUGH an attribute — aria-label, alt, title, |
| +// placeholder — is dropped with the rest of the tag, so vocabulary leaking |
| +// via one of those (most seriously to a screen reader, which is a reader) is |
| +// invisible to this check. If a one-channel surface ever grows such a label, |
| +// it wants asserting directly rather than trusting this projection. |
| +func visibleText(html string) string { |
| + var b strings.Builder |
| + inTag := false |
| + for _, r := range html { |
| + switch { |
| + case r == '<': |
| + inTag = true |
| + case r == '>': |
| + inTag = false |
| + case !inTag: |
| + b.WriteRune(r) |
| + } |
| + } |
| + return b.String() |
| +} |
| + |
| +// TestOneChannelAppKeepsCanariesAndRollback pins the other half of "hide the |
| +// vocabulary": what must NOT be hidden with it. The one-channel guard wrapped |
| +// the whole details block on the Releases tab, so opting into the default |
| +// one-channel pipeline also took away the canary table, the create-canary |
| +// form and the rollback control — none of which is channel vocabulary. |
| +// canary/* is "always allowed" and lives outside the pipeline entirely (spec |
| +// §2), and a rollback is a pointer write every app can need; a one-channel |
| +// app is precisely the app with no promote step to fall back on if a bad |
| +// build lands. The ladder, the promote form and the channel table stay gone. |
| +func TestOneChannelAppKeepsCanariesAndRollback(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-solo-canary") |
| + if status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/new", |
| + map[string]string{"name": "solocan"}, token); status != http.StatusOK { |
| + t.Fatalf("create app = %d %v", status, out) |
| + } |
| + shipVersion(t, fx, "solocan", "v1") |
| + shipVersion(t, fx, "solocan", "v2") |
| + // Two promotions onto the one channel: the second gives the pointer a |
| + // history, which is what a rollback needs to exist at all. |
| + seedChannel(t, fx, "solocan", "edge", "v1") |
| + seedChannel(t, fx, "solocan", "edge", "v2") |
| + seedChannel(t, fx, "solocan", "canary/rc1", "v1") |
| + |
| + c := signInPaul(t, fx) |
| + status, _, body := get(t, c, fx.u("/apps/solocan/releases")) |
| + if status != http.StatusOK { |
| + t.Fatalf("app page = %d", status) |
| + } |
| + _, main, ok := strings.Cut(body, "<main>") |
| + if !ok { |
| + t.Fatalf("page has no <main> to scope the check to:\n%s", body) |
| + } |
| + // The canary table, with the app's live canary in it. |
| + if !strings.Contains(main, "canary/rc1") { |
| + t.Errorf("one-channel app page hides its canary table:\n%s", main) |
| + } |
| + if !strings.Contains(main, `/apps/solocan/canary/retire"`) { |
| + t.Errorf("one-channel app page hides the canary retire control:\n%s", main) |
| + } |
| + if !strings.Contains(main, `/apps/solocan/canary"`) { |
| + t.Errorf("one-channel app page hides the create-canary form:\n%s", main) |
| + } |
| + // The rollback control. |
| + if !strings.Contains(main, `/apps/solocan/rollback"`) { |
| + t.Errorf("one-channel app page hides the rollback control:\n%s", main) |
| + } |
| + if !strings.Contains(main, "Roll back") { |
| + t.Errorf("one-channel app page has no rollback button:\n%s", main) |
| + } |
| + // And still none of the vocabulary the one-channel rule hides: no ladder |
| + // rungs, no promote form, no channel column. Checked on what the page |
| + // SHOWS (visibleText), the same projection assertNoChannelVocabulary |
| + // uses — the rollback form's hidden channel field is the door's |
| + // parameter, not a word on the page. |
| + text := visibleText(main) |
| + for _, forbidden := range []string{"beta", "stable", "Promoting", "Promote", "ladder"} { |
| + if strings.Contains(text, forbidden) { |
| + t.Errorf("one-channel app page SHOWS %q, want the vocabulary still hidden.\nvisible text:\n%s", forbidden, text) |
| } |
| } |
| } |
diff --git a/internal/console/server.go b/internal/console/server.go| index 23fa6fef6..781b9cb81 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -2460,8 +2460,10 @@ func (s *Server) handleApp(w http.ResponseWriter, r *http.Request) { |
| } |
| if pcs, err := s.store.PendingChangesFor(acct.ID, app, time.Now()); err != nil { |
| log.Printf("console: pending changes %s/%s: %v", acct.Sqid, app, err) |
| - } else { |
| - p.Pending = pendingChangeViews(pcs) |
| + } else if len(pcs) > 0 { |
| + // One pipeline read for the whole card: the rows show the quorum |
| + // EXECUTION will require, not the one they were proposed under. |
| + p.Pending = s.pendingChangeViews(pcs, s.pendingPipeline(r.Context(), acct, app)) |
| } |
| s.render(w, http.StatusOK, "app_overview", p) |
| } |
diff --git a/internal/console/templates/app_releases.html b/internal/console/templates/app_releases.html| index 2bda181c6..7e00f916e 100644 |
| --- a/internal/console/templates/app_releases.html |
| +++ b/internal/console/templates/app_releases.html |
| @@ -96,7 +96,14 @@ carlos ship --app {{.Detail.App}} --kind static --version v1 <site-dir></c |
| <p class="small">Releases appear here once shipped, and get promoted to channels from this page.</p> |
| {{end}} |
| -{{if not .Detail.OneChannelPipeline}} |
| +{{if .Detail.OneChannelPipeline}} |
| +<details class="reveal"> |
| +<summary>Test versions and rolling back</summary> |
| +{{template "solorollback" .}} |
| +{{template "canaries" .}} |
| +{{template "apptags" .}} |
| +</details> |
| +{{else}} |
| <details class="reveal"> |
| <summary>Release pipeline — channels, test versions, and the promotion ladder</summary> |
| {{template "ladder" .}} |
| @@ -131,7 +138,24 @@ carlos ship --app {{.Detail.App}} --kind static --version v1 <site-dir></c |
| <p class="small">Go-live times are estimates — each copy keeps its own clock. Rollbacks skip the delay.</p> |
| {{else}}<p class="small">No channels yet — ship a release, then promote it below to create the first one.</p>{{end}} |
| -<h2>Canaries</h2> |
| +{{template "canaries" .}} |
| +{{template "apptags" .}} |
| +</details> |
| +{{end}} |
| +{{if .Detail.OneChannelPipeline}} |
| +<p class="small">This app has one channel — <code>carlos deploy</code> ships straight to it, with no |
| +promotion step in between. <a href="{{.Nav}}/apps/{{.Detail.App}}/pipeline">Set up a pipeline</a> to |
| +add more.</p> |
| +{{end}} |
| +{{if not .CanPromote}}<p class="small">Deploys are unavailable on this console — it has no signing key.</p>{{end}} |
| +{{template "bottom" .}}{{end}} |
| + |
| +{{/* The canary section and the tag list are NOT channel vocabulary, so both |
| + app shapes get them: canary/* lives outside the pipeline entirely and is |
| + always allowed (release pipelines v2 §2), and a tag names a release, not |
| + a rung. Shared through one definition so the two shapes cannot drift into |
| + different canary controls. */}} |
| +{{define "canaries"}}<h2>Canaries</h2> |
| {{if .Detail.Canaries}} |
| <div class="tablewrap"> |
| <table> |
| @@ -177,9 +201,9 @@ carlos ship --app {{.Detail.App}} --kind static --version v1 <site-dir></c |
| {{else}}<pre><code>{{.AttachCmd}}</code></pre> |
| <p class="small">The address comes live within a couple of minutes.</p> |
| {{end}} |
| -{{end}} |
| +{{end}}{{end}} |
| -{{if .Tags}} |
| +{{define "apptags"}}{{if .Tags}} |
| <h2>Tags</h2> |
| {{range .Tags}} |
| <details> |
| @@ -187,13 +211,24 @@ carlos ship --app {{.Detail.App}} --kind static --version v1 <site-dir></c |
| <pre class="changelog">{{.Changelog}}</pre> |
| </details> |
| {{end}} |
| +{{end}}{{end}} |
| + |
| +{{/* The one-channel app's rollback control. A rollback is not channel |
| + vocabulary — it is the pointer write that puts the previous build back, |
| + and a one-channel app is exactly the app with no promotion step to fall |
| + back on when a bad build lands. It names no channel on screen (the name |
| + rides in the hidden field the door needs), draws no table and offers no |
| + promote; the ladder stays hidden. RollbackChannels is empty until the |
| + channel actually has somewhere to go back to. */}} |
| +{{define "solorollback"}}{{if .CanPromote}}{{with .Detail.RollbackChannels}} |
| +<h2>Rolling back</h2> |
| +<p class="small">Puts the previous build back in front of everyone straight away — rollbacks skip the safety delay. You'll be asked to confirm, and told which build it lands on.</p> |
| +{{range .}} |
| +<form method="post" action="{{$.Base}}/apps/{{$.Detail.App}}/rollback"> |
| + <input type="hidden" name="csrf" value="{{$.CSRF}}"> |
| + <input type="hidden" name="channel" value="{{.Name}}"> |
| + <input type="hidden" name="back" value="releases"> |
| + <button type="submit" class="btn-sm btn-danger">Roll back</button> |
| +</form> |
| {{end}} |
| -</details> |
| -{{end}} |
| -{{if .Detail.OneChannelPipeline}} |
| -<p class="small">This app has one channel — <code>carlos deploy</code> ships straight to it, with no |
| -promotion step in between. <a href="{{.Nav}}/apps/{{.Detail.App}}/pipeline">Set up a pipeline</a> to |
| -add more.</p> |
| -{{end}} |
| -{{if not .CanPromote}}<p class="small">Deploys are unavailable on this console — it has no signing key.</p>{{end}} |
| -{{template "bottom" .}}{{end}} |
| +{{end}}{{end}}{{end}} |
diff --git a/internal/console/templates/layout.html b/internal/console/templates/layout.html| index 2a36a9fa8..dd4ca0b4e 100644 |
| --- a/internal/console/templates/layout.html |
| +++ b/internal/console/templates/layout.html |
| @@ -429,6 +429,11 @@ |
| node's outline sitting on a filled body, invisible for a line that has |
| to be followed across the page. */ |
| .archsvg .archedge { fill: none; stroke: var(--muted); stroke-width: 1.5; } |
| + /* An alarm rides the node it watches (archlayout.go): a healthy one is a |
| + quiet mark, a firing one takes the bad colour so it reads at a glance |
| + rather than only in the words of its hover title. */ |
| + .archsvg .archalarm { fill: var(--good); } |
| + .archsvg .archalarm-firing { fill: var(--bad); } |
| .archbox { |
| border: 1px solid var(--rule); border-radius: 0.6rem; |
| padding: 0.9rem 1.1rem; margin: 0 0 0.6rem; max-width: 44rem; |
diff --git a/internal/console/view.go b/internal/console/view.go| index 0d8a116c3..c0c2a9926 100644 |
| --- a/internal/console/view.go |
| +++ b/internal/console/view.go |
| @@ -77,6 +77,35 @@ func (v AppView) OneChannelPipeline() bool { |
| return v.Pipeline != nil && len(v.Pipeline.Channels) == 1 |
| } |
| +// RollbackChannels is the app's OWN declared channels that have somewhere to |
| +// roll back to — for the one-channel page, which hides the channel table the |
| +// rollback control normally rides in but must not hide the control itself: a |
| +// rollback is a pointer write, not channel vocabulary, and a one-channel app |
| +// is precisely the app with no promotion step to fall back on when a bad |
| +// build lands (spec §5 hides the VOCABULARY, not the actions). |
| +// |
| +// Declared channels only: canary/* is retired, never rolled back (its control |
| +// is the canary table's Retire button), and a channel the pipeline does not |
| +// name is not this app's to offer here. A row that failed to read, or has no |
| +// history, is left out — there is nothing to go back to. |
| +func (v AppView) RollbackChannels() []ChannelView { |
| + if v.Pipeline == nil { |
| + return nil |
| + } |
| + declared := make(map[string]bool, len(v.Pipeline.Channels)) |
| + for _, c := range v.Pipeline.Channels { |
| + declared[c.Name] = true |
| + } |
| + var out []ChannelView |
| + for _, ch := range v.Channels { |
| + if ch.Err != "" || !ch.HasHistory || !declared[ch.Name] { |
| + continue |
| + } |
| + out = append(out, ch) |
| + } |
| + return out |
| +} |
| + |
| // MismatchedRoute is one route whose stamped account disagrees with the |
| // account whose page is being viewed: host plus where it actually reads |
| // releases from. The banner names these — a warning that says WHICH routes |
diff --git a/internal/mail/walk.go b/internal/mail/walk.go| index 687069906..cf2ada639 100644 |
| --- a/internal/mail/walk.go |
| +++ b/internal/mail/walk.go |
| @@ -227,9 +227,15 @@ func EnsureDomain(ctx context.Context, s release.Store, p Provisioner, w dnsrec. |
| // (this function's own doc comment on Deployment), and calling |
| // this with an empty name would either error at a real backend or |
| // silently bind to nothing. |
| - if err := p.SetIdentityConfigSet(ctx, d.Region, domain, configSet); err != nil { |
| - return nil, nil, fmt.Errorf("mail: bind identity %s to config set %s: %w", domain, configSet, err) |
| - } |
| + // |
| + // THE BIND ITSELF HAPPENS AT THE END OF THIS FUNCTION, not here, |
| + // and the gap is the whole point. SES authorises a configuration |
| + // set as a resource in its own right (#271), so an identity bound |
| + // to a set that the app's IAM policy does not name refuses every |
| + // send with a 554. Creating the set is harmless; binding it is not |
| + // — so the order has to be create, record, GRANT, then bind. Bind |
| + // here and there is a window, however short, in which this app |
| + // cannot send at all. |
| } |
| records := RecordSet(domain, d.Region, ident.DKIMTokens, d.DMARCRua) |
| @@ -274,9 +280,96 @@ func EnsureDomain(ctx context.Context, s release.Store, p Provisioner, w dnsrec. |
| if _, err := PutDomain(ctx, s, acct, rec, etag); err != nil { |
| return nil, nil, err |
| } |
| + |
| + // GRANT BEFORE BIND. Every app that sends on this domain — and every |
| + // standalone credential those apps hold — gets its policy recomputed |
| + // from the record just written, so the configuration-set ARN is in |
| + // place before anything is bound to it. |
| + // |
| + // This is not an optimisation, it is the difference between a repair |
| + // and an outage. EnsureUser is the only place a policy is written, and |
| + // it runs in Enable and CreateStandalone — never here. So before this, |
| + // an app enabled BEFORE a config set existed (every app on a |
| + // deployment that arms SNS later) got the set bound underneath it with |
| + // a policy that never mentioned it: 554 on every send. `carlos email |
| + // domains add` was the documented repair for missing events, and it |
| + // was also the thing that broke sending. |
| + if err := RefreshPolicies(ctx, s, p, acct, rec.Apps); err != nil { |
| + return nil, nil, err |
| + } |
| + if configSet != "" { |
| + if err := p.SetIdentityConfigSet(ctx, d.Region, domain, configSet); err != nil { |
| + return nil, nil, fmt.Errorf("mail: bind identity %s to config set %s: %w", domain, configSet, err) |
| + } |
| + } |
| return rec, pending, nil |
| } |
| +// RefreshPolicies rewrites the inline policy of every principal belonging to |
| +// each of apps, from the CURRENT granted domain set — the delivered |
| +// credential and every standalone one alike. |
| +// |
| +// It exists because the grant is a function of state that changes OUTSIDE |
| +// the walks that write it. Enable and CreateStandalone each recompute it for |
| +// one app at the moment they mint a key; nothing recomputed it when a |
| +// DOMAIN changed underneath an app that was already provisioned. Binding a |
| +// configuration set is exactly that kind of change, and so is a domain |
| +// joining or leaving the granted set. |
| +// |
| +// Silent on an app with nothing to grant: no record, no declaration, no |
| +// granted domains, or nothing provisioned yet all mean there is no principal |
| +// whose policy could be stale. An app that IS provisioned and whose grant |
| +// cannot be computed is an error, not a skip — that is a policy this |
| +// function was asked to refresh and could not. |
| +func RefreshPolicies(ctx context.Context, s release.Store, p Provisioner, acct string, apps []string) error { |
| + for _, app := range apps { |
| + rec, _, err := Get(ctx, s, acct, app) |
| + if err != nil { |
| + return err |
| + } |
| + if rec == nil || rec.Declared == nil { |
| + continue |
| + } |
| + if rec.Provisioned == nil && len(rec.Standalone) == 0 { |
| + continue |
| + } |
| + domains, err := grantedDomains(ctx, s, acct, app) |
| + if err != nil { |
| + return err |
| + } |
| + if len(domains) == 0 { |
| + continue |
| + } |
| + var arns, configSets, names []string |
| + for _, dom := range domains { |
| + arns = append(arns, dom.VerifiedARNs()...) |
| + configSets = append(configSets, dom.VerifiedConfigSetARNs()...) |
| + names = append(names, dom.Domain) |
| + } |
| + sort.Strings(arns) |
| + sort.Strings(configSets) |
| + sort.Strings(names) |
| + if len(arns) == 0 { |
| + // Granted domains exist but none is verified in any region, so |
| + // SendPolicyJSON would refuse anyway. Nothing to write. |
| + continue |
| + } |
| + if rec.Provisioned != nil { |
| + user := UserName(acct, app) |
| + if _, err := p.EnsureUser(ctx, user, UserPath, arns, configSets, names); err != nil { |
| + return fmt.Errorf("mail: refresh policy for %s: %w", user, err) |
| + } |
| + } |
| + for _, sa := range rec.Standalone { |
| + user := StandaloneUserName(acct, app, sa.Name) |
| + if _, err := p.EnsureUser(ctx, user, UserPath, arns, configSets, names); err != nil { |
| + return fmt.Errorf("mail: refresh policy for %s: %w", user, err) |
| + } |
| + } |
| + } |
| + return nil |
| +} |
| + |
| // ConfigSetName is the per-account SES configuration set that carries the |
| // event destination feeding bounce, complaint and send counts (spec §9). |
| func ConfigSetName(acct string) string { return "carlos-" + acct } |
diff --git a/internal/mail/walk_enable_test.go b/internal/mail/walk_enable_test.go| index 7977d968c..45561feb5 100644 |
| --- a/internal/mail/walk_enable_test.go |
| +++ b/internal/mail/walk_enable_test.go |
| @@ -142,6 +142,90 @@ func TestEnableDeliversTheDerivedPasswordNotTheSecret(t *testing.T) { |
| // A healthy provisioned app makes a re-run a no-op: no fresh key, no |
| // rewrite. Minting on every pass would burn IAM's two-key ceiling and |
| // churn every instance's config. |
| +// TestEnsureDomainRefreshesPoliciesWhenItBindsAConfigSet is the trap that |
| +// shipped in v0.12.0 and that #268's own advice walks a member straight into. |
| +// |
| +// EnsureDomain binds the identity to a configuration set. EnsureUser — the |
| +// only place a credential's policy is written — runs in Enable and |
| +// CreateStandalone, NOT here. So for an app enabled BEFORE a config set |
| +// existed (every app on a deployment that arms SNS later), `carlos email |
| +// domains add` bound the set and left the policy naming only the identity, |
| +// which SES answers with a 554 on every send. That is the #271 shape |
| +// arriving through the repair path instead of the provisioning one, and it |
| +// is strictly worse than the drift it was meant to fix: silent-no-events |
| +// becomes silent-no-sending. |
| +// |
| +// The fix is that binding and granting happen together. This test pins the |
| +// pairing rather than either half. |
| +func TestEnsureDomainRefreshesPoliciesWhenItBindsAConfigSet(t *testing.T) { |
| + ctx := context.Background() |
| + s, p := release.NewDirStore(t.TempDir()), NewMemProvisioner("1") |
| + now := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC) |
| + |
| + _, pubB64, err := appconfig.GenerateKey() |
| + if err != nil { |
| + t.Fatalf("GenerateKey: %v", err) |
| + } |
| + pub, err := appconfig.ParsePublicKey(pubB64, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePublicKey: %v", err) |
| + } |
| + |
| + // A deployment with NO SNS topic: the domain verifies and the app is |
| + // enabled with no configuration set anywhere — the ordinary shape |
| + // before event ingest is armed. |
| + noSNS := Deployment{Region: "eu-west-1", PlatformSuffix: "oncarlos.com"} |
| + if _, err := Declare(ctx, s, "bag", "seapointish", "SEAPOINTISH_SMTP", |
| + "noreply@seapointish.bag.oncarlos.com", "paul@rslw.com", now); err != nil { |
| + t.Fatalf("Declare: %v", err) |
| + } |
| + if _, _, err := EnsureDomain(ctx, s, p, dnsrec.NewMemWriter(), noSNS, |
| + "bag", "seapointish", "seapointish.bag.oncarlos.com", now); err != nil { |
| + t.Fatalf("EnsureDomain: %v", err) |
| + } |
| + p.SetIdentityStatus("eu-west-1", "seapointish.bag.oncarlos.com", |
| + Status{DKIM: "SUCCESS", MailFrom: "SUCCESS", Sending: "SUCCESS"}) |
| + if _, err := VerifyDomain(ctx, s, p, "bag", "seapointish.bag.oncarlos.com", "eu-west-1", now); err != nil { |
| + t.Fatalf("VerifyDomain: %v", err) |
| + } |
| + if _, err := Enable(ctx, s, p, pub, noSNS, "bag", "seapointish", now); err != nil { |
| + t.Fatalf("Enable: %v", err) |
| + } |
| + if _, err := CreateStandalone(ctx, s, p, noSNS, "bag", "seapointish", "smoke", "paul@rslw.com", now); err != nil { |
| + t.Fatalf("CreateStandalone: %v", err) |
| + } |
| + |
| + // Now the deployment arms SNS and the member follows the advice the |
| + // status warning prints. |
| + withSNS := testDeployment() |
| + if _, _, err := EnsureDomain(ctx, s, p, dnsrec.NewMemWriter(), withSNS, |
| + "bag", "seapointish", "seapointish.bag.oncarlos.com", now); err != nil { |
| + t.Fatalf("EnsureDomain (with SNS): %v", err) |
| + } |
| + |
| + dom, _, err := GetDomain(ctx, s, "bag", "seapointish.bag.oncarlos.com") |
| + if err != nil { |
| + t.Fatalf("GetDomain: %v", err) |
| + } |
| + want := dom.VerifiedConfigSetARNs() |
| + if len(want) == 0 { |
| + t.Fatal("no configuration set was bound, so this test proves nothing") |
| + } |
| + for _, u := range []string{UserName("bag", "seapointish"), StandaloneUserName("bag", "seapointish", "smoke")} { |
| + mu, ok := p.Users[u] |
| + if !ok { |
| + t.Fatalf("user %s does not exist", u) |
| + } |
| + for _, arn := range want { |
| + if !strings.Contains(mu.Policy, arn) { |
| + t.Errorf("%s: policy does not grant %s after the set was bound\n"+ |
| + "binding without refreshing the policy is a 554 on every send — worse than the drift it repairs\npolicy = %s", |
| + u, arn, mu.Policy) |
| + } |
| + } |
| + } |
| +} |
| + |
| // TestEnableGrantsTheConfigSetToTheDeliveredUser is the WALK half of issue |
| // #271, and it exists because mutation testing showed the policy-level tests |
| // could not see this: deleting the line in Enable that gathers config-set |
diff --git a/internal/release/adopt.go b/internal/release/adopt.go| index 718c6cee2..7e7d453b3 100644 |
| --- a/internal/release/adopt.go |
| +++ b/internal/release/adopt.go |
| @@ -348,9 +348,14 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551 |
| // name like edge its ZERO legacy window back). A channel with no memory |
| // keeps the fail-closed DefaultHolds resolution — the old-binary |
| // posture (spec §3's old-binary edge), whose production: true co-write |
| - // remains the mitigation for a MEMORY-LESS box only. The production- |
| - // flag machinery still applies to an errored app exactly as to any |
| - // legacy app. The error is NEVER folded into the ratchet as a nil: nil |
| + // remains the mitigation for a MEMORY-LESS box only. Where memory DOES |
| + // exist, the floor is enforcement in full: the bake VERDICT rides on |
| + // the same memory (bakeIgnore below never ignores an app whose ratchet |
| + // is still live — which is a WIDER condition than "the memory raises a |
| + // window", deliberately, since an app-defined channel's ratchet |
| + // normally raises nothing at all), so nothing here depends on the |
| + // console having managed the co-write. |
| + // The error is NEVER folded into the ratchet as a nil: nil |
| // means verified-absent only (ObservePipeline's contract), and a |
| // corrupt object must not masquerade as a deletion — a deletion RATCHETS |
| // each remembered channel down to its legacy default over one old-window |
| @@ -379,7 +384,7 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551 |
| // non-tampering trigger too: on a GetObject-without-ListBucket |
| // fleet a GET of an ABSENT key answers 403, not 404, and every |
| // app would land here every pass. |
| - fmt.Printf(" %s/%s: pipeline unreadable (%v) — legacy holds and the production flag apply\n", ak.acct, ak.app, perr) |
| + fmt.Printf(" %s/%s: pipeline unreadable (%v) — remembered windows still apply; anything unremembered falls to legacy holds\n", ak.acct, ak.app, perr) |
| } |
| } |
| @@ -429,13 +434,22 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551 |
| // the pipeline does take that 72h (spec §3: a channel with no pipeline |
| // entry keeps the fail-closed default), including rung names like beta |
| // whose DefaultHolds entry no longer applies. |
| + // |
| + // floored is the SEPARATE question the bake verdict asks: does this box |
| + // hold pipeline evidence for the app at all (see bakeIgnore)? It is a |
| + // membership set, never derived from the overlay above — a ratchet can |
| + // be live while raising nothing, which is the ordinary state of every |
| + // app-defined channel name (holdMemory.liveRatchet has the full |
| + // argument). |
| appHolds := map[acctApp]map[string]time.Duration{} |
| + floored := map[acctApp]bool{} |
| for ak, ps := range pipes { |
| switch { |
| case ps.err == nil && ps.p != nil: |
| h := bake.HoldsForApp(ak.acct, ak.app, ps.p) |
| h["canary/*"] = 0 |
| appHolds[ak] = h |
| + floored[ak] = true |
| default: |
| // The memory floor — for an ERRORED read and for VERIFIED |
| // ABSENCE alike. Overrides overlay a COPY of DefaultHolds, |
| @@ -449,16 +463,31 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551 |
| // account, an app name nor a non-canary channel can contain |
| // clockSep or "/" (channelClockKey's own injectivity argument). |
| // |
| - // For verified absence the floor is what makes the deletion |
| - // ratchet (ObservePipeline's p == nil branch, folded above) |
| - // ENFORCED: during the ratchet-down cycle a remembered channel's |
| - // window still exceeds its legacy default, so deleting the |
| - // object cannot adopt a held head this pass. Once the cycle |
| - // lands the memory at legacy, nothing exceeds the default, no |
| - // map is built, and the app is back on the bare-DefaultHolds |
| - // path. An app with no memory at all — every legacy app, every |
| - // pass — takes that bare path byte-identically, and the len |
| - // check keeps the fleet-wide no-pipeline case allocation-free. |
| + // For verified absence the floor is half of what makes the |
| + // deletion ratchet (ObservePipeline's p == nil branch, folded |
| + // above) ENFORCED: while a remembered channel's window still |
| + // exceeds its legacy default, deleting the object cannot adopt |
| + // a held head this pass. That covers the names whose legacy |
| + // default is NARROWER than what was enforced (edge, 0); the |
| + // other half — an app-defined name, whose legacy default is the |
| + // 72h fallback and therefore lands instantly, raising nothing — |
| + // is carried by the verdict alone (floored below, |
| + // holdMemory.DeletedAt). Once every remembered channel has |
| + // finished ratcheting, nothing exceeds the default, no map is |
| + // built, and the app is back on the bare-DefaultHolds path. An |
| + // app with no memory at all — every legacy app, every pass — |
| + // takes that bare path byte-identically, and the len check |
| + // keeps the fleet-wide no-pipeline case allocation-free. |
| + // |
| + // The overlay is NOT the pass's answer to "is this app |
| + // pipelined as far as this box knows" — floored below is, and |
| + // the two are deliberately separate. A raised window is only |
| + // one shape a live ratchet takes: an app-defined channel like |
| + // "production" resolves to HoldFor's 72h fail-closed default |
| + // under DefaultHolds, so a declared 24h ratchets DOWN to a |
| + // window BELOW legacy and raises nothing at all. Deriving the |
| + // verdict from this overlay would have found no evidence for |
| + // exactly that app and fallen to the production flag. |
| if len(bake.Holds) == 0 { |
| break |
| } |
| @@ -476,6 +505,9 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551 |
| if name == key || name == "" { |
| continue |
| } |
| + if mem.liveRatchet(name, bake.Jitter, now) { |
| + floored[ak] = true |
| + } |
| if mem.Window > HoldFor(DefaultHolds, name) { |
| if h == nil { |
| h = make(map[string]time.Duration, len(DefaultHolds)+1) |
| @@ -513,7 +545,8 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551 |
| // backoff window at worst, not one per tick. A PIPELINED app's held |
| // head pays the read as well even though its verdict never consults the |
| // flag (bakeIgnore below): the fold keeps the legacy latch fresh for |
| - // the day the pipeline is deleted and the app falls back to it. |
| + // the day the pipeline is deleted AND its deletion ratchet has landed, |
| + // which is when the app falls back to the flag for real. |
| type prodState struct { |
| flag *ProductionFlag |
| err error // non-nil: the object exists but would not read or verify |
| @@ -627,11 +660,37 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551 |
| ak := acctApp{RouteAccount(rt), rt.App} |
| // A PIPELINED app never consults the flag: its bake policy is the |
| // pipeline's alone (spec §3) — never ignore, and a zero-window |
| - // channel holds nothing anyway. The flag stays meaningful for |
| - // legacy apps, and for THIS app on a pass whose pipeline read |
| - // errored, which takes exactly that legacy verdict path (its |
| - // WINDOWS are memory-floored above; the verdict is unchanged). |
| - if ps := pipes[ak]; ps.err == nil && ps.p != nil { |
| + // channel holds nothing anyway. |
| + // |
| + // "Pipelined" here is every app this pass found pipeline EVIDENCE |
| + // for (floored), not only the apps whose read verified this pass: |
| + // a verified pipeline, or a ratchet this box is still running for |
| + // the app (holdMemory.liveRatchet — a window that differs from the |
| + // name's legacy resolution in EITHER direction, a pending |
| + // loosening, or a deletion whose cycle has not run out). That |
| + // memory IS the evidence the app was pipelined, and it is |
| + // deliberately NOT the appHolds overlay: the overlay only ever |
| + // RAISES a window above legacy, and an app-defined channel name |
| + // resolves to HoldFor's 72h fail-closed default, so a declared 24h |
| + // on "production" raises nothing while being the very policy the |
| + // hold exists to enforce. Reading the overlay collapsed exactly |
| + // that app (PR #258 review, C1). Riding the |
| + // verdict on the console's production: true CO-WRITE instead would |
| + // make enforcement depend on a bucket write this box can never |
| + // confirm happened: an app pipelined before the co-write existed, an |
| + // app whose flag was deleted, a direct-bucket pipeline write — each |
| + // leaves a flagless app whose held heads would adopt the instant one |
| + // GET of pipeline.json errored, or the instant the object was |
| + // deleted, defeating the very floor computed above. The co-write |
| + // keeps its own job, which was always the MEMORY-LESS box (spec §3's |
| + // old-binary edge — any box that never saw the pipeline). |
| + // |
| + // The flag path is left to apps with no pipeline signal at all — |
| + // every legacy app, byte-identically — and to a once-pipelined app |
| + // whose every remembered channel has finished ratcheting back to |
| + // its legacy default: nothing is live then, the set is empty for |
| + // the app, and it is a legacy app again in full. |
| + if floored[ak] { |
| return false |
| } |
| return !bake.ProductionBake(ak.acct, ak.app, prod[ak].err, bakeable[ak], DefaultHolds, now) |
| @@ -764,12 +823,15 @@ func RouteAccount(rt registry.Route) string { |
| // holds is the enforced per-channel window map the caller resolved for this |
| // route's app: a verified pipeline's ratcheted HoldsForApp map (canary/* |
| // stitched back in at zero), DefaultHolds floored by the ratchet memory for |
| -// an app whose pipeline read errored, else DefaultHolds — never consulted |
| -// app-blind. |
| +// an app whose pipeline read errored or is verified-absent mid-ratchet, else |
| +// DefaultHolds — never consulted app-blind. |
| // |
| // ignore is the app-level bake verdict from ProductionBake — the LEGACY |
| // production flag's doing, never a publisher field, and constant false for |
| -// a pipelined app, whose windows come from holds alone (spec §3). True |
| +// any app this box carries pipeline evidence for: a verified pipeline, or a |
| +// ratchet this box is still running for it (holdMemory.liveRatchet, which is |
| +// NOT the same question as "does the memory raise a window" — see |
| +// bakeIgnore). Such an app's windows come from holds alone (spec §3). True |
| // means the supply-chain window does not CURRENTLY apply to this app on |
| // this box (not declared production, or declared off and past the ratchet); |
| // the caller computes it once per route from the flag it read this pass. |
diff --git a/internal/release/adopt_test.go b/internal/release/adopt_test.go| index 0268cd28b..43e94c7d7 100644 |
| --- a/internal/release/adopt_test.go |
| +++ b/internal/release/adopt_test.go |
| @@ -3095,6 +3095,384 @@ func TestPassDeletedPipelineRatchetsDown(t *testing.T) { |
| } |
| } |
| +// ratchetedProductionApp arranges the case the whole app-defined-channel |
| +// posture turns on, and the one the edge-named tests below cannot reach: a |
| +// FLAGLESS app whose pipeline names a channel OUTSIDE the frozen vocabulary |
| +// ("production"), with a bake SHORTER than that name's legacy resolution. |
| +// HoldFor's fallback for an unrecognised name is the 72h fail-closed default, |
| +// so the declared 24h is a LOOSENING that ratchets in over the old window — |
| +// and once it has, the box enforces 24h while DefaultHolds still resolves the |
| +// same name at 72h. Any "is this app pipelined" test phrased as "does the |
| +// memory RAISE a window above legacy" answers NO here, which is exactly the |
| +// shape that made an app-defined channel the one place the verdict collapsed. |
| +// |
| +// Leaves: v1 adopted, the memory ratcheted in at 24h with nothing pending, |
| +// and v2 at the production head INSIDE that 24h window — held, on the |
| +// pipeline's word alone, with no production flag anywhere. |
| +func ratchetedProductionApp(t *testing.T, s Store, ctx context.Context, priv ed25519.PrivateKey, pub ed25519.PublicKey, opt string, reg *registry.Registry) { |
| + t.Helper() |
| + if _, err := SetPipeline(ctx, s, priv, "bab", "myapp", []PipelineChannel{ |
| + {Name: "edge"}, {Name: "production", Bake: "24h"}, |
| + }, "test"); err != nil { |
| + t.Fatalf("SetPipeline: %v", err) |
| + } |
| + shipRelease(t, s, "bab", "myapp", "v1", adoptPlatform, []byte("bin one")) |
| + if err := Promote(ctx, s, priv, "bab", "myapp", "edge", "v1"); err != nil { |
| + t.Fatalf("Promote edge: %v", err) |
| + } |
| + if err := Promote(ctx, s, priv, "bab", "myapp", "production", "v1"); err != nil { |
| + t.Fatalf("Promote production: %v", err) |
| + } |
| + addRoute(t, reg, "p.example.com", "myapp", "production", false) |
| + |
| + // Pass 1 latches the 72h fail-closed baseline and parks the declared 24h |
| + // as pending — held. |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass 1: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "p.example.com"); v != "" { |
| + t.Fatalf("live = %q, want held at the 72h baseline", v) |
| + } |
| + // Elapse the OLD window on the box's own clock so the 24h lands, and age |
| + // v1 past 24h (but not past 72h) so adoption proves WHICH window governs. |
| + key := channelClockKey("bab", "myapp", "production") |
| + st := LoadFirstSeen(FirstSeenPath(opt)) |
| + mem := st.Holds[key] |
| + if mem == nil || mem.Window != 72*time.Hour || mem.Pending != 24*time.Hour { |
| + t.Fatalf("hold memory = %+v, want the 72h baseline with 24h pending", mem) |
| + } |
| + mem.PendingAt = mem.PendingAt.Add(-91 * time.Hour) |
| + if seen, ok := st.Channels[key]["v1"]; !ok { |
| + t.Fatalf("no first sighting recorded for v1") |
| + } else { |
| + st.Channels[key]["v1"] = seen.Add(-31 * time.Hour) |
| + } |
| + if err := st.Save(); err != nil { |
| + t.Fatalf("Save backdated clock: %v", err) |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass 2: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "p.example.com"); v != "v1" { |
| + t.Fatalf("live = %q, want v1 — the ratcheted-in 24h elapsed", v) |
| + } |
| + if mem := LoadFirstSeen(FirstSeenPath(opt)).Holds[key]; mem == nil || mem.Window != 24*time.Hour || !mem.PendingAt.IsZero() { |
| + t.Fatalf("hold memory = %+v, want the landed 24h with nothing pending", mem) |
| + } |
| + // A fresh head inside the ratcheted window: held while the pipeline reads. |
| + shipRelease(t, s, "bab", "myapp", "v2", adoptPlatform, []byte("bin two")) |
| + if err := Promote(ctx, s, priv, "bab", "myapp", "production", "v2"); err != nil { |
| + t.Fatalf("Promote v2: %v", err) |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass 3: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "p.example.com"); v != "v1" { |
| + t.Fatalf("live = %q, want still v1 — v2 is inside the ratcheted 24h window", v) |
| + } |
| +} |
| + |
| +// TestPassErroredReadOnAppDefinedChannelHoldsWithoutFlag is the C1 probe: the |
| +// same collapse the edge-named test below covers, on the naming that actually |
| +// hides it. With the memory ratcheted DOWN to 24h against a 72h legacy |
| +// resolution, no window in the map is raised above legacy — so a verdict |
| +// derived from the OVERLAY finds nothing, falls to the production flag this |
| +// app has never had, and adopts v2 the moment one GET of pipeline.json errors. |
| +// The evidence has to be the ratchet's own liveness, not the overlay. |
| +func TestPassErroredReadOnAppDefinedChannelHoldsWithoutFlag(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt := t.TempDir() |
| + reg := testRegistry(t) |
| + ratchetedProductionApp(t, s, ctx, priv, pub, opt, reg) |
| + |
| + _, etag, err := s.Get(ctx, PipelineKey("bab", "myapp")) |
| + if err != nil { |
| + t.Fatalf("read pipeline etag: %v", err) |
| + } |
| + if _, err := s.Put(ctx, PipelineKey("bab", "myapp"), []byte("not a wrapper"), etag); err != nil { |
| + t.Fatalf("corrupt the pipeline object: %v", err) |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "p.example.com"); v != "v1" { |
| + t.Fatalf("live = %q, want still v1 — an errored read on an app-defined channel must keep the hold, with no production flag in sight", v) |
| + } |
| +} |
| + |
| +// TestPassDeletedPipelineOnAppDefinedChannelHoldsWithoutFlag: the same probe |
| +// on the other keyless bucket write. Deleting pipeline.json hands the channel |
| +// its legacy 72h back — LONGER than the 24h the box was enforcing, so the |
| +// window itself never loosens — but the app must not fall to the flagless |
| +// legacy VERDICT in the same pass either, or one DELETE adopts a build that |
| +// was minutes old. It returns to the legacy path only once the deletion |
| +// ratchet has run the old window out on the box's own clock. |
| +func TestPassDeletedPipelineOnAppDefinedChannelHoldsWithoutFlag(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt := t.TempDir() |
| + reg := testRegistry(t) |
| + ratchetedProductionApp(t, s, ctx, priv, pub, opt, reg) |
| + |
| + if err := s.Delete(ctx, PipelineKey("bab", "myapp")); err != nil { |
| + t.Fatalf("delete pipeline: %v", err) |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "p.example.com"); v != "v1" { |
| + t.Fatalf("live = %q, want still v1 — deleting the pipeline must not adopt a held head in the same pass", v) |
| + } |
| + |
| + // And it does return to legacy: elapse the deletion ratchet's own cycle on |
| + // the box's clock, and the app is a legacy app again — no floor, no |
| + // pipeline evidence, the flagless legacy verdict, so v2 adopts. |
| + key := channelClockKey("bab", "myapp", "production") |
| + st := LoadFirstSeen(FirstSeenPath(opt)) |
| + mem := st.Holds[key] |
| + if mem == nil { |
| + t.Fatalf("no hold memory for the deleted pipeline's channel") |
| + } |
| + if mem.DeletedAt.IsZero() { |
| + t.Fatalf("hold memory = %+v, want the deletion stamped so its ratchet can run out", mem) |
| + } |
| + mem.DeletedAt = mem.DeletedAt.Add(-91 * time.Hour) |
| + st.Holds[key] = mem |
| + if err := st.Save(); err != nil { |
| + t.Fatalf("Save backdated clock: %v", err) |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "p.example.com"); v != "v2" { |
| + t.Fatalf("live = %q, want v2 — once the deletion ratchet has run out the app is legacy again, and a flagless legacy app holds nothing", v) |
| + } |
| +} |
| + |
| +// TestPassReplayedSubsetPipelineCannotPoisonTheDeletionStamp: the re-review |
| +// probe (PR #258, IMPORTANT-1). The deletion stamp is what carries the bake |
| +// verdict across a DELETE for an app-defined channel, so anything that can |
| +// leave a STALE stamp standing — one that expires unnoticed and then blocks |
| +// its own re-arming — is a way to strip the app of every ratchet it has. |
| +// |
| +// The chain, all of it keyless bucket writes plus one historical SIGNED |
| +// object anybody who ever saw the bucket can keep a copy of: |
| +// |
| +// 1. DELETE pipeline.json — every remembered channel is stamped. |
| +// 2. PUT BACK an OLD, still-validly-signed pipeline that names only a SUBSET |
| +// of the app's channels (here: edge, not production). There is no |
| +// object-level replay floor — only a per-channel SetAt one — so this is |
| +// accepted as a verified read. |
| +// 3. Dwell until the un-named channel's stale stamp has run out. |
| +// 4. DELETE again. A stamp is never restamped while non-zero, so production |
| +// is left with window == legacy, nothing pending and a dead stamp: no |
| +// live ratchet, no evidence, and the flagless legacy verdict adopts a |
| +// build that is minutes old. |
| +// |
| +// The fix is that a verified read clears the stamp for EVERY remembered |
| +// channel of the app, not only the ones the object names — so step 2 can |
| +// only ever DISARM the machinery, never leave a fuse burning behind it, and |
| +// step 4 stamps afresh. |
| +func TestPassReplayedSubsetPipelineCannotPoisonTheDeletionStamp(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt := t.TempDir() |
| + reg := testRegistry(t) |
| + |
| + // The historical object: a legitimately signed, edge-only pipeline from |
| + // before production existed. Kept aside, exactly as an attacker with a |
| + // copy of yesterday's bucket would. |
| + if _, err := SetPipeline(ctx, s, priv, "bab", "myapp", []PipelineChannel{{Name: "edge"}}, "test"); err != nil { |
| + t.Fatalf("SetPipeline (historical): %v", err) |
| + } |
| + historical, _, err := s.Get(ctx, PipelineKey("bab", "myapp")) |
| + if err != nil { |
| + t.Fatalf("read the historical pipeline: %v", err) |
| + } |
| + ratchetedProductionApp(t, s, ctx, priv, pub, opt, reg) |
| + |
| + // 1. Delete: production is stamped, and its held head stays held. |
| + if err := s.Delete(ctx, PipelineKey("bab", "myapp")); err != nil { |
| + t.Fatalf("delete pipeline: %v", err) |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass after delete: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "p.example.com"); v != "v1" { |
| + t.Fatalf("live = %q, want still v1 after the delete", v) |
| + } |
| + |
| + // 2. Replay the historical edge-only object (create-only PUT: the key is |
| + // absent after the delete, which is precisely the opening). |
| + if _, err := s.Put(ctx, PipelineKey("bab", "myapp"), historical, ""); err != nil { |
| + t.Fatalf("replay the historical pipeline: %v", err) |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass after the replay: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "p.example.com"); v != "v1" { |
| + t.Fatalf("live = %q, want still v1 — a verified pipeline is the app being pipelined, not a reason to adopt", v) |
| + } |
| + // The property that makes step 4 safe: the verified read left NO stamp |
| + // standing anywhere under the app, including on the channel it never |
| + // named. |
| + st := LoadFirstSeen(FirstSeenPath(opt)) |
| + for _, name := range []string{"edge", "production"} { |
| + if mem := st.Holds[channelClockKey("bab", "myapp", name)]; mem != nil && !mem.DeletedAt.IsZero() { |
| + t.Fatalf("%s keeps a deletion stamp (%v) after a verified read — a stale stamp can expire unnoticed and then refuse to re-arm", name, mem.DeletedAt) |
| + } |
| + } |
| + |
| + // 3. Dwell. Anything still stamped ages out; v2's own first sighting is |
| + // deliberately NOT aged, so the only thing that can adopt it below is the |
| + // verdict collapsing. |
| + st = LoadFirstSeen(FirstSeenPath(opt)) |
| + for _, name := range []string{"edge", "production"} { |
| + if mem := st.Holds[channelClockKey("bab", "myapp", name)]; mem != nil && !mem.DeletedAt.IsZero() { |
| + mem.DeletedAt = mem.DeletedAt.Add(-91 * time.Hour) |
| + } |
| + } |
| + if err := st.Save(); err != nil { |
| + t.Fatalf("Save dwelt clock: %v", err) |
| + } |
| + |
| + // 4. Delete again. The stamp re-arms, so the head is still held. |
| + if err := s.Delete(ctx, PipelineKey("bab", "myapp")); err != nil { |
| + t.Fatalf("second delete: %v", err) |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass after the second delete: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "p.example.com"); v != "v1" { |
| + t.Fatalf("live = %q, want STILL v1 — a replayed subset pipeline must not be able to strip the app of its deletion ratchet", v) |
| + } |
| +} |
| + |
| +// TestPassErroredPipelineHoldsWithoutProductionFlag: the memory floor is only |
| +// half a defence if the app's held head is then ADOPTED anyway. The bake |
| +// VERDICT ("does the window apply at all?") must ride on the same evidence |
| +// the WINDOWS do — the box's own ratchet memory — not on the production flag |
| +// the console co-writes. A co-write is a bucket write the box cannot verify |
| +// happened: an app pipelined before that co-write existed, an app whose flag |
| +// was deleted, a direct-bucket pipeline write — all leave a flagless app with |
| +// a live ratchet memory, and a legacy verdict would adopt its held head the |
| +// moment one GET of pipeline.json errored. NO production flag is set here. |
| +func TestPassErroredPipelineHoldsWithoutProductionFlag(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt := t.TempDir() |
| + reg := testRegistry(t) |
| + |
| + if _, err := SetPipeline(ctx, s, priv, "bab", "myapp", []PipelineChannel{ |
| + {Name: "edge", Bake: "24h"}, |
| + }, "test"); err != nil { |
| + t.Fatalf("SetPipeline: %v", err) |
| + } |
| + shipRelease(t, s, "bab", "myapp", "v1", adoptPlatform, []byte("bin")) |
| + if err := Promote(ctx, s, priv, "bab", "myapp", "edge", "v1"); err != nil { |
| + t.Fatalf("Promote: %v", err) |
| + } |
| + addRoute(t, reg, "e.example.com", "myapp", "edge", false) |
| + |
| + // Pass 1, pipeline verified: edge's zero baseline tightens to 24h |
| + // immediately (a tighten is never pending) — held, flag or no flag. |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "e.example.com"); v != "" { |
| + t.Fatalf("live = %q, want held — a pipelined app's window applies with no production flag at all", v) |
| + } |
| + |
| + // Corrupt the object in place: the next read ERRORS. |
| + _, etag, err := s.Get(ctx, PipelineKey("bab", "myapp")) |
| + if err != nil { |
| + t.Fatalf("read pipeline etag: %v", err) |
| + } |
| + if _, err := s.Put(ctx, PipelineKey("bab", "myapp"), []byte("not a wrapper"), etag); err != nil { |
| + t.Fatalf("corrupt the pipeline object: %v", err) |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "e.example.com"); v != "" { |
| + t.Fatalf("live = %q, want STILL held — the ratchet memory IS the evidence this app is pipelined; the verdict must not fall back to the production flag", v) |
| + } |
| + if mem := LoadFirstSeen(FirstSeenPath(opt)).Holds[channelClockKey("bab", "myapp", "edge")]; mem == nil || mem.Window != 24*time.Hour { |
| + t.Fatalf("hold memory = %+v, want the 24h floor still remembered — otherwise this test proves nothing", mem) |
| + } |
| +} |
| + |
| +// TestPassDeletedPipelineHoldsWithoutProductionFlag: same law on the |
| +// verified-absent path. Deleting pipeline.json is a plain bucket write; the |
| +// deletion ratchet keeps the 24h window enforced for one old-window cycle, |
| +// and the VERDICT must honour it without any production flag — otherwise |
| +// deleting one object adopts every held head of a flagless pipelined app in |
| +// the very next pass, which is precisely the collapse the ratchet exists to |
| +// prevent. |
| +func TestPassDeletedPipelineHoldsWithoutProductionFlag(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt := t.TempDir() |
| + reg := testRegistry(t) |
| + |
| + if _, err := SetPipeline(ctx, s, priv, "bab", "myapp", []PipelineChannel{ |
| + {Name: "edge", Bake: "24h"}, |
| + }, "test"); err != nil { |
| + t.Fatalf("SetPipeline: %v", err) |
| + } |
| + shipRelease(t, s, "bab", "myapp", "v1", adoptPlatform, []byte("bin")) |
| + if err := Promote(ctx, s, priv, "bab", "myapp", "edge", "v1"); err != nil { |
| + t.Fatalf("Promote: %v", err) |
| + } |
| + addRoute(t, reg, "e.example.com", "myapp", "edge", false) |
| + |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "e.example.com"); v != "" { |
| + t.Fatalf("live = %q, want held — the verified pipeline tightened edge to 24h", v) |
| + } |
| + |
| + if err := s.Delete(ctx, PipelineKey("bab", "myapp")); err != nil { |
| + t.Fatalf("delete pipeline: %v", err) |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "e.example.com"); v != "" { |
| + t.Fatalf("live = %q, want STILL held — deletion ratchets the window down over a cycle, and the verdict must ride on that memory, not on a production flag nobody set", v) |
| + } |
| + key := channelClockKey("bab", "myapp", "edge") |
| + if mem := LoadFirstSeen(FirstSeenPath(opt)).Holds[key]; mem == nil || mem.Window != 24*time.Hour || mem.PendingAt.IsZero() { |
| + t.Fatalf("hold memory = %+v, want 24h enforced with the legacy 0 pending", mem) |
| + } |
| + |
| + // And the ratchet still LANDS: once the old window has elapsed on the |
| + // box's own clock the app is legacy again — memory at the legacy default, |
| + // no floor map, and the bare-DefaultHolds verdict adopts. The fix must not |
| + // pin a once-pipelined app to a hold forever. |
| + st := LoadFirstSeen(FirstSeenPath(opt)) |
| + st.Holds[key].PendingAt = st.Holds[key].PendingAt.Add(-31 * time.Hour) |
| + if seen, ok := st.Channels[key]["v1"]; ok { |
| + st.Channels[key]["v1"] = seen.Add(-31 * time.Hour) |
| + } |
| + if err := st.Save(); err != nil { |
| + t.Fatalf("Save backdated clock: %v", err) |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, "e.example.com"); v != "v1" { |
| + t.Fatalf("live = %q, want v1 — the old window elapsed, so legacy holds (edge 0) resume and the legacy verdict adopts", v) |
| + } |
| +} |
| + |
| // TestPassNoPipelineNoMemoryStaysLegacy pins the other half of the deletion |
| // ratchet's contract: an app with NO pipeline and NO ratchet memory — every |
| // legacy app on every pass — keeps the bare DefaultHolds path byte-identically. |
diff --git a/internal/release/bake.go b/internal/release/bake.go| index 9372889c4..4964572ac 100644 |
| --- a/internal/release/bake.go |
| +++ b/internal/release/bake.go |
| @@ -120,13 +120,19 @@ type FirstSeen struct { |
| // |
| // ADDITIVE, but only one pass deep, same as Production above: an OLD |
| // binary's Save re-marshals a struct with no Holds field, which DELETES |
| - // every app's ratchet memory from the shared file. That is not silently |
| - // unsafe: the next new-binary pass re-latches every channel from |
| - // HoldFor(DefaultHolds, name) — the fail-closed baseline — and any |
| - // off-ratchet in flight simply restarts its clock from that pass's |
| - // sighting. A restarted clock only ever DELAYS a loosening; it can |
| - // never let one through early, so a mixed-version rollout costs at most |
| - // one pass of this memory in the closed direction, same as Production. |
| + // every app's ratchet memory from the shared file. For the WINDOW that |
| + // is not silently unsafe: the next new-binary pass re-latches every |
| + // channel from HoldFor(DefaultHolds, name) — the fail-closed baseline — |
| + // and any off-ratchet in flight simply restarts its clock from that |
| + // pass's sighting. A restarted clock only ever DELAYS a loosening; it |
| + // can never let one through early. |
| + // |
| + // That closed-direction argument covers Window/Pending/PendingAt and NOT |
| + // holdMemory.DeletedAt, which fails OPEN when dropped (a deleted-pipeline |
| + // app loses the only evidence it was ever pipelined, and the flagless |
| + // legacy verdict adopts). See that field's own comment — including why |
| + // today's exposure is a rollback within this branch rather than a fleet |
| + // property. |
| Holds map[string]*holdMemory `json:"holds,omitempty"` |
| path string // where Save persists; set by LoadFirstSeen |
| @@ -534,6 +540,96 @@ type holdMemory struct { |
| // unchanged must not push this forward, or the clock would never |
| // elapse against a fleet that polls faster than the window itself. |
| PendingAt time.Time `json:"pending_at,omitzero"` |
| + // DeletedAt is this box's own first sighting of the pipeline's ABSENCE |
| + // for this channel — additive, and stamped only on the p == nil path. |
| + // |
| + // It exists because the pending machinery cannot carry the deletion |
| + // ratchet on its own for every channel name. Deletion re-declares the |
| + // name's LEGACY default, and for an app-defined name that default is |
| + // HoldFor's 72h fail-closed fallback — LONGER than the window such a |
| + // channel usually enforces, so it lands instantly (a longer bake always |
| + // does) and stamps no Pending at all. The window is then perfectly safe |
| + // and the memory looks, to any reader, exactly like a legacy app's: |
| + // nothing pending, window == legacy. Without this stamp the adopt pass |
| + // would have no evidence that this app was pipelined ninety seconds ago, |
| + // would take the flagless legacy VERDICT, and one keyless DELETE would |
| + // adopt a build minutes old — collapsing the very protection the ratchet |
| + // exists to give (spec §3: "deletion never collapses a window |
| + // instantly"). Re-observing absence must not restart it, exactly as |
| + // PendingAt must not. |
| + // |
| + // It means "absence has been seen since the last VERIFIED read", which is |
| + // a fact about the app, not about one channel: a verified pipeline clears |
| + // it for every remembered channel under the app's prefix, including the |
| + // ones that object does not name. Clearing only the named channels was |
| + // poisonable — see the sweep in ObservePipeline's p != nil path. |
| + // |
| + // ADDITIVE, and one pass deep like every other field here — but NOT in |
| + // the same direction, which is worth saying plainly. Holds's own caveat |
| + // argues that a dropped ratchet memory restarts closed; that argument |
| + // does not extend to this field. An old binary's Save drops DeletedAt |
| + // along with the rest of Holds, and a dropped stamp does not delay a |
| + // loosening — it removes the only evidence the adopt pass has that a |
| + // deleted-pipeline app was ever pipelined, so floored comes up empty and |
| + // the flagless legacy verdict adopts. That is FAIL OPEN, for the window |
| + // between an old binary's Save and the next verified read (which, for a |
| + // deleted pipeline, never comes). |
| + // |
| + // The exposure is bounded by deployment reality, not by the design: |
| + // nothing in the fleet carries a Holds map at all yet — pipelines v2 has |
| + // not shipped — so the only binary that could drop it is a rollback |
| + // WITHIN this branch. If that stops being true before this ships, the |
| + // mitigation is the same one spec §3 gives for the old-binary edge (the |
| + // production: true co-write), and it wants saying in the spec rather |
| + // than assumed here. |
| + DeletedAt time.Time `json:"deleted_at,omitzero"` |
| +} |
| + |
| +// liveRatchet reports whether this box is still running a pipeline ratchet |
| +// for the channel — the adopt pass's evidence that the app IS pipelined as |
| +// far as this box knows, independent of whether the memory happens to RAISE |
| +// the window above legacy (an app-defined name's legacy resolution is 72h, |
| +// so an ordinary 24h production channel raises nothing at all). |
| +// |
| +// Three shapes, any of which is live: |
| +// - the enforced window differs from the name's legacy resolution, in |
| +// EITHER direction: something other than the legacy ladder set it. |
| +// - a pending loosening is waiting out its old window. |
| +// - a deletion is still running its cycle: the window now ENFORCED has not |
| +// yet elapsed on this box's own clock since the absence was first seen. |
| +// |
| +// Measuring that last cycle against the CURRENTLY enforced window is exact, |
| +// not an approximation, because the two deletion shapes divide cleanly: |
| +// |
| +// - the legacy default is LONGER (every app-defined name, whose legacy |
| +// resolution is HoldFor's 72h fallback): it lands immediately, stamps no |
| +// Pending, and the enforced window is now that longer one — so the cycle |
| +// served here is at least the window it replaced. |
| +// - the legacy default is SHORTER (edge, 0): it goes PENDING instead, and |
| +// the pending machinery runs the FULL old window before landing. By the |
| +// time the enforced window drops to the shorter value, the cycle this |
| +// clause would ask for has already been served in full — which is why a |
| +// landed deletion on a zero-window channel is over the moment it lands, |
| +// and why that is not a hole. |
| +// |
| +// The first two conditions cover the whole of the pending case anyway |
| +// (Window != legacy, then PendingAt non-zero), so nothing is resting on this |
| +// clause while a pending is in flight. |
| +// |
| +// name is the channel's own name, jitter and now the pass's (FirstSeen.Jitter |
| +// and its single stamped instant), so every hold on the box answers the same |
| +// clock. |
| +func (mem *holdMemory) liveRatchet(name string, jitter float64, now time.Time) bool { |
| + if mem == nil { |
| + return false |
| + } |
| + if mem.Window != HoldFor(DefaultHolds, name) || !mem.PendingAt.IsZero() { |
| + return true |
| + } |
| + if mem.DeletedAt.IsZero() { |
| + return false |
| + } |
| + return now.Before(mem.DeletedAt.Add(time.Duration(float64(mem.Window) * (1 + jitter)))) |
| } |
| // holdKeyPrefix is channelClockKey's key shape without the channel segment, |
| @@ -631,6 +727,19 @@ func (f *FirstSeen) ObservePipeline(acct, app string, p *Pipeline, now time.Time |
| // is deliberately left standing, so a replayed pipeline object |
| // older than the one that was deleted still cannot tick the |
| // memory later. |
| + // Stamp the absence itself, once. The window machinery below |
| + // cannot express "this app was pipelined until just now" for a |
| + // channel whose legacy default is LONGER than what it was |
| + // enforcing (every app-defined name, whose legacy resolution is |
| + // the 72h fallback): that lands instantly and leaves a memory |
| + // indistinguishable from a legacy app's. DeletedAt is what the |
| + // adopt pass's verdict rides on for one cycle afterwards — see |
| + // holdMemory.DeletedAt and liveRatchet. Never restarted by |
| + // re-observing absence, same rule as PendingAt. |
| + if mem.DeletedAt.IsZero() { |
| + mem.DeletedAt = now.UTC().Truncate(time.Second) |
| + changed = true |
| + } |
| w := HoldFor(DefaultHolds, name) |
| if w >= mem.Window { |
| if mem.Window != w || !mem.PendingAt.IsZero() { |
| @@ -652,6 +761,31 @@ func (f *FirstSeen) ObservePipeline(acct, app string, p *Pipeline, now time.Time |
| } |
| return changed |
| } |
| + // A verified pipeline ends every deletion cycle this app is carrying — |
| + // EVERY remembered channel under its prefix, not only the ones this |
| + // object happens to name. The stamp means "absence has been seen since |
| + // the last verified read", which is a fact about the APP, so it is |
| + // cleared the same way the p == nil branch stamps it: by prefix. |
| + // |
| + // Per-channel clearing was poisonable (PR #258 re-review). There is no |
| + // object-level replay floor — only a per-channel SetAt one — so a |
| + // HISTORICAL signed pipeline naming a SUBSET of the app's channels can be |
| + // PUT back: delete (every channel stamped), replay an old edge-only |
| + // object (edge cleared, production's stamp left standing while the app |
| + // reads as verifiably pipelined), dwell past production's cycle (the |
| + // stale stamp expires unnoticed), delete again (already non-zero, so |
| + // never restamped). The app then carries no live ratchet at all and its |
| + // held heads adopt with no bake — a keyless bucket write reaching exactly |
| + // the outcome the ratchet exists to prevent. |
| + if prefix := holdKeyPrefix(acct, app); len(f.Holds) > 0 { |
| + for key, mem := range f.Holds { |
| + if mem == nil || !strings.HasPrefix(key, prefix) || mem.DeletedAt.IsZero() { |
| + continue |
| + } |
| + mem.DeletedAt = time.Time{} |
| + changed = true |
| + } |
| + } |
| for _, ch := range p.Channels { |
| key := channelClockKey(acct, app, ch.Name) |
| mem := f.Holds[key] |
diff --git a/internal/release/bake_test.go b/internal/release/bake_test.go| index 2f39f2a4d..97280d87b 100644 |
| --- a/internal/release/bake_test.go |
| +++ b/internal/release/bake_test.go |
| @@ -766,15 +766,26 @@ func TestObservePipelineChangedContract(t *testing.T) { |
| } |
| }) |
| - t.Run("p == nil with windows already at legacy reports no change", func(t *testing.T) { |
| + t.Run("p == nil stamps the deletion once, then reports no change", func(t *testing.T) { |
| f := &FirstSeen{Jitter: 0} |
| // 72h == stable's legacy default: tightens to exactly the baseline, |
| - // nothing pending — so absence has nothing to ratchet down and must |
| - // report no change on every subsequent pass. |
| + // nothing pending — so absence has no WINDOW to ratchet down. The |
| + // FIRST absence is still a change: it stamps DeletedAt, which is the |
| + // box's own record that this app was pipelined until just now, and |
| + // which the adopt pass's bake verdict rides on for one cycle (PR #258 |
| + // review, C1 — for an app-defined channel name the legacy default is |
| + // LONGER than the enforced window, so deletion lands instantly and |
| + // this stamp is the only evidence left). A stamp that is not |
| + // persisted is no evidence at all, so it must report changed. |
| p := &Pipeline{Schema: 1, Channels: []PipelineChannel{{Name: "stable", Bake: "72h"}}, SetAt: now} |
| f.ObservePipeline("bae", "web", p, now) |
| - if f.ObservePipeline("bae", "web", nil, now.Add(time.Hour)) { |
| - t.Fatal("p == nil with windows already at legacy reported changed") |
| + if !f.ObservePipeline("bae", "web", nil, now.Add(time.Hour)) { |
| + t.Fatal("the first p == nil did not report changed — the deletion stamp would never be saved") |
| + } |
| + // Every pass after it is the steady state a fleet actually sits in, |
| + // and must not force a Save: re-observed absence changes nothing. |
| + if f.ObservePipeline("bae", "web", nil, now.Add(2*time.Hour)) { |
| + t.Fatal("re-observed absence reported changed — the stamp must not be restamped") |
| } |
| }) |
| @@ -878,6 +889,80 @@ func TestHoldMemoryDeletionTightensBelowLegacyImmediately(t *testing.T) { |
| } |
| } |
| +// TestLiveRatchetIsWiderThanARaisedWindow pins the distinction PR #258's |
| +// review turned on (C1): "is this box still running a pipeline ratchet for |
| +// this channel" is NOT "does the memory raise the window above legacy". An |
| +// app-defined name resolves to HoldFor's 72h fallback, so an ordinary |
| +// production channel enforcing 24h raises nothing while being fully |
| +// pipelined — and after a deletion its window lands on that same 72h |
| +// instantly, leaving a memory that looks exactly like a legacy app's. The |
| +// DeletedAt stamp is what carries the verdict across that gap, and it runs |
| +// out. |
| +func TestLiveRatchetIsWiderThanARaisedWindow(t *testing.T) { |
| + now := time.Date(2026, 8, 22, 12, 0, 0, 0, time.UTC) |
| + legacyProduction := 72 * time.Hour // HoldFor's unknown-name fallback |
| + |
| + // Ratcheted DOWN below legacy: raises nothing, still live. |
| + mem := &holdMemory{Window: 24 * time.Hour, SetAt: now} |
| + if !mem.liveRatchet("production", 0, now) { |
| + t.Error("a window ratcheted BELOW legacy did not read as a live ratchet — this is the C1 collapse") |
| + } |
| + // Raised above legacy: live, the case that always worked. |
| + if !(&holdMemory{Window: 24 * time.Hour}).liveRatchet("edge", 0, now) { |
| + t.Error("a window raised above legacy did not read as a live ratchet") |
| + } |
| + // Exactly legacy with a pending loosening: live. |
| + if !(&holdMemory{Window: legacyProduction, Pending: time.Hour, PendingAt: now}).liveRatchet("production", 0, now) { |
| + t.Error("a pending loosening did not read as a live ratchet") |
| + } |
| + // Exactly legacy, nothing pending, never deleted: a legacy app's memory. |
| + if (&holdMemory{Window: legacyProduction, SetAt: now}).liveRatchet("production", 0, now) { |
| + t.Error("a memory sitting at legacy with nothing in flight read as a live ratchet") |
| + } |
| + // Deleted, cycle still running / run out. |
| + deleted := &holdMemory{Window: legacyProduction, SetAt: now, DeletedAt: now} |
| + if !deleted.liveRatchet("production", 0, now.Add(71*time.Hour)) { |
| + t.Error("a deletion inside its own cycle did not read as a live ratchet") |
| + } |
| + if deleted.liveRatchet("production", 0, now.Add(73*time.Hour)) { |
| + t.Error("a deletion whose cycle has run out still read as live — the app must return to legacy") |
| + } |
| + // nil is never live (the {"holds":{"x":null}} shape). |
| + if (*holdMemory)(nil).liveRatchet("production", 0, now) { |
| + t.Error("a nil memory read as a live ratchet") |
| + } |
| +} |
| + |
| +// TestHoldMemoryDeletionStampClearedByAVerifiedPipeline: the stamp says "this |
| +// app was pipelined until just now". A verified pipeline naming the channel |
| +// says it IS pipelined, on the object's own word, so the stamp has no job |
| +// left — and must not go on ticking a cycle that would later expire under a |
| +// live pipeline. |
| +func TestHoldMemoryDeletionStampClearedByAVerifiedPipeline(t *testing.T) { |
| + f := &FirstSeen{Jitter: 0} |
| + now := time.Date(2026, 8, 22, 12, 0, 0, 0, time.UTC) |
| + key := channelClockKey("bae", "web", "production") |
| + p := &Pipeline{Schema: 1, Channels: []PipelineChannel{{Name: "production", Bake: "24h"}}, SetAt: now} |
| + f.ObservePipeline("bae", "web", p, now) |
| + f.ObservePipeline("bae", "web", nil, now.Add(time.Hour)) |
| + if f.Holds[key].DeletedAt.IsZero() { |
| + t.Fatal("absence did not stamp DeletedAt") |
| + } |
| + stamp := f.Holds[key].DeletedAt |
| + // Re-observed absence must not restamp it. |
| + f.ObservePipeline("bae", "web", nil, now.Add(2*time.Hour)) |
| + if !f.Holds[key].DeletedAt.Equal(stamp) { |
| + t.Fatalf("re-observed absence restamped DeletedAt: %v, want %v", f.Holds[key].DeletedAt, stamp) |
| + } |
| + // The object comes back — same SetAt, the byte-identical restore. |
| + if !f.ObservePipeline("bae", "web", p, now.Add(3*time.Hour)) { |
| + t.Fatal("a restored pipeline that clears the deletion stamp did not report changed") |
| + } |
| + if !f.Holds[key].DeletedAt.IsZero() { |
| + t.Fatalf("DeletedAt = %v after the pipeline was restored, want cleared", f.Holds[key].DeletedAt) |
| + } |
| +} |
| + |
| // TestHoldMemoryRestoredPipelineCancelsAbsencePending: the re-review probe. |
| // Delete the pipeline (absence stamps the legacy default pending), then |
| // restore the object BYTE-IDENTICALLY — same SetAt: a bucket-versioning |
diff --git a/internal/release/pipeline_test.go b/internal/release/pipeline_test.go| index 49b74f4ba..18e58e3f9 100644 |
| --- a/internal/release/pipeline_test.go |
| +++ b/internal/release/pipeline_test.go |
| @@ -232,3 +232,83 @@ func TestScopedKeyCannotSignPipelineChannel(t *testing.T) { |
| t.Fatal("scoped key signed a pipeline channel pointer; want refusal even with --hotfix") |
| } |
| } |
| + |
| +// TestPipelinePromoteNamesTheChannelAsTo: PromoteResult.To was Rung(channel) |
| +// with no fallback, so a promotion onto an app-defined channel reported "" |
| +// — an empty "to" in the API's answer and, worse, an empty To in the |
| +// Promotion record written into the pointer's own audit trail, which is the |
| +// permanent record of where a build went. From already carried the channel |
| +// name on this path (VersionStage answers in channel names), so the trail |
| +// read "edge -> " for the app's whole history. The fallback is the channel's |
| +// own name; frozen rungs keep answering with the rung. |
| +func TestPipelinePromoteNamesTheChannelAsTo(t *testing.T) { |
| + s, priv, pub := pipelineTestHarness(t) |
| + ctx := context.Background() |
| + if _, err := SetPipeline(ctx, s, priv, "bae", "web", |
| + []PipelineChannel{{Name: "edge"}, {Name: "production"}}, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + shipRelease(t, s, "bae", "web", "v1", "linux-arm64", []byte("bin v1")) |
| + if _, err := PromoteOnLadder(ctx, s, priv, "bae", "web", "edge", "v1", PromoteOptions{}); err != nil { |
| + t.Fatalf("promote to edge: %v", err) |
| + } |
| + res, err := PromoteOnLadder(ctx, s, priv, "bae", "web", "production", "v1", PromoteOptions{Actor: "paul@keymail.dev"}) |
| + if err != nil { |
| + t.Fatalf("promote to production: %v", err) |
| + } |
| + if res.To != "production" { |
| + t.Errorf("PromoteResult.To = %q, want %q — an app-defined channel names itself", res.To, "production") |
| + } |
| + if res.From != "edge" { |
| + t.Errorf("PromoteResult.From = %q, want %q", res.From, "edge") |
| + } |
| + p, err := Channel(ctx, s, pub, "bae", "web", "production") |
| + if err != nil { |
| + t.Fatalf("read pointer: %v", err) |
| + } |
| + if len(p.Audit) == 0 { |
| + t.Fatalf("pointer carries no audit trail: %+v", p) |
| + } |
| + if got := p.Audit[len(p.Audit)-1]; got.To != "production" || got.From != "edge" { |
| + t.Errorf("promotion record = %+v, want from edge to production", got) |
| + } |
| + |
| + // The repeat (a no-op) names the channel on BOTH sides: the build is on |
| + // production, which is exactly what makes this a repeat, so reporting "" |
| + // for where it sits would be the same empty answer one rung down. |
| + again, err := PromoteOnLadder(ctx, s, priv, "bae", "web", "production", "v1", PromoteOptions{}) |
| + if err != nil { |
| + t.Fatalf("repeat promote: %v", err) |
| + } |
| + if !again.NoOp { |
| + t.Errorf("repeat promote = %+v, want NoOp", again) |
| + } |
| + if again.From != "production" || again.To != "production" { |
| + t.Errorf("repeat promote from/to = %q/%q, want production/production", again.From, again.To) |
| + } |
| +} |
| + |
| +// The frozen rungs are untouched: a legacy promote still reports the RUNG, |
| +// which is not always the channel name (canary/x reports "canary"). |
| +func TestLegacyPromoteStillReportsTheRung(t *testing.T) { |
| + s, priv, _ := pipelineTestHarness(t) |
| + ctx := context.Background() |
| + shipRelease(t, s, "bae", "web", "v1", "linux-arm64", []byte("bin v1")) |
| + res, err := PromoteOnLadder(ctx, s, priv, "bae", "web", "canary/x", "v1", PromoteOptions{}) |
| + if err != nil { |
| + t.Fatalf("promote to canary/x: %v", err) |
| + } |
| + if res.To != RungCanary { |
| + t.Errorf("PromoteResult.To = %q, want the rung %q", res.To, RungCanary) |
| + } |
| + // A different build for edge: v1 is on canary now, and the legacy ladder |
| + // only lets a build with no rung yet enter at edge. |
| + shipRelease(t, s, "bae", "web", "v2", "linux-arm64", []byte("bin v2")) |
| + res, err = PromoteOnLadder(ctx, s, priv, "bae", "web", "edge", "v2", PromoteOptions{}) |
| + if err != nil { |
| + t.Fatalf("promote to edge: %v", err) |
| + } |
| + if res.To != RungEdge { |
| + t.Errorf("PromoteResult.To = %q, want the rung %q", res.To, RungEdge) |
| + } |
| +} |
diff --git a/internal/release/release.go b/internal/release/release.go| index 21615f227..b694c2fca 100644 |
| --- a/internal/release/release.go |
| +++ b/internal/release/release.go |
| @@ -825,7 +825,21 @@ func (sg Signer) PromoteOnLadder(ctx context.Context, s Store, acct, app, channe |
| if err := sg.allows(channel, time.Now().UTC()); err != nil { |
| return res, fmt.Errorf("promote: %w", err) |
| } |
| - res.To, _ = Rung(channel) |
| + // The target, named: its RUNG when the channel is one of the frozen |
| + // rungs (canary/x reports "canary" — the rung, not the channel, which is |
| + // what the ladder's vocabulary means by "to"), else the channel's own |
| + // name. Without the fallback an app-defined channel reported "", which is |
| + // not merely a thin API answer: it is the To written into the Promotion |
| + // record in the pointer's own audit trail, the permanent record of where |
| + // a build went — and From on that same path already carries the channel |
| + // name (VersionStage answers in channel names), so a pipelined app's |
| + // trail read "edge -> " forever. Wire-additive in both places: a fuller |
| + // string exactly where an empty one used to be. |
| + if rung, ok := Rung(channel); ok { |
| + res.To = rung |
| + } else { |
| + res.To = channel |
| + } |
| pub := sg.verifyPub() |
| // A repeat is a no-op, checked before anything else: re-running a command |
| @@ -833,7 +847,11 @@ func (sg Signer) PromoteOnLadder(ctx context.Context, s Store, acct, app, channe |
| if cur, _, err := s.Get(ctx, ChannelKey(acct, app, channel)); err == nil { |
| if p, verr := verifyPointer(pub, cur, app, channel); verr == nil && p.Version == version { |
| res.NoOp = true |
| - res.From, _ = Rung(channel) |
| + // The same fallback res.To takes above, for the same reason: the |
| + // build IS on this channel (that is what makes this a repeat), so |
| + // an app-defined channel names itself here too rather than |
| + // reporting "" for where the build already sits. |
| + res.From = res.To |
| // A repeat still reports the tag — and cuts it if a previous |
| // attempt's write failed, so re-running the same command heals a |
| // stable release that ended up nameless. |
| @@ -885,12 +903,10 @@ func (sg Signer) PromoteOnLadder(ctx context.Context, s Store, acct, app, channe |
| } |
| res.From = from |
| if !opt.Hotfix { |
| - // A channel that is not a rung is named as itself in the refusal — |
| - // "wip is not a rung" reads better than an empty target. |
| + // res.To already names a channel that is not a rung as itself (see |
| + // its fallback above) — "wip is not a rung" reads better in the |
| + // refusal than an empty target. |
| to := res.To |
| - if to == "" { |
| - to = channel |
| - } |
| if pipe != nil { |
| err = PipelinePromotionAllowed(pipe, from, channel) |
| } else { |
diff --git a/mail_cli.go b/mail_cli.go| index 05275a66a..e00d7dfba 100644 |
| --- a/mail_cli.go |
| +++ b/mail_cli.go |
| @@ -861,7 +861,8 @@ func printEmailStatus(w io.Writer, res apiEmailReply) { |
| if len(eventless) > 0 { |
| warnf(w, " no SES event destination in %s — sends, bounces and complaints are never counted for this domain, so the daily cap and the automatic pause cannot fire", |
| strings.Join(eventless, ", ")) |
| - mutedf(w, " re-run `carlos email domains add --app %s %s` to bind one — it is idempotent and leaves a verified identity verified", |
| + mutedf(w, " re-run `carlos email domains add --app %s %s` to bind one — it binds the event "+ |
| + "destination and refreshes this app's sending policy together, and leaves a verified identity verified", |
| res.App, d.Domain) |
| } |
| } |