carlos / platform Public

fix wave 1: the failure book's sequence fails closed before it could wrap

bumpLocked hands out nothing past MaxUint64-2: the book is exhausted, every save answers ErrSeqExhausted, nothing is published or cleared, and Snapshot reads 0 at the boundary.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude agent, operated by paul@keymail.dev 692d1533e79547d9b45422a220d75dca7953e546 parent f7e27b3
2 files changed, +86 −3
  • internal/rollout/failures.go +39 −3
  • internal/rollout/failures_epoch_test.go +47 −0
diff --git a/internal/rollout/failures.go b/internal/rollout/failures.go
index 2e46480e1..bdaf63f65 100644
--- a/internal/rollout/failures.go
+++ b/internal/rollout/failures.go
@@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"maps"
+ "math"
"os"
"path/filepath"
"slices"
@@ -193,6 +194,12 @@ func wakeIntent(id string) bool { return strings.HasPrefix(id, "w") }
// for the host: an attempt's, or a wake's that an attempt would replace. Its
// outcome is not yet written, so replacing its intent would lose it across a
// failed save and a restart.
+// ErrSeqExhausted is every save's answer once the book's sequence has run
+// out (seqLimit, fix wave 1, item 5): the book is never saved or published
+// again, so no generation repeats under its identity. Recovery is a new
+// book: drop the file's "book" key, or the file, and restart the agent.
+var ErrSeqExhausted = errors.New("rollout: the failure book's sequence is exhausted; it is never saved or published again (give the box a new book)")
+
var ErrIntentStands = errors.New("another execution on this instance has not recorded its outcome yet")
// reconciledReason is a reconciled intent's Reason. reasonWord reads it as
@@ -340,7 +347,8 @@ func genKey(acct, app string) string { return acct + "/" + app }
// when X is this book, memory equals disk, and A's generation now equals E
// with build still restricting host. Within a run every change takes a
// fresh larger value (1), so equality means no change of A's restrictions
-// since publication. Across a crash the reopened book starts at the disk's
+// since publication. The values never wrap: past seqLimit the book is
+// exhausted and never saved or published again (ErrSeqExhausted). Across a crash the reopened book starts at the disk's
// seq, which is at least E, and hands out larger values (3). A book written
// before the rule has another identity (4), and another book has another
// random identity (5). So G clears only the restriction state it was
@@ -393,10 +401,15 @@ type FailureBook struct {
// event exactly when its snapshot is not greater than the event's value.
// It is saved with the markers and the generations, so snapshots and
// generations after a restart are newer than every one on disk (rule 3).
- // The cleared map below is not saved; a host retry's clear moves the
+ // It never reaches seqLimit (bumpLocked). The cleared map below is not
+ // saved; a host retry's clear moves the
// app's generation, which is, so a spent grant stays spent across a
// restart.
seq uint64
+ // exhausted is set once a change needed a sequence value past the last
+ // usable one (bumpLocked): from then on no save lands and nothing is
+ // published (saveLocked), so memory never publishes a repeated value.
+ exhausted bool
// gens are the apps' restriction generations by genKey (rule 1), saved
// with the records. A signed host retry names the one it was issued for.
gens map[string]uint64
@@ -713,6 +726,10 @@ func (b *FailureBook) End(host, id string) error {
// durable_epochs, since everything this agent publishes is on disk first
// (rule 2).
func (b *FailureBook) saveLocked() error {
+ if b.exhausted {
+ b.dirty = true
+ return ErrSeqExhausted
+ }
if b.path == "" {
return nil
}
@@ -748,11 +765,30 @@ func (b *FailureBook) find(acct, app, build string) int {
func (b *FailureBook) Snapshot() uint64 {
b.mu.Lock()
defer b.mu.Unlock()
+ if b.exhausted || b.seq >= seqLimit-1 {
+ // At the boundary a snapshot would be seqLimit or more: "not known"
+ // instead, the safe answer (fix wave 1, item 5).
+ return 0
+ }
return b.seq + 1
}
-// bumpLocked takes the book's next sequence value. The caller holds b.mu.
+// seqLimit bounds the book's sequence (fix wave 1, item 5): every value it
+// hands out, and every snapshot, is below it, so the sequence never wraps
+// and no generation repeats under one identity. The last value handed out
+// is seqLimit-1 (math.MaxUint64-2).
+const seqLimit = math.MaxUint64 - 1
+
+// bumpLocked takes the book's next sequence value. At the boundary it hands
+// out nothing new: the book is exhausted and owes a save that never lands
+// (saveLocked), so nothing is published or cleared again, and the value it
+// returns (seqLimit, never a real one) is never seen outside memory. The
+// caller holds b.mu.
func (b *FailureBook) bumpLocked() uint64 {
+ if b.exhausted || b.seq >= seqLimit-1 {
+ b.exhausted, b.dirty = true, true
+ return seqLimit
+ }
b.seq++
return b.seq
}
diff --git a/internal/rollout/failures_epoch_test.go b/internal/rollout/failures_epoch_test.go
index 6f54b6888..7c525dd86 100644
--- a/internal/rollout/failures_epoch_test.go
+++ b/internal/rollout/failures_epoch_test.go
@@ -3,6 +3,7 @@ package rollout
import (
"encoding/json"
"errors"
+ "math"
"os"
"path/filepath"
"testing"
@@ -485,3 +486,49 @@ func TestABookThatCannotGetAnIdentityDoesNotOpen(t *testing.T) {
t.Fatalf("identity %q, want %q", b.ID(), durable.ID())
}
}
+
+// Fix wave 1, item 5 (Astra final review 1, finding 5): the sequence never
+// wraps under one identity. The largest value it hands out is
+// MaxUint64-2. A change that needs another refuses to save, so nothing is
+// published and no grant clears; a snapshot at the boundary reads "not
+// known" (0); and the file keeps the last value that landed, so a reopen
+// refuses the same way.
+func TestAnExhaustedSequenceFailsClosed(t *testing.T) {
+ b, dd := diskBook(t)
+ b.mu.Lock()
+ b.seq = math.MaxUint64 - 3
+ b.mu.Unlock()
+ mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h1"})
+ e := published(b, "v2", "h1")
+ if e != math.MaxUint64-2 {
+ t.Fatalf("published %d, want the last usable value %d", e, uint64(math.MaxUint64-2))
+ }
+ if s := b.Snapshot(); s != 0 {
+ t.Fatalf("Snapshot = %d at the boundary, want 0 (not known)", s)
+ }
+ if err := b.MarkHoldRan("h1", HoldRan{Account: "bab", App: "blog", Build: "v2", Exec: "e1"}); err == nil {
+ t.Fatal("a change that needs a value past the boundary was saved")
+ }
+ if p := published(b, "v2", "h1"); p != 0 {
+ t.Fatalf("published %d after the sequence ran out, want 0", p)
+ }
+ if rec, mark := grantAt(b, "v2", "h1", e); rec || mark {
+ t.Fatalf("a grant cleared record %v, marker %v on an exhausted book", rec, mark)
+ }
+ again, err := OpenFailureBook(dd)
+ if err != nil {
+ t.Fatal(err)
+ }
+ again.mu.Lock()
+ seq, gen := again.seq, again.gens[genKey("bab", "blog")]
+ again.mu.Unlock()
+ if seq != math.MaxUint64-2 || gen != e {
+ t.Fatalf("the file holds seq %d and generation %d, want the last landed %d", seq, gen, e)
+ }
+ if _, err := again.Record(FailureRecord{Account: "bab", App: "blog", Build: "v3", Host: "h2"}); err == nil {
+ t.Fatal("the reopened book saved a change past the boundary")
+ }
+ if p := published(again, "v2", "h1"); p != 0 {
+ t.Fatalf("the reopened book published %d after it ran out, want 0", p)
+ }
+}