amadan
Merge pull request #258 from carlosframework/pipelines-v2-residuals
pipelines v2 follow-ups: ratchet-backed bake verdicts, one-channel canary visibility, execution re-checks
16 files changed,
+1629
−86
internal/console/apicli_test.go+36 −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/view.go+29 −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 −7
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/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 d3764c96c..424b8792d 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -2354,8 +2354,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/view.go b/internal/console/view.go| index 43ea3c7e3..214a4fa5a 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/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 { |