amadan

carlos / platform Public

rollout: pin the marker-only and cross-app generation bumps; failure book schema 3 (Task 1 fix round 1)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 191c12b994cd4d72d352a3ef3a644d9013e1724e parent 7c7c614
4 files changed, +188 −51
  • internal/rollout/failures.go +50 −25
  • internal/rollout/failures_epoch_test.go +122 −14
  • internal/rollout/holdran_test.go +5 −5
  • internal/rollout/intent_test.go +11 −7
diff --git a/internal/rollout/failures.go b/internal/rollout/failures.go
index 35e359853..1a5878036 100644
--- a/internal/rollout/failures.go
+++ b/internal/rollout/failures.go
@@ -200,12 +200,18 @@ var ErrIntentStands = errors.New("another execution on this instance has not rec
// never recorded, which is all the box knows.
const reconciledReason = "the agent restarted before this execution recorded its outcome"
-// failureSchema is the schema the book is written as. Schema 2 added the
-// intents (Task 5). It is written as 2 whatever the file was, so an older
-// agent, which accepts only schema 1, refuses the file rather than read it
-// and drop its markers and intents (an agent downgrade with the flag on
-// refuses managed starts until the file is upgraded or removed).
-const failureSchema = 2
+// failureSchema is the schema the book is written as, whatever the file was,
+// so an older agent refuses the file rather than read it and break what the
+// newer one keeps. Schema 2 added the intents (Task 5): an agent before it
+// accepts only schema 1 and would drop markers and intents. Schema 3 is the
+// crash-epoch hard limit (readiness round 2, Task 1; FailureBook's doc): an
+// agent before it accepts schema 1 and 2, and would keep the book's
+// identity, publish epochs it never saved and drop durable_epochs, so a
+// grant it issued could later match a generation this rule hands out. Either
+// way an agent downgrade with the flag on refuses managed starts until the
+// file is upgraded or removed. This agent reads schema 1, 2 and 3, and gives
+// a file of schema 1 or 2 a new identity (rule 4).
+const failureSchema = 3
type failureFile struct {
Schema int `json:"schema"`
@@ -228,9 +234,9 @@ type failureFile struct {
Gens map[string]uint64 `json:"gens,omitempty"`
// Durable says the file was written under the crash-epoch rule (rule 4
// in FailureBook's doc): every generation it ever published was on disk
- // first. A file without it was written by an earlier agent, which may
- // have published epochs it never saved, so OpenFailureBook gives such a
- // book a new identity.
+ // first. A file without it, or of schema 1 or 2, was written by an
+ // earlier agent, which may have published epochs it never saved, so
+ // OpenFailureBook gives such a book a new identity.
Durable bool `json:"durable_epochs,omitempty"`
}
@@ -316,11 +322,12 @@ func genKey(acct, app string) string { return acct + "/" + app }
// with every saved generation, marker and epoch (highestSaved). So the
// disk's seq only grows, every published generation is at most it, and
// every value a later run hands out is larger.
-// 4. Adoption rotates the identity. A file without durable_epochs, written
-// by an earlier agent that may have published epochs it never saved,
-// opens with a new identity and owes a save (dirty), so nothing is
-// published until the rotation is on disk, and every grant issued
-// before names the old identity.
+// 4. Adoption rotates the identity. A file of schema 1 or 2, or without
+// durable_epochs, written by an earlier agent that may have published
+// epochs it never saved, opens with a new identity and owes a save
+// (dirty), so nothing is published until the rotation is on disk, and
+// every grant issued before names the old identity. The book is written
+// as schema 3 (failureSchema), which an earlier agent refuses to open.
// 5. The identity comes from crypto/rand only (newBookID). An open that
// needs one and cannot get one fails. Only a memory-only book, which
// tests and the simulation use, falls back to the clock.
@@ -338,6 +345,22 @@ func genKey(acct, app string) string { return acct + "/" + app }
// before the rule has another identity (4), and another book has another
// random identity (5). So G clears only the restriction state it was
// issued for.
+//
+// The guarantee rests on two assumptions about the file, which the book
+// cannot check from inside:
+//
+// - Only an agent of this rule writes it. An earlier agent would keep the
+// identity while publishing epochs it never saved, and write the file
+// back without durable_epochs. Schema 3 enforces this against a
+// downgrade (failureSchema): an earlier agent refuses the file. A hand
+// edit or another tool writing it is outside the guarantee.
+// - It is never restored from an older copy. A snapshot, a backup or a
+// cloned data dir carries the identity with a lower seq than the disk
+// last held, so a reopen from it can hand out again a generation
+// already published, and a stale grant naming that value would match
+// (rule 3 fails); a clone also gives two books one identity (rule 5
+// fails). Restoring or cloning a data dir must give the copy a new
+// identity: drop the file's "book" key, and the open makes one (rule 4).
type FailureBook struct {
path string
mu sync.Mutex
@@ -450,13 +473,13 @@ func cloneRec(r FailureRecord) FailureRecord {
// The sequence starts at the largest value the file holds (rule 3 in
// FailureBook's doc), so nothing handed out from here repeats a generation
// status published: that was on disk. A book with no file gets a new
-// identity, and so does a file without durable_epochs (rule 4): an earlier
-// agent wrote it and may have published epochs it never saved. Either owes
-// a save (dirty), as does an open that reconciled intents or gave older
-// entries epochs; nothing is published until it lands. An identity
-// crypto/rand cannot make is an error (rule 5): the caller treats the book
-// as unreadable. A book this rule wrote opens with no save owed unless the
-// open changed something.
+// identity, and so does a file of schema 1 or 2 or without durable_epochs
+// (rule 4): an earlier agent wrote it and may have published epochs it
+// never saved. Either owes a save (dirty), as does an open that reconciled
+// intents or gave older entries epochs; nothing is published until it
+// lands. An identity crypto/rand cannot make is an error (rule 5): the
+// caller treats the book as unreadable. A book this rule wrote opens with
+// no save owed unless the open changed something.
func OpenFailureBook(dataDir string) (*FailureBook, error) {
b := &FailureBook{path: filepath.Join(dataDir, FailureFile)}
raw, err := os.ReadFile(b.path)
@@ -475,8 +498,9 @@ func OpenFailureBook(dataDir string) (*FailureBook, error) {
if err := json.Unmarshal(raw, &f); err != nil {
return nil, fmt.Errorf("%s: %w", b.path, err)
}
- // Schema 1 is an agent before Task 5 (no intents); 2 is written now.
- if f.Schema != 1 && f.Schema != failureSchema {
+ // Schema 1 is an agent before Task 5 (no intents), 2 an agent before the
+ // crash-epoch rule; 3 is written now.
+ if f.Schema != 1 && f.Schema != 2 && f.Schema != failureSchema {
return nil, fmt.Errorf("%s: unknown schema %d", b.path, f.Schema)
}
b.recs = f.Records
@@ -493,9 +517,10 @@ func OpenFailureBook(dataDir string) (*FailureBook, error) {
b.dirty = true
}
}
- if !f.Durable || b.id == "" {
+ if f.Schema < failureSchema || !f.Durable || b.id == "" {
// Adoption of the rule (rule 4): grants issued before name the old
- // identity, or none.
+ // identity, or none. Only an agent of this rule writes schema 3, so a
+ // file of an older schema is adopted even if it says durable_epochs.
id, err := newBookID()
if err != nil {
return nil, fmt.Errorf("%s: %w", b.path, err)
diff --git a/internal/rollout/failures_epoch_test.go b/internal/rollout/failures_epoch_test.go
index 25e24508d..6f54b6888 100644
--- a/internal/rollout/failures_epoch_test.go
+++ b/internal/rollout/failures_epoch_test.go
@@ -157,22 +157,52 @@ func TestEveryRemovalMovesTheGeneration(t *testing.T) {
} {
t.Run(name, func(t *testing.T) {
b := setup(t)
- e := published(b, "v2", host)
- if e == 0 {
- t.Fatal("no generation published before the change")
- }
- change(t, b)
- now := published(b, "v2", host)
- if now == e {
- t.Fatalf("the generation stayed at %d across the change", e)
- }
- if now == 0 {
- t.Fatal("v2 still restricts h1 through its record or marker, but no generation is published")
- }
- if rec, mark := grantAt(b, "v2", host, e); rec || mark {
- t.Fatalf("a grant from before the change cleared record %v, marker %v", rec, mark)
+ movesAcross(t, b, "blog", "v2", host, func() { change(t, b) })
+ })
+ }
+ // Fix round 1, finding 1a: a grant that clears only a marker (no record
+ // of its build holds the host) moves the app's generation, so a grant at
+ // the value published before it for another build of the app clears
+ // nothing.
+ t.Run("a marker-only grant for another build", func(t *testing.T) {
+ b, _ := diskBook(t)
+ mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "sib", PromotedAt: promotedAt, At: t0})
+ mustMark(t, b, host, HoldRan{Account: "bab", App: "blog", Build: "v3", Exec: "a1", At: t0})
+ movesAcross(t, b, "blog", "v2", "h2", func() {
+ if rec, mark := grantAt(b, "v3", host, published(b, "v3", host)); rec || !mark {
+ t.Fatalf("the grant for v3 on h1 cleared record %v, marker %v; want the marker only", rec, mark)
}
})
+ })
+ // Fix round 1, finding 1b: a host's marker of one app replaced by another
+ // app's (the host now runs another app) moves the first app's generation.
+ t.Run("the marker replaced by another app's", func(t *testing.T) {
+ b := setup(t)
+ movesAcross(t, b, "blog", "v2", host, func() {
+ mustMark(t, b, host, HoldRan{Account: "bab", App: "shop", Build: "w1", Exec: "a2", At: t0})
+ })
+ })
+}
+
+// movesAcross checks that change moves bab/app's generation as build's
+// restriction of host publishes it, that build still restricts host after
+// it, and that a grant at the value published before it clears nothing.
+func movesAcross(t *testing.T, b *FailureBook, app, build, host string, change func()) {
+ t.Helper()
+ e := b.PublishedEpoch("bab", app, build, host)
+ if e == 0 {
+ t.Fatal("no generation published before the change")
+ }
+ change()
+ now := b.PublishedEpoch("bab", app, build, host)
+ if now == e {
+ t.Fatalf("the generation stayed at %d across the change", e)
+ }
+ if now == 0 {
+ t.Fatalf("%s still restricts %s through its record or marker, but no generation is published", build, host)
+ }
+ if rec, mark, _, _ := b.RetryHostAt("bab", app, build, host, b.ID(), e); rec || mark {
+ t.Fatalf("a grant from before the change cleared record %v, marker %v", rec, mark)
}
}
@@ -274,6 +304,84 @@ func TestABookFromBeforeTheRuleGetsANewIdentity(t *testing.T) {
}
}
+// Fix round 1, finding 2: the book is written as schema 3, which an agent
+// before the rule (it accepts schema 1 and 2) refuses, so it can never keep
+// the identity while publishing unsaved epochs. A schema-3 file round-trips
+// with its identity and owes no save. A file of schema 1 or 2 still opens,
+// with a new identity, even one that claims durable_epochs: only an agent of
+// this rule writes schema 3.
+func TestTheBookIsSchema3AndOlderSchemasOpenRotated(t *testing.T) {
+ t.Run("schema 3 round-trips", func(t *testing.T) {
+ b, dd := diskBook(t)
+ mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h1", HeldDown: []string{"h1"}})
+ e := published(b, "v2", "h1")
+ if e == 0 {
+ t.Fatal("a saved restriction published no generation")
+ }
+ if f := fileOf(t, dd); f.Schema != 3 || !f.Durable || f.Book != b.ID() {
+ t.Fatalf("file schema %d, durable_epochs %v, book %q: want 3, true, %q", f.Schema, f.Durable, f.Book, b.ID())
+ }
+ again, err := OpenFailureBook(dd)
+ if err != nil {
+ t.Fatalf("a schema-3 file did not open: %v", err)
+ }
+ if again.ID() != b.ID() || again.SaveOwed() || published(again, "v2", "h1") != e {
+ t.Fatalf("reopened as %q, save owed %v, at %d: want %q, false, %d",
+ again.ID(), again.SaveOwed(), published(again, "v2", "h1"), b.ID(), e)
+ }
+ if rec, _ := grantAt(again, "v2", "h1", e); !rec {
+ t.Fatal("a grant at the generation published before the reopen cleared nothing")
+ }
+ })
+ for name, file := range map[string]map[string]any{
+ "schema 1": {"schema": 1},
+ "schema 2": {"schema": 2},
+ "schema 2 with durable_epochs": {"schema": 2, "durable_epochs": true, "gens": map[string]uint64{"bab/blog": 10}},
+ } {
+ t.Run(name, func(t *testing.T) {
+ dd := t.TempDir()
+ file["book"], file["seq"] = "old", 10
+ file["records"] = []map[string]any{{"account": "bab", "app": "blog", "build": "v2", "host": "h1", "held_down": []string{"h1"}}}
+ raw, err := json.Marshal(file)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(filepath.Join(dd, FailureFile), raw, 0o644); err != nil {
+ t.Fatal(err)
+ }
+ b, err := OpenFailureBook(dd)
+ if err != nil {
+ t.Fatalf("the file did not open: %v", err)
+ }
+ if !b.HeldDown("bab", "blog", "v2", "h1") {
+ t.Fatal("the file's record was lost")
+ }
+ if id := b.ID(); id == "old" || id == "" {
+ t.Fatalf("identity %q, want a new one", id)
+ }
+ if !b.SaveOwed() || published(b, "v2", "h1") != 0 {
+ t.Fatalf("save owed %v, published %d: want the rotation owed and nothing published", b.SaveOwed(), published(b, "v2", "h1"))
+ }
+ if rec, mark, _, _ := b.RetryHostAt("bab", "blog", "v2", "h1", "old", 10); rec || mark {
+ t.Fatalf("a grant naming the old identity cleared record %v, marker %v", rec, mark)
+ }
+ if err := b.Flush(); err != nil {
+ t.Fatal(err)
+ }
+ if f := fileOf(t, dd); f.Schema != 3 || !f.Durable || f.Book != b.ID() {
+ t.Fatalf("written as schema %d, durable_epochs %v, book %q: want 3, true, %q", f.Schema, f.Durable, f.Book, b.ID())
+ }
+ e := published(b, "v2", "h1")
+ if e == 0 {
+ t.Fatal("published nothing after the rotation was saved")
+ }
+ if rec, _ := grantAt(b, "v2", "h1", e); !rec {
+ t.Fatal("a grant naming the new identity at the published generation cleared nothing")
+ }
+ })
+ }
+}
+
// Rule 3: a reopen starts from the disk's sequence, so every generation
// handed out after it is larger than every one published before it.
func TestAReopenNeverReissuesAPublishedGeneration(t *testing.T) {
diff --git a/internal/rollout/holdran_test.go b/internal/rollout/holdran_test.go
index 48a1447d2..2eb2aa930 100644
--- a/internal/rollout/holdran_test.go
+++ b/internal/rollout/holdran_test.go
@@ -453,9 +453,9 @@ func TestTheHoldRanMarkerSurvivesReopen(t *testing.T) {
// older agent wrote (no hold_ran key) opens with no markers. A file with
// markers still parses into the old shape (plain encoding/json skips the
// unknown key) with every record intact. Since Task 5 it is written as
-// schema 2, which an older agent's reader refuses on purpose rather than
-// read it and drop the markers and intents
-// (TestASchema2BookIsUnreadableToASchema1Reader).
+// schema 2, and since the crash-epoch hard limit as schema 3, which an older
+// agent's reader refuses on purpose rather than read it and drop the
+// markers and intents (TestTheBookIsUnreadableToAnOlderReader).
func TestTheHoldRanMarkerIsSchemaAdditive(t *testing.T) {
dir := t.TempDir()
old := `{"schema": 1, "records": [{"account": "bab", "app": "blog", "build": "v2", "host": "a", "cause": "release",
@@ -488,8 +488,8 @@ func TestTheHoldRanMarkerIsSchemaAdditive(t *testing.T) {
if err := json.Unmarshal(raw, &older); err != nil {
t.Fatalf("an older agent cannot parse the file with markers: %v", err)
}
- if older.Schema != 2 || len(older.Records) != 1 || older.Records[0].Build != "v2" {
- t.Fatalf("older agent's view = %+v: want schema 2 and the one record", older)
+ if older.Schema != 3 || len(older.Records) != 1 || older.Records[0].Build != "v2" {
+ t.Fatalf("older agent's view = %+v: want schema 3 and the one record", older)
}
if !strings.Contains(string(raw), `"hold_ran"`) {
t.Fatalf("the marker is not in the file:\n%s", raw)
diff --git a/internal/rollout/intent_test.go b/internal/rollout/intent_test.go
index d4aaa26a3..73e9cea50 100644
--- a/internal/rollout/intent_test.go
+++ b/internal/rollout/intent_test.go
@@ -127,7 +127,7 @@ func TestABeginThatCannotSaveRefusesAndLeavesNothing(t *testing.T) {
}
}
-func TestASchema1BookReadsAndIsWrittenAsSchema2(t *testing.T) {
+func TestASchema1BookReadsAndIsWrittenAsSchema3(t *testing.T) {
dd := t.TempDir()
raw := `{"schema":1,"records":[{"account":"bab","app":"blog","build":"v2","host":"h","cause":"release","mode":"overlap","reason":"x","promoted_at":"2026-10-01T00:00:00Z","at":"2026-10-01T00:00:00Z"}]}`
if err := os.WriteFile(filepath.Join(dd, FailureFile), []byte(raw), 0o644); err != nil {
@@ -141,17 +141,18 @@ func TestASchema1BookReadsAndIsWrittenAsSchema2(t *testing.T) {
t.Fatal(err)
}
out, _ := os.ReadFile(filepath.Join(dd, FailureFile))
- if !strings.Contains(string(out), `"schema": 2`) {
+ if !strings.Contains(string(out), `"schema": 3`) {
t.Fatalf("written as %s", out)
}
if _, err := OpenFailureBook(dd); err != nil {
- t.Fatalf("schema 2 written, then refused: %v", err)
+ t.Fatalf("schema 3 written, then refused: %v", err)
}
}
// Review Focus 2: what an older agent sees. Its reader refused anything
-// but schema 1 (OpenFailureBook before Task 5).
-func TestASchema2BookIsUnreadableToASchema1Reader(t *testing.T) {
+// but schema 1 (OpenFailureBook before Task 5), or but schema 1 and 2
+// (before the crash-epoch hard limit, readiness round 2 Task 1, fix round 1).
+func TestTheBookIsUnreadableToAnOlderReader(t *testing.T) {
var f struct {
Schema int `json:"schema"`
}
@@ -167,10 +168,13 @@ func TestASchema2BookIsUnreadableToASchema1Reader(t *testing.T) {
if f.Schema == 1 {
t.Fatal("an old reader would accept the file and drop markers and intents")
}
+ if f.Schema == 2 {
+ t.Fatal("a reader before the crash-epoch rule would accept the file, keep its identity and publish unsaved epochs")
+ }
}
func TestABookOfAnUnknownSchemaIsRefused(t *testing.T) {
- for _, schema := range []string{"0", "3"} {
+ for _, schema := range []string{"0", "4"} {
dd := t.TempDir()
if err := os.WriteFile(filepath.Join(dd, FailureFile), []byte(`{"schema":`+schema+`,"records":[]}`), 0o644); err != nil {
t.Fatal(err)
@@ -244,7 +248,7 @@ func TestAnIntentLeftOnDiskBecomesItsFailure(t *testing.T) {
if err := b2.Flush(); err != nil {
t.Fatal(err)
}
- if f := fileOf(t, dd); len(f.Intents) != 0 || len(f.Records) != 2 || f.Schema != 2 {
+ if f := fileOf(t, dd); len(f.Intents) != 0 || len(f.Records) != 2 || f.Schema != failureSchema {
t.Fatalf("after the flush: %+v", f)
}
if b3, _ := OpenFailureBook(dd); b3.Reconciled() != 0 || !b3.HeldDown("bab", "blog", "v2", "b") {