amadan
ship: the program rule judges only a new version
An identical re-ship of a version already on record stays idempotent. The API door runs CheckShipProgram after the already-shipped short-circuit; direct mode skips it when the version's manifest exists.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
3 files changed,
+62
−19
cli.go+7 −3internal/console/apicli.go+19 −16internal/console/apicli_test.go+36 −0
diff --git a/cli.go b/cli.go| index eaa178390..d0e329fa1 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -2192,9 +2192,13 @@ func runShip(args []string) { |
| } |
| // The wrong-program rule, shared with the console's API door: refuse a |
| // release whose Go main package differs from the app's previous release |
| - // unless --program-change says it is meant. |
| - if err := release.CheckShipProgram(context.Background(), st, acct, &m, files, *programChange); err != nil { |
| - log.Fatal(err) |
| + // unless --program-change says it is meant. Only a NEW version meets it: |
| + // re-shipping a version already on record stays idempotent, and |
| + // release.Ship below decides between identical and conflict as before. |
| + if _, gerr := release.GetManifest(context.Background(), st, acct, *app, shipVersion); gerr != nil { |
| + if err := release.CheckShipProgram(context.Background(), st, acct, &m, files, *programChange); err != nil { |
| + log.Fatal(err) |
| + } |
| } |
| if err := release.Ship(context.Background(), st, acct, m, files); err != nil { |
| log.Fatal(err) |
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index 825f204b5..31b324ab7 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -2001,22 +2001,6 @@ func (s *Server) handleAPIShip(w http.ResponseWriter, r *http.Request) { |
| Label: meta.Label, |
| Notes: meta.Notes, |
| } |
| - // The wrong-program rule, the same one the direct-bucket `carlos ship` |
| - // calls: a release whose Go main package differs from the app's previous |
| - // release is refused unless the caller said --program-change. It reads |
| - // the artifact bytes that arrived, so it runs here, after m exists and |
| - // before anything is written. A mismatch is a 409 (the request is well |
| - // formed, the app's history disagrees); anything else is a 400, the |
| - // artifact or the app's releases being unreadable. |
| - if err := release.CheckShipProgram(r.Context(), s.cfg.Releases, acct.Sqid, &m, files, meta.ProgramChange); err != nil { |
| - var pm *release.ProgramMismatchError |
| - if errors.As(err, &pm) { |
| - jsonError(w, http.StatusConflict, err.Error()) |
| - return |
| - } |
| - jsonError(w, http.StatusBadRequest, err.Error()) |
| - return |
| - } |
| // Longer than the other CLI doors' 15-20s: this one moves real bytes — |
| // up to apiShipMaxBytes of them — into the bucket, not a handful of |
| // short strings. |
| @@ -2079,6 +2063,25 @@ func (s *Server) handleAPIShip(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| + // The wrong-program rule, the same one the direct-bucket `carlos ship` |
| + // calls: a release whose Go main package differs from the app's previous |
| + // release is refused unless the caller said --program-change. It reads |
| + // the artifact bytes that arrived, so it runs after m exists and before |
| + // anything is written, but AFTER the identical-reship short-circuit |
| + // above: re-shipping a version that is already on record must stay |
| + // idempotent (a CI retry of an older version after a program change is |
| + // still a 200), so only a NEW version meets the rule. A mismatch is a 409 (the request is well |
| + // formed, the app's history disagrees); anything else is a 400, the |
| + // artifact or the app's releases being unreadable. |
| + if err := release.CheckShipProgram(ctx, s.cfg.Releases, acct.Sqid, &m, files, meta.ProgramChange); err != nil { |
| + var pm *release.ProgramMismatchError |
| + if errors.As(err, &pm) { |
| + jsonError(w, http.StatusConflict, err.Error()) |
| + return |
| + } |
| + jsonError(w, http.StatusBadRequest, err.Error()) |
| + return |
| + } |
| if err := release.Ship(ctx, s.cfg.Releases, acct.Sqid, m, files); err != nil { |
| log.Printf("console: api ship %s %s: %v", app, ver, err) |
| if shipImmutableConflict(err) { |
diff --git a/internal/console/apicli_test.go b/internal/console/apicli_test.go| index c652091b7..1860b22d0 100644 |
| --- a/internal/console/apicli_test.go |
| +++ b/internal/console/apicli_test.go |
| @@ -1426,6 +1426,42 @@ func TestAPIShipRefusesAProgramChange(t *testing.T) { |
| } |
| } |
| +// TestAPIShipReshipAfterAProgramChangeStaysIdempotent: the wrong-program rule |
| +// judges only a NEW version. Re-shipping v1 with identical bytes after the app |
| +// moved on to another program (v2, shipped with program_change) is still the |
| +// idempotent 200, not a 409 against v2's program. |
| +func TestAPIShipReshipAfterAProgramChangeStaysIdempotent(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + if _, err := fx.st.CreateAppIn(OpsAccountID, "titogo", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-ship-program-reship") |
| + liftShipBudget(fx) |
| + url := fx.ts.URL + "/api/cli/apps/bab/titogo/releases" |
| + console := tinyProgramForConsole(t, "example.com/console") |
| + carlos := tinyProgramForConsole(t, "example.com/carlos") |
| + |
| + status, out := postShip(t, url, token, |
| + []shipPart{metaPart(t, "v1", "binary"), {name: "linux-arm64", body: console}}) |
| + if status != http.StatusCreated { |
| + t.Fatalf("v1 = %d %v", status, out) |
| + } |
| + raw, err := json.Marshal(map[string]any{"version": "v2", "kind": "binary", "program_change": true}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + status, out = postShip(t, url, token, |
| + []shipPart{{name: "meta", body: raw}, {name: "linux-arm64", body: carlos}}) |
| + if status != http.StatusCreated { |
| + t.Fatalf("v2 with program_change = %d %v", status, out) |
| + } |
| + status, out = postShip(t, url, token, |
| + []shipPart{metaPart(t, "v1", "binary"), {name: "linux-arm64", body: console}}) |
| + if status != http.StatusOK { |
| + t.Fatalf("identical re-ship of v1 = %d %v, want 200 (idempotent)", status, out) |
| + } |
| +} |
| + |
| // TestAPIShipConcurrencyLimit: the rate limiter bounds requests over TIME, |
| // but says nothing about how many run AT ONCE. Two ships already in flight |
| // (each holding the server's ship semaphore while its body trickles in, |