amadan

carlos / platform Public

console: the promote API reports held on a no-op too

A re-run of the same deploy against a production app whose beta head is
mid-bake is a no-op that is STILL HELD. Answering {"result":"noop"} with
no held field decodes as held:false and sends the client off to watch for
an adopt a day away — the exact false failure the field exists to prevent,
on the most ordinary path there is (CI retries, a repeated command).

The computation becomes one method, channelHeld, used by both exit paths.
The no-op still announces nothing: the pointer did not move, so no bump.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell a74e48b725fc3cc62d83d04af41a1d6093753501 parent f08d1fa
2 files changed, +81 −22
  • internal/console/apicli.go +36 −22
  • internal/console/serialbump_test.go +45 −0
diff --git a/internal/console/apicli.go b/internal/console/apicli.go
index 4de87d8c3..2880051b1 100644
--- a/internal/console/apicli.go
+++ b/internal/console/apicli.go
@@ -352,36 +352,50 @@ func (s *Server) handleAPIPromote(w http.ResponseWriter, r *http.Request) {
return
}
if res.NoOp {
- writeJSON(w, http.StatusOK, map[string]any{"result": "noop"})
+ // held on the no-op too, and this is not decoration: re-running the
+ // same deploy (a CI retry, a repeated `carlos deploy`) against a
+ // production app whose beta head is mid-bake is a NO-OP THAT IS STILL
+ // HELD. A no-op answer without the field decodes as held:false and
+ // sends the client off to watch for an adopt that is a day away —
+ // exactly the false failure this field exists to prevent. Nothing is
+ // announced here (no bump): the pointer did not move.
+ writeJSON(w, http.StatusOK, map[string]any{
+ "result": "noop", "held": s.channelHeld(ctx, acct.Sqid, app, channel),
+ })
return
}
s.audit(acct, app, addr, "promote", "promoted "+version+" to "+channel+" (cli)")
s.bumpSerial(ctx)
- // held answers the CLI's next question without a second round trip: is the
- // thing I just promoted actually going to be adopted, or is it waiting out
- // a bake window? Two conditions, exactly as the browser door's flash
- // computes them (handlePromote, server.go): the CHANNEL has a window at
- // all, and the APP has declared production — a non-production app's boxes
- // adopt a held channel immediately, so a hold claim would be a lie.
- //
- // Only beta can be both here: canary/* and edge have zero-length windows,
- // and stable never reaches this line (the rung gate 403s it above). An
- // UNREADABLE flag reports held, matching what the boxes will actually do —
- // the adopt path fails TOWARD baking (spec §4), so reporting "not held"
- // there would be the one wrong answer.
- held := false
- if release.HoldFor(release.DefaultHolds, channel) > 0 {
- flag, ferr := release.GetProduction(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app)
- if ferr != nil {
- log.Printf("console: api promote production flag %s: %v", app, ferr)
- }
- held = ferr != nil || (flag != nil && flag.Production)
- }
writeJSON(w, http.StatusOK, map[string]any{
- "result": "promoted", "from": res.From, "to": res.To, "noop": false, "held": held,
+ "result": "promoted", "from": res.From, "to": res.To, "noop": false,
+ "held": s.channelHeld(ctx, acct.Sqid, app, channel),
})
}
+// channelHeld answers the CLI's next question without a second round trip: is
+// what this channel now carries actually going to be adopted, or is it waiting
+// out a bake window? Two conditions, exactly as the browser door's flash
+// computes them (handlePromote, server.go): the CHANNEL has a window at all,
+// and the APP has declared production — a non-production app's boxes adopt a
+// held channel immediately, so a hold claim would be a lie.
+//
+// Only beta can be both on the promote door: canary/* and edge have
+// zero-length windows, and stable never reaches it (the rung gate 403s it).
+// An UNREADABLE flag reports held, matching what the boxes will actually do —
+// the adopt path fails TOWARD baking (spec §4), so reporting "not held" there
+// would be the one wrong answer.
+func (s *Server) channelHeld(ctx context.Context, acct, app, channel string) bool {
+ if release.HoldFor(release.DefaultHolds, channel) == 0 {
+ return false
+ }
+ flag, err := release.GetProduction(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct, app)
+ if err != nil {
+ log.Printf("console: api promote production flag %s: %v", app, err)
+ return true
+ }
+ return flag != nil && flag.Production
+}
+
// handleAPIRollback answers `carlos rollback`: the CLI's write door onto the
// same swap-to-previous-version primitive as the console's own rollback
// button (handleRollback, server.go). Below stable there is no rung gate,
diff --git a/internal/console/serialbump_test.go b/internal/console/serialbump_test.go
index c3e03aaf7..e05c4272c 100644
--- a/internal/console/serialbump_test.go
+++ b/internal/console/serialbump_test.go
@@ -179,6 +179,51 @@ func TestAPIPromoteNotHeldOffProduction(t *testing.T) {
}
}
+// TestAPIPromoteNoOpReportsHeld: the re-run case. `carlos deploy` twice with
+// the same version — a CI retry, a repeated command — is a NO-OP, and on a
+// production app whose beta head is mid-bake it is a no-op that is STILL HELD.
+// A no-op answer without the field decodes as held:false and sends the client
+// off to watch for an adopt a day away, which is the exact false failure the
+// field exists to prevent.
+func TestAPIPromoteNoOpReportsHeld(t *testing.T) {
+ fx := newConsoleFixture(t)
+ if _, err := fx.st.CreateAppIn(OpsAccountID, "titogo", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ shipVersion(t, fx, "titogo", "abc1234")
+ seedChannel(t, fx, "titogo", "beta", "abc1234")
+ seedChannel(t, fx, "titogo", "canary/x", "abc1234")
+ if err := release.SetProduction(context.Background(), fx.rel, fx.priv,
+ release.OpsAccount, "titogo", true, "test"); err != nil {
+ t.Fatal(err)
+ }
+ token := cliToken(t, fx, "paul@keymail.dev", "code-serial-noopheld")
+
+ // beta already carries this version: a no-op, and held.
+ status, body := postRaw(t, fx.ts.URL+"/api/cli/apps/bab/titogo/promote",
+ map[string]string{"version": "abc1234", "channel": "beta"}, token)
+ if status != http.StatusOK || !strings.Contains(body, `"result":"noop"`) {
+ t.Fatalf("repeat beta promote = %d %s, want 200 noop", status, body)
+ }
+ if !held(t, body) {
+ t.Fatalf("no-op beta promote on a production app = %s, want held true", body)
+ }
+ // The same re-run on a rung with no window is a no-op that is NOT held —
+ // so the field tracks the bake, not merely the no-op.
+ status, body = postRaw(t, fx.ts.URL+"/api/cli/apps/bab/titogo/promote",
+ map[string]string{"version": "abc1234", "channel": "canary/x"}, token)
+ if status != http.StatusOK || !strings.Contains(body, `"result":"noop"`) {
+ t.Fatalf("repeat canary promote = %d %s, want 200 noop", status, body)
+ }
+ if held(t, body) {
+ t.Fatalf("no-op canary promote = %s, want held false — canaries never bake", body)
+ }
+ // Still no announcement: neither call moved a pointer.
+ if got := serial(t, fx); got != 0 {
+ t.Fatalf("serial after two no-op promotes = %d, want 0", got)
+ }
+}
+
// TestAPIRollbackBumpsSerial: a rollback moves the same pointer a promote
// does, and is the one people are in the biggest hurry about.
func TestAPIRollbackBumpsSerial(t *testing.T) {