amadan

carlos / platform Public

console: promote/rollback proposals pin the observed head; rollback audit voice

Final-review finding 2 plus its rider. Spec §4 says a head that moved
underneath a proposal invalidates it, and promotePayload pinned
Version/Channel but never the target channel's head — a proposal
confirmed days later executed with no re-check at all (hotfix and
rollback skipped even the order re-check). promotePayload gains an
additive observed_head, pinned at all four propose sites (API and
browser, promote and rollback; "" when the channel has no pointer yet).
executePendingPromote refuses on a mismatch — surfaced exactly as the
policy pin's SetAt mismatch: an error return, nothing written, the
change still listed, never consumed. Old rows (absent field) keep their
behaviour byte-for-byte.

Rider: a rollback proposal's execution audited as "promoted vX to Y"
while the card said "roll back". The audit row and the flash now honour
p.Rollback in the direct rollback door's own voice.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 764802a58e248415ee5f3f3e16404f1db8265140 parent 1c03714
4 files changed, +298 −6
  • internal/console/apicli.go +13 −2
  • internal/console/pendingflow_test.go +219 −0
  • internal/console/pendinghandlers.go +53 −2
  • internal/console/server.go +13 −2
diff --git a/internal/console/apicli.go b/internal/console/apicli.go
index 8c9de35cc..e1eb461fe 100644
--- a/internal/console/apicli.go
+++ b/internal/console/apicli.go
@@ -735,8 +735,18 @@ func (s *Server) handleAPIPromote(w http.ResponseWriter, r *http.Request) {
return
}
}
+ // Pin the target channel's head at propose time (spec §4: a
+ // head that moved underneath a proposal invalidates it); ""
+ // when the channel has no pointer yet. An unreadable head
+ // refuses — a pin that cannot be read cannot be recorded.
+ head, herr := s.channelHead(ctx, acct, app, channel)
+ if herr != nil {
+ log.Printf("console: api promote %s/%s: read channel %s: %v", acct.Sqid, app, channel, herr)
+ jsonError(w, http.StatusBadGateway, "could not read channel "+channel)
+ return
+ }
s.proposeOverAPI(w, acct, app, kindPromote,
- promotePayload{Version: version, Channel: channel, Hotfix: hotfix},
+ promotePayload{Version: version, Channel: channel, Hotfix: hotfix, ObservedHead: head},
rules.PromoteApprovals, addr)
return
case rules.Passkey:
@@ -922,7 +932,8 @@ func (s *Server) handleAPIRollback(w http.ResponseWriter, r *http.Request) {
switch rules := channelRules(pipe, channel); {
case rules.PromoteApprovals >= 1:
s.proposeOverAPI(w, acct, app, kindPromote,
- promotePayload{Version: target, Channel: channel, Hotfix: true, Rollback: true},
+ promotePayload{Version: target, Channel: channel, Hotfix: true, Rollback: true,
+ ObservedHead: cur.Version},
rules.PromoteApprovals, addr)
return
case rules.Passkey:
diff --git a/internal/console/pendingflow_test.go b/internal/console/pendingflow_test.go
index b2ea2362c..78c137977 100644
--- a/internal/console/pendingflow_test.go
+++ b/internal/console/pendingflow_test.go
@@ -676,6 +676,225 @@ func TestPendingDoorsAreNotAnOracle(t *testing.T) {
channelAbsent(t, fx, "widget", "production")
}
+// TestPromoteProposalPinsObservedHead is spec §4's other pin: "a head that
+// moved underneath a proposal invalidates it". A promote proposal records the
+// target channel's head at propose time; execution refuses when the head has
+// moved since — the pointer stays where whoever moved it put it, and the
+// change stays listed (retryable-in-principle, consistent with the policy
+// pin), never silently landed over a world nobody approved.
+func TestPromoteProposalPinsObservedHead(t *testing.T) {
+ fx, c, csrf := memberFixture(t) // paul owns, kim is a member
+ pipelinedApp(t, fx, "widget", "v2",
+ release.PipelineChannel{Name: "edge"},
+ release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 2}})
+ shipVersion(t, fx, "widget", "v1")
+ seedChannel(t, fx, "widget", "production", "v1") // head A at propose time
+
+ status, loc, body := postGated(t, fx, c, fx.u("/apps/widget/promote"),
+ url.Values{"csrf": {csrf}, "channel": {"production"}, "version": {"v2"}, "confirmed": {"1"}})
+ if status != http.StatusSeeOther {
+ t.Fatalf("proposal = %d -> %q:\n%s", status, loc, body)
+ }
+ pending, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now())
+ if err != nil || len(pending) != 1 {
+ t.Fatalf("pending = %v, %v, want exactly one", pending, err)
+ }
+ pc := pending[0]
+ if !strings.Contains(string(pc.Payload), `"observed_head":"v1"`) {
+ t.Fatalf("payload = %s, want the observed head v1 pinned", pc.Payload)
+ }
+
+ // The head moves underneath the proposal.
+ shipVersion(t, fx, "widget", "v3")
+ seedChannel(t, fx, "widget", "production", "v3")
+
+ // Confirming to quorum: execution refuses, nothing is written, and the
+ // change stays listed — exactly the policy pin's shape.
+ kim := newBrowser(t)
+ fx.setUser("kim@keymail.dev")
+ signUp(t, fx, kim, "code-kim-headpin")
+ 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("confirming a moved-head proposal = %d -> %q:\n%s", status, loc, body)
+ }
+ if got := fx.channelVersion(t, "widget", "production"); got.Version != "v3" {
+ t.Fatalf("production serves %q after the refused execution, want v3 untouched", got.Version)
+ }
+ if got := serial(t, fx); got != before {
+ t.Fatalf("serial = %d after a refused execution, want %d — nothing was written for a box to read", got, before)
+ }
+ left, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now())
+ if err != nil || len(left) != 1 || left[0].ID != pc.ID {
+ t.Fatalf("pending after a refused execution = %v, %v, want the proposal still there", left, err)
+ }
+}
+
+// TestPromoteAPIPinsObservedHead: the bearer-token doors pin the head too —
+// the API promote door records the target channel's current version, and
+// omits the field entirely for a channel with no pointer yet.
+func TestPromoteAPIPinsObservedHead(t *testing.T) {
+ fx := newConsoleFixture(t)
+ if err := fx.st.AddMember(OpsAccountID, "alice@keymail.dev", RoleMember, time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ pipelinedApp(t, fx, "widget", "v2",
+ release.PipelineChannel{Name: "edge"},
+ release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 2}})
+ token := cliToken(t, fx, "paul@keymail.dev", "code-head-pin-api")
+ promoteURL := fx.ts.URL + "/api/cli/apps/bab/widget/promote"
+
+ // production has NO pointer yet: the proposal carries no pin.
+ status, out := postJSON(t, promoteURL, map[string]any{"version": "v2", "channel": "production"}, token)
+ if status != http.StatusAccepted {
+ t.Fatalf("promote proposal = %d %v, want 202", status, out)
+ }
+ pending, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now())
+ if err != nil || len(pending) != 1 {
+ t.Fatalf("pending = %v, %v, want one", pending, err)
+ }
+ if strings.Contains(string(pending[0].Payload), "observed_head") {
+ t.Fatalf("payload = %s, want NO observed_head for a channel with no pointer", pending[0].Payload)
+ }
+ if err := fx.st.DeleteChange(pending[0].ID); err != nil {
+ t.Fatal(err)
+ }
+
+ // With a head in place, the proposal pins it.
+ shipVersion(t, fx, "widget", "v1")
+ seedChannel(t, fx, "widget", "production", "v1")
+ status, out = postJSON(t, promoteURL, map[string]any{"version": "v2", "channel": "production"}, token)
+ if status != http.StatusAccepted {
+ t.Fatalf("promote proposal = %d %v, want 202", status, out)
+ }
+ pending, err = fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now())
+ if err != nil || len(pending) != 1 {
+ t.Fatalf("pending = %v, %v, want one", pending, err)
+ }
+ if !strings.Contains(string(pending[0].Payload), `"observed_head":"v1"`) {
+ t.Fatalf("payload = %s, want the observed head v1 pinned", pending[0].Payload)
+ }
+}
+
+// TestRollbackProposalPinsObservedHead: the rollback doors pin the head they
+// read — the version the rollback is FROM — so a channel that moved again
+// while the proposal waited invalidates it at execution.
+func TestRollbackProposalPinsObservedHead(t *testing.T) {
+ fx := newConsoleFixture(t)
+ if err := fx.st.AddMember(OpsAccountID, "alice@keymail.dev", RoleMember, time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := fx.st.CreateAppIn(OpsAccountID, "widget", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ shipVersion(t, fx, "widget", "v1")
+ shipVersion(t, fx, "widget", "v2")
+ seedChannel(t, fx, "widget", "production", "v1")
+ seedChannel(t, fx, "widget", "production", "v2")
+ setPipeline(t, fx, "widget",
+ release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 2}})
+ token := cliToken(t, fx, "paul@keymail.dev", "code-head-pin-rollback")
+
+ status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/widget/rollback",
+ map[string]any{"channel": "production"}, token)
+ if status != http.StatusAccepted {
+ t.Fatalf("rollback proposal = %d %v, want 202", status, out)
+ }
+ pending, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now())
+ if err != nil || len(pending) != 1 {
+ t.Fatalf("pending = %v, %v, want one", pending, err)
+ }
+ if !strings.Contains(string(pending[0].Payload), `"observed_head":"v2"`) {
+ t.Fatalf("payload = %s, want the head being rolled back from (v2) pinned", pending[0].Payload)
+ }
+}
+
+// TestPromoteOldRowWithoutObservedHeadExecutes: a row written before the
+// observed_head field existed carries no pin and keeps its old behaviour —
+// wire-shape discipline for rows already sitting in production consoles.
+func TestPromoteOldRowWithoutObservedHeadExecutes(t *testing.T) {
+ fx := newConsoleFixture(t)
+ pipelinedApp(t, fx, "widget", "v2",
+ release.PipelineChannel{Name: "edge"},
+ release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 1}})
+ shipVersion(t, fx, "widget", "v1")
+ seedChannel(t, fx, "widget", "production", "v1")
+ acct, err := fx.st.AccountBySqid(fx.sqid)
+ if err != nil || acct == nil {
+ t.Fatalf("account: %v, %v", acct, err)
+ }
+ // The pre-change payload shape, verbatim: no observed_head.
+ pc, err := fx.st.ProposeChange(OpsAccountID, "widget", "promote",
+ []byte(`{"version":"v2","channel":"production"}`), 1, "paul@keymail.dev", true, time.Now())
+ if err != nil {
+ t.Fatal(err)
+ }
+ if msg, err := fx.srv.executePending(context.Background(), acct, "widget", "paul@keymail.dev", pc); err != nil {
+ t.Fatalf("executing an old row without the pin = %v (%q), want it to land as before", err, msg)
+ }
+ if got := fx.channelVersion(t, "widget", "production"); got.Version != "v2" {
+ t.Fatalf("production serves %q, want v2 — old rows keep current behaviour", got.Version)
+ }
+}
+
+// TestRollbackExecutionAuditSaysRolledBack: the card says "roll back", so the
+// execution's audit row must too — "promoted v1 to production" for a rollback
+// is illegible to whoever reads the trail later (same Task 8 argument that
+// fixed the summary).
+func TestRollbackExecutionAuditSaysRolledBack(t *testing.T) {
+ fx, c, csrf := memberFixture(t) // paul owns, kim is a member
+ if _, err := fx.st.CreateAppIn(OpsAccountID, "widget", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ shipVersion(t, fx, "widget", "v1")
+ shipVersion(t, fx, "widget", "v2")
+ seedChannel(t, fx, "widget", "production", "v1")
+ seedChannel(t, fx, "widget", "production", "v2")
+ setPipeline(t, fx, "widget",
+ release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 2}})
+
+ status, loc, body := postGated(t, fx, c, fx.u("/apps/widget/rollback"),
+ url.Values{"csrf": {csrf}, "channel": {"production"}, "confirmed": {"1"}})
+ if status != http.StatusSeeOther {
+ t.Fatalf("rollback proposal = %d -> %q:\n%s", status, loc, body)
+ }
+ pending, err := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now())
+ if err != nil || len(pending) != 1 {
+ t.Fatalf("pending = %v, %v, want one", pending, err)
+ }
+ kim := newBrowser(t)
+ fx.setUser("kim@keymail.dev")
+ signUp(t, fx, kim, "code-kim-audit")
+ kimCSRF := sessionToken(t, fx, kim)
+ status, loc, body = postGated(t, fx, kim,
+ fx.u("/apps/widget/pending/"+pending[0].ID+"/confirm"), url.Values{"csrf": {kimCSRF}})
+ if status != http.StatusSeeOther {
+ t.Fatalf("confirmation = %d -> %q:\n%s", status, loc, body)
+ }
+ if got := fx.channelVersion(t, "widget", "production"); got.Version != "v1" {
+ t.Fatalf("production serves %q after quorum, want the rolled-back v1", got.Version)
+ }
+ events, err := fx.st.Events(OpsAccountID, "widget", 50)
+ if err != nil {
+ t.Fatal(err)
+ }
+ var found bool
+ for _, ev := range events {
+ if strings.Contains(ev.Detail, "promoted v1 to production") {
+ t.Fatalf("audit row %q says promoted for a rollback proposal", ev.Detail)
+ }
+ if ev.Action == "rollback" && strings.Contains(ev.Detail, "rolled back production to v1") &&
+ strings.Contains(ev.Detail, "proposed by") {
+ found = true
+ }
+ }
+ if !found {
+ t.Fatalf("no rollback-voiced audit row in %+v", events)
+ }
+}
+
// TestHotfixBypassesOrderNotRules is spec §5's exact sentence, twice over:
// the flag never buys past a channel's rules, and it does bypass the order
// on a channel that carries none.
diff --git a/internal/console/pendinghandlers.go b/internal/console/pendinghandlers.go
index 8a380064d..c8574fb1a 100644
--- a/internal/console/pendinghandlers.go
+++ b/internal/console/pendinghandlers.go
@@ -32,6 +32,7 @@ import (
"log"
"net/http"
"net/url"
+ "os"
"strings"
"time"
@@ -65,9 +66,16 @@ type promotePayload struct {
// parses and simply renders as a promotion, exactly as it did before
// (Task 8 review: pendingSummary said "promote vX to Y" for a rollback,
// which is illegible to a confirmer who never asked for vX). Display
- // only: both doors already pin the OLDER version as a promotion with
- // Hotfix set, and execution (executePendingPromote) is unchanged by it.
+ // only for what lands: both doors already pin the OLDER version as a
+ // promotion with Hotfix set; execution reads it solely to voice the
+ // audit row and the flash as the rollback the confirmer approved.
Rollback bool `json:"rollback,omitempty"`
+ // ObservedHead is the target channel's version at PROPOSE time — spec
+ // §4's other pin: "a head that moved underneath a proposal invalidates
+ // it". Execution refuses when the channel no longer serves it. Additive:
+ // "" (an old row, or a channel with no pointer yet at propose time) pins
+ // nothing and keeps the pre-field behaviour.
+ ObservedHead string `json:"observed_head,omitempty"`
}
// policyPayload pins a pipeline edit: the full proposed channel list, plus
@@ -227,6 +235,21 @@ func (s *Server) proposeThenExecute(ctx context.Context, acct *Account, app, kin
return "ok", msg
}
+// channelHead is the target channel's current version — read at a propose
+// site to pin promotePayload.ObservedHead, and again at execution to check
+// the pin. "" with a nil error means the channel has no pointer yet; any
+// real read/verify failure is the error, never folded into "".
+func (s *Server) channelHead(ctx context.Context, acct *Account, app, channel string) (string, error) {
+ cur, err := release.Channel(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app, channel)
+ if errors.Is(err, os.ErrNotExist) {
+ return "", nil
+ }
+ if err != nil {
+ return "", err
+ }
+ return cur.Version, nil
+}
+
// policyProposal builds a policy payload against the pipeline it was composed
// from. cur is nil for an app that has never had one, whose zero SetAt is
// itself a legitimate pin: "there was nothing here when I proposed this".
@@ -321,6 +344,25 @@ func (s *Server) executePendingPromote(ctx context.Context, acct *Account, app,
if pipe == nil {
return "", staleProposal("this app no longer has a pipeline")
}
+ // The head pin (spec §4: "a head that moved underneath a proposal
+ // invalidates it"), checked for EVERY promote-kind proposal — hotfixes
+ // and rollbacks bypass the order re-check below, never this one. On a
+ // mismatch the proposal can never succeed, but it is refused rather than
+ // consumed — surfaced exactly as the policy pin's SetAt mismatch is: an
+ // error return, nothing written, the change still listed until someone
+ // rejects or re-proposes. An empty pin (an old row, or a channel that
+ // had no pointer at propose time) keeps the pre-field behaviour.
+ if p.ObservedHead != "" {
+ head, herr := s.channelHead(ctx, acct, app, p.Channel)
+ if herr != nil {
+ log.Printf("console: execute promote %s/%s: read channel %s: %v", acct.Sqid, app, p.Channel, herr)
+ return "", errors.New("could not read channel " + p.Channel)
+ }
+ if head != p.ObservedHead {
+ return "", errors.New("the channel moved underneath this proposal — re-propose (" +
+ p.Channel + " no longer serves " + p.ObservedHead + ")")
+ }
+ }
if !p.Hotfix {
stage, serr := release.VersionStage(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app, p.Version, pipe)
if serr != nil {
@@ -343,6 +385,15 @@ func (s *Server) executePendingPromote(ctx context.Context, acct *Account, app,
return p.Channel + " already serves " + p.Version + tagSuffix(res) + " — nothing to do.", nil
}
s.bumpSerial(ctx)
+ if p.Rollback {
+ // The card said "roll back" (pendingSummary); the trail and the
+ // flash must say what the confirmer approved, in the house voice the
+ // direct rollback door already uses — never "promoted vX" for a
+ // build nobody asked to promote.
+ s.audit(acct, app, actor, "rollback", fmt.Sprintf("rolled back %s to %s (proposed by %s, %d confirmations)",
+ p.Channel, p.Version, pc.Proposer, len(pc.Confirms)))
+ return "Rolled back " + p.Channel + " to " + p.Version + tagSuffix(res) + ".", nil
+ }
s.audit(acct, app, actor, "promote", fmt.Sprintf("promoted %s to %s (proposed by %s, %d confirmations)",
p.Version, p.Channel, pc.Proposer, len(pc.Confirms)))
return "Promoted " + p.Version + " to " + p.Channel + tagSuffix(res) + ".", nil
diff --git a/internal/console/server.go b/internal/console/server.go
index ca1306310..04e8eb9e8 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -2769,8 +2769,18 @@ func (s *Server) handlePromote(w http.ResponseWriter, r *http.Request) {
back("err", "Promote refused: "+aerr.Error())
return
}
+ // Pin the target channel's head at propose time (spec §4: a head
+ // that moved underneath a proposal invalidates it); "" when the
+ // channel has no pointer yet. An unreadable head refuses — a pin
+ // that cannot be read cannot be recorded.
+ head, herr := s.channelHead(ctx, acct, app, channel)
+ if herr != nil {
+ log.Printf("console: promote %s/%s: read channel %s: %v", acct.Sqid, app, channel, herr)
+ back("err", "Could not read channel "+channel+". Nothing has been changed.")
+ return
+ }
kind, msg := s.proposeThenExecute(ctx, acct, app, kindPromote,
- promotePayload{Version: version, Channel: channel}, rules.PromoteApprovals, addr)
+ promotePayload{Version: version, Channel: channel, ObservedHead: head}, rules.PromoteApprovals, addr)
back(kind, msg)
return
}
@@ -2986,7 +2996,8 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) {
// leaves — a promotion of the older build, confirmed by a
// quorum, rather than a history pop.
kind, msg := s.proposeThenExecute(ctx, acct, app, kindPromote,
- promotePayload{Version: target, Channel: channel, Hotfix: true, Rollback: true},
+ promotePayload{Version: target, Channel: channel, Hotfix: true, Rollback: true,
+ ObservedHead: cur.Version},
rules.PromoteApprovals, addr)
back(kind, msg)
return