amadan
console: an approvals-gated promote --retry keeps its retry semantics
Final review F3 (Plan 1 D1). The pending promote payload carries retry (json retry,omitempty) and executePendingPromote passes it into PromoteOptions, so PromoteOnLadder's retry rules (renew only, version guard at the write) apply when the approved proposal executes. Two adaptations so a retry proposal can exist and fail in the retry's own words: the API propose door and execution skip the pipeline ORDER check for a retry (it moves nothing along the pipeline; PromoteOnLadder's repeat path skips order for the same reason), and the propose door instead refuses a retry whose channel head is not the version (409, 'promote --retry renews a promotion: ...'). Execution leaves the observed-head pin to PromoteOnLadder's version guard for a retry, which is the stronger pin. Authorization gates (deploy scope, control plane, quorum, passkey) are unchanged. Tests: approved retry renews PromotedAt (audit marked retry); retry over a moved channel fails with 'renews a promotion'; retry onto a channel without the version is refused at propose; a deploy credential's retry onto an out-of-scope channel gets the plain promote's identical 403 (ledger Task 7).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 files changed,
+146
−5
internal/console/apicli.go+18 −2internal/console/apicli_test.go+21 −0internal/console/pendingflow_test.go+89 −0internal/console/pendinghandlers.go+18 −3
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index 191948298..6fd6d2040 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -829,7 +829,12 @@ func (s *Server) handleAPIPromote(w http.ResponseWriter, r *http.Request) { |
| // strictly after the rules decision this switch is making. |
| // pipe is non-nil in this arm by construction, which matters |
| // because VersionStage dereferences the pipeline it is handed. |
| - if !hotfix { |
| + // |
| + // A retry asks no order question (it moves nothing along the |
| + // pipeline: PromoteOnLadder's repeat path skips order for the |
| + // same reason); its precondition is checked against the head |
| + // below instead. |
| + if !hotfix && !req.Retry { |
| stage, serr := release.VersionStage(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app, version, pipe) |
| if serr != nil { |
| log.Printf("console: api promote %s/%s: stage: %v", acct.Sqid, app, serr) |
| @@ -851,8 +856,19 @@ func (s *Server) handleAPIPromote(w http.ResponseWriter, r *http.Request) { |
| jsonError(w, http.StatusBadGateway, "could not read channel "+channel) |
| return |
| } |
| + // A retry renews the head's promotion, so one whose channel does |
| + // not serve this version could never execute: refused now, in |
| + // PromoteOnLadder's own words, before it goes out for signatures. |
| + if req.Retry && head != version { |
| + names := head |
| + if names == "" { |
| + names = "nothing" |
| + } |
| + jsonError(w, http.StatusConflict, fmt.Sprintf("promote --retry renews a promotion: %s names %s, not %s", channel, names, version)) |
| + return |
| + } |
| s.proposeOverAPI(w, acct, app, kindPromote, |
| - promotePayload{Version: version, Channel: channel, Hotfix: hotfix, ObservedHead: head}, |
| + promotePayload{Version: version, Channel: channel, Hotfix: hotfix, ObservedHead: head, Retry: req.Retry}, |
| rules.PromoteApprovals, addr) |
| return |
| case rules.Passkey: |
diff --git a/internal/console/apicli_test.go b/internal/console/apicli_test.go| index 5c99143d0..b592e9c78 100644 |
| --- a/internal/console/apicli_test.go |
| +++ b/internal/console/apicli_test.go |
| @@ -2815,3 +2815,24 @@ func TestAPIPromoteRetryResignsThePointer(t *testing.T) { |
| t.Fatalf("pointer after retry = %+v, want abc1234 with a later PromotedAt than %v", after, before.PromotedAt) |
| } |
| } |
| + |
| +// Ledger Task 7: --retry buys nothing past a deploy credential's scope. A |
| +// retry onto a channel the credential cannot promote to gets exactly the |
| +// plain promote's refusal, and the pointer is not re-signed. |
| +func TestAPIPromoteRetryMeetsTheDeployScope(t *testing.T) { |
| + fx, _, deploy := deployFixture(t) |
| + seedChannel(t, fx, "hello", "beta", "v1") |
| + before := fx.channelVersion(t, "hello", "beta") |
| + url := fx.ts.URL + "/api/cli/apps/bab/hello/promote" |
| + plainStatus, plain := postJSON(t, url, map[string]any{"version": "v1", "channel": "beta"}, deploy) |
| + retryStatus, retry := postJSON(t, url, map[string]any{"version": "v1", "channel": "beta", "retry": true}, deploy) |
| + if plainStatus != http.StatusForbidden || retryStatus != plainStatus { |
| + t.Fatalf("plain = %d %v, retry = %d %v; want the same 403", plainStatus, plain, retryStatus, retry) |
| + } |
| + if plain["error"] != retry["error"] || !strings.Contains(str(t, retry, "error"), "beta is not one of them") { |
| + t.Fatalf("retry refusal %q, plain %q: want the identical D22 text", retry["error"], plain["error"]) |
| + } |
| + if after := fx.channelVersion(t, "hello", "beta"); !after.PromotedAt.Equal(before.PromotedAt) { |
| + t.Fatalf("beta re-signed at %v (was %v) by an out-of-scope retry", after.PromotedAt, before.PromotedAt) |
| + } |
| +} |
diff --git a/internal/console/pendingflow_test.go b/internal/console/pendingflow_test.go| index a300fcf63..257d9a5ae 100644 |
| --- a/internal/console/pendingflow_test.go |
| +++ b/internal/console/pendingflow_test.go |
| @@ -1227,3 +1227,92 @@ func TestDepartedMemberConfirmDoesNotCount(t *testing.T) { |
| t.Fatalf("confirmations = %v, want all three rows KEPT — they simply do not all count", left[0].Confirms) |
| } |
| } |
| + |
| +// retryProposal is the D1 approvals world: widget's production needs one |
| +// approval and already serves v2, and paul proposes `promote --retry v2 |
| +// production` over the API. It returns the pending change with paul's |
| +// confirmation recorded, ready to execute. |
| +func retryProposal(t *testing.T, fx *consoleFixture) (*Account, *PendingChange) { |
| + t.Helper() |
| + pipelinedApp(t, fx, "widget", "v2", |
| + release.PipelineChannel{Name: "edge"}, |
| + release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 1}}) |
| + seedChannel(t, fx, "widget", "production", "v2") |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-retry-proposal") |
| + status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/widget/promote", |
| + map[string]any{"version": "v2", "channel": "production", "retry": true}, token) |
| + if status != http.StatusAccepted { |
| + t.Fatalf("retry into an approvals channel = %d %v, want 202 (a proposal)", 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), `"retry":true`) { |
| + t.Fatalf("payload = %s, want the retry carried", pending[0].Payload) |
| + } |
| + pc, met, err := fx.st.ConfirmChange(pending[0].ID, "paul@keymail.dev", time.Now()) |
| + if err != nil || !met { |
| + t.Fatalf("confirm = %v, met %v", err, met) |
| + } |
| + acct, err := fx.st.AccountBySqid(fx.sqid) |
| + if err != nil || acct == nil { |
| + t.Fatalf("account: %v, %v", acct, err) |
| + } |
| + return acct, pc |
| +} |
| + |
| +// Final review F3: an approvals-gated retry keeps its retry semantics. Once |
| +// approved it RENEWS the promotion (a fresh PromotedAt on the same version), |
| +// rather than landing as an ordinary promote of a version already there, |
| +// which is a no-op that leaves every box's refusal standing. |
| +func TestRetryProposalRenewsThePromotionOnceApproved(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + acct, pc := retryProposal(t, fx) |
| + before := fx.channelVersion(t, "widget", "production") |
| + if msg, err := fx.srv.executePending(context.Background(), acct, "widget", "paul@keymail.dev", pc); err != nil { |
| + t.Fatalf("executing the approved retry = %v (%q)", err, msg) |
| + } |
| + after := fx.channelVersion(t, "widget", "production") |
| + if after.Version != "v2" || !after.PromotedAt.After(before.PromotedAt) { |
| + t.Fatalf("production after the retry = %s at %v, want v2 renewed after %v", after.Version, after.PromotedAt, before.PromotedAt) |
| + } |
| + if len(after.Audit) == 0 || !after.Audit[0].Retry { |
| + t.Fatalf("audit = %+v, want the newest entry marked as a retry", after.Audit) |
| + } |
| +} |
| + |
| +// ... and the retry's version guard holds at execution: a channel that moved |
| +// to another version while the proposal waited is refused with the retry's |
| +// own words, and the new head is left alone. |
| +func TestRetryProposalRefusedWhenTheChannelMoved(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + acct, pc := retryProposal(t, fx) |
| + shipVersion(t, fx, "widget", "v3") |
| + seedChannel(t, fx, "widget", "production", "v3") |
| + _, err := fx.srv.executePending(context.Background(), acct, "widget", "paul@keymail.dev", pc) |
| + if err == nil || !strings.Contains(err.Error(), "renews a promotion") { |
| + t.Fatalf("executing a retry over a moved channel = %v, want the renews-a-promotion refusal", err) |
| + } |
| + if got := fx.channelVersion(t, "widget", "production"); got.Version != "v3" { |
| + t.Fatalf("production serves %q, want v3 untouched", got.Version) |
| + } |
| +} |
| + |
| +// A retry whose channel does not serve the version could never execute, so |
| +// the propose door refuses it before it goes out for signatures. |
| +func TestRetryProposalNeedsTheHeadToBeTheVersion(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + pipelinedApp(t, fx, "widget", "v2", |
| + release.PipelineChannel{Name: "edge"}, |
| + release.PipelineChannel{Name: "production", Rules: release.ChannelRules{PromoteApprovals: 1}}) |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-retry-no-head") |
| + status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/widget/promote", |
| + map[string]any{"version": "v2", "channel": "production", "retry": true}, token) |
| + if status != http.StatusConflict || !strings.Contains(str(t, out, "error"), "renews a promotion: production names nothing, not v2") { |
| + t.Fatalf("retry onto a channel without the version = %d %v, want 409 naming what production serves", status, out) |
| + } |
| + if pending, _ := fx.st.PendingChangesFor(OpsAccountID, "widget", time.Now()); len(pending) != 0 { |
| + t.Fatalf("pending = %v, want no proposal", pending) |
| + } |
| +} |
diff --git a/internal/console/pendinghandlers.go b/internal/console/pendinghandlers.go| index a52c1b1d5..b5230370f 100644 |
| --- a/internal/console/pendinghandlers.go |
| +++ b/internal/console/pendinghandlers.go |
| @@ -81,6 +81,13 @@ type promotePayload struct { |
| // "" (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"` |
| + // Retry is `carlos promote --retry` (D1 of the console release |
| + // defences): renew the channel's current promotion of Version with a |
| + // fresh PromotedAt, never change the version. Execution passes it to |
| + // PromoteOnLadder, whose retry rules (renew only, version guard at the |
| + // write) then apply at the moment the write happens. Additive: an old |
| + // row has none and is an ordinary promotion. |
| + Retry bool `json:"retry,omitempty"` |
| } |
| // policyPayload pins a pipeline edit: the full proposed channel list, plus |
| @@ -506,7 +513,13 @@ func (s *Server) executePendingPromote(ctx context.Context, acct *Account, app, |
| // 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 != "" { |
| + // |
| + // A retry is pinned harder than this, by PromoteOnLadder itself: its |
| + // version guard requires the head to be Version on the very pointer the |
| + // write is conditional on (and the propose door only lets a retry out |
| + // with head == Version). It is left to that guard so a moved channel |
| + // fails with the retry's own refusal, "renews a promotion". |
| + if p.ObservedHead != "" && !p.Retry { |
| 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) |
| @@ -517,7 +530,9 @@ func (s *Server) executePendingPromote(ctx context.Context, acct *Account, app, |
| p.Channel + " no longer serves " + p.ObservedHead + ")") |
| } |
| } |
| - if !p.Hotfix { |
| + // No order re-check for a retry: it moves nothing along the pipeline, |
| + // the same reason PromoteOnLadder's repeat path asks no order question. |
| + if !p.Hotfix && !p.Retry { |
| stage, serr := release.VersionStage(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app, p.Version, pipe) |
| if serr != nil { |
| log.Printf("console: execute promote %s/%s: stage: %v", acct.Sqid, app, serr) |
| @@ -528,7 +543,7 @@ func (s *Server) executePendingPromote(ctx context.Context, acct *Account, app, |
| } |
| } |
| res, err := release.PromoteOnLadder(ctx, s.cfg.Releases, s.cfg.Key, acct.Sqid, app, p.Channel, p.Version, |
| - release.PromoteOptions{Actor: actor, Hotfix: p.Hotfix}) |
| + release.PromoteOptions{Actor: actor, Hotfix: p.Hotfix, Retry: p.Retry}) |
| if err != nil { |
| log.Printf("console: execute promote %s %s -> %s: %v", app, p.Version, p.Channel, err) |
| return "", err |