amadan

carlos / platform Public

release: fix production-memory review findings — corrupt clocks fail toward baking, prodKey folds ops account, ratchet mutants closed

Task-4 review found the box's own corrupt clock file (read but unparsed)
silently collapsed to "no memory", making ProductionBake return false and
un-baking every production app on the box — the opposite of its fail-toward
-baking design. LoadFirstSeen now marks the clock tainted when the file
reads but doesn't parse (distinct from simply absent), and ProductionBake
checks that first. Also: prodKey now folds "" and OpsAccount to the same
key like channelClockKey does; SetProduction bumps SetAt past an equal or
regressed existing stamp so a same-second double flip can't share a
set_at and get swallowed by the box-side replay floor; prodMemory's time
fields use omitzero; and five scenarios (equal-set_at re-read, off-state
persistence, jitter, off-only apps, ops-account folding) get their own
regression tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VioXzogWf1Xi4Rswz2B6EK
Paul Campbell d69801d9c2974d1b10019d7b6e123514f4477159 parent a28bbbc
5 files changed, +247 −17
  • docs/superpowers/specs/2026-08-04-console-bake-controls-design.md +4 −1
  • internal/release/bake.go +75 −14
  • internal/release/bake_test.go +125 −0
  • internal/release/production.go +13 −2
  • internal/release/production_test.go +30 −0
diff --git a/docs/superpowers/specs/2026-08-04-console-bake-controls-design.md b/docs/superpowers/specs/2026-08-04-console-bake-controls-design.md
index baf25c278..1f6b5f31d 100644
--- a/docs/superpowers/specs/2026-08-04-console-bake-controls-design.md
+++ b/docs/superpowers/specs/2026-08-04-console-bake-controls-design.md
@@ -46,7 +46,10 @@ promotions; its boxes just don't sit out the hold.
Live pointers and manifests exist in production buckets; everything written
before this change must still parse (repo rule). Both shapes here are
additive, and both fail SAFE on old binaries: a box that predates this feature
-keeps baking.
+keeps baking. The same additive rule applies to the box's own bake-clock
+file: an old binary that Saves it drops the production key it doesn't know
+about until that box is restarted onto the new binary; the loss self-heals
+in the closed direction (§4).
**The flag object** is a new signed bucket object per app, in the same Ed25519
envelope as channel pointers, root-key-signed:
diff --git a/internal/release/bake.go b/internal/release/bake.go
index dfb5c8a9e..76536775e 100644
--- a/internal/release/bake.go
+++ b/internal/release/bake.go
@@ -87,9 +87,24 @@ type FirstSeen struct {
Channels map[string]map[string]time.Time `json:"channels"`
// Production is the per-app production-flag memory (2026-08-04 console
- // bake controls spec §4), keyed "<acct>|<app>" with clockSep — the same
- // collision argument as channelClockKey. ADDITIVE: old clock files have
- // no key, old binaries drop this one.
+ // bake controls spec §4), keyed with clockSep — the same SEPARATOR
+ // argument as channelClockKey (a channel segment may contain "/"; an
+ // account or app name never contains clockSep). prodKey folds "" and
+ // OpsAccount to the same key — there is no legacy bare shape to
+ // preserve here, this memory is new, so callers can pass whatever
+ // RouteAccount(rt) (adopt.go) returns and land on the right memory
+ // either way.
+ //
+ // ADDITIVE, but only one pass deep: an OLD binary that Saves this file
+ // re-marshals the struct IT knows about, which has no Production field —
+ // its write DELETES every app's production memory from the shared file.
+ // That is not silently unsafe: the latch re-latches from the bucket flag
+ // the very next pass a new binary reads it (true stays true), and a lost
+ // ratchet clock simply restarts from that pass's sighting (an
+ // in-progress off-ratchet gets pushed out, never skipped) — both are the
+ // closed direction on their own. A mixed-version rollout only ever costs
+ // up to one pass of this memory; it never lets an off-flag through
+ // early, and never lets a deleted true downgrade anything either.
Production map[string]*prodMemory `json:"production,omitempty"`
path string // where Save persists; set by LoadFirstSeen
@@ -100,6 +115,12 @@ type FirstSeen struct {
// change this pass — see LoadFirstSeenMigrating's doc comment for why
// a changed-gated Save alone isn't enough to make the migration durable.
migrated bool
+
+ // tainted marks a clock whose file read succeeded but did NOT parse as
+ // JSON — distinct from a missing file, which is just a fresh, empty
+ // clock. Never serialized (unexported); see LoadFirstSeen and
+ // ProductionBake for why the two failure modes must be told apart.
+ tainted bool
}
// channelClockKey identifies one channel head's clock. The ACCOUNT is part of
@@ -134,13 +155,32 @@ func channelClockKey(acct, app, channel string) string {
// name (appRe) or a channel segment (channelSegRe).
const clockSep = "|"
-// LoadFirstSeen reads the bake clock from path. A missing or corrupt file is
-// an empty clock — every head then bakes from "now", which fails closed (a
-// wiped clock delays updates; it never lets one through early).
+// LoadFirstSeen reads the bake clock from path. For the CHANNEL clock, a
+// missing OR corrupt file is simply an empty clock — every head then bakes
+// from "now", which fails closed (a wiped clock delays updates; it never
+// lets one through early either way).
+//
+// For the PRODUCTION memory (§4), those two failure modes must be told
+// apart. A file that reads but does not PARSE marks the clock tainted, and
+// ProductionBake fails toward baking for every held head on a tainted
+// clock — the same closed direction, reached differently, because an
+// unmarshal error can leave Production nil or half-written rather than
+// usably empty. A file that is simply ABSENT, though, is indistinguishable
+// from a fresh box: the same read that lets the channel clock safely
+// restart "from now" also silently loses the production latch and the
+// off-ratchet's replay floor, because there is nothing on disk to say a
+// latch ever existed. That gap is accepted: the latch defends against
+// BUCKET-side tampering (someone with bucket write but no key deleting the
+// flag object) — a box's own /data is a different trust boundary. Whatever
+// already has write access there already holds adopt authority, i.e.
+// already controls what this box installs; deleting its bake-clock file
+// buys it nothing a corrupt bucket object would.
func LoadFirstSeen(path string) *FirstSeen {
st := &FirstSeen{path: path}
if raw, err := os.ReadFile(path); err == nil {
- _ = json.Unmarshal(raw, st)
+ if err := json.Unmarshal(raw, st); err != nil {
+ st.tainted = true
+ }
}
if st.Channels == nil {
st.Channels = map[string]map[string]time.Time{}
@@ -327,15 +367,29 @@ type prodMemory struct {
WasProduction bool `json:"was_production,omitempty"`
// NewestSetAt is the replay floor: the newest set_at this box has
// honored. Anything at or below it is old news.
- NewestSetAt time.Time `json:"newest_set_at,omitempty"`
- // OffSetAt identifies the current off-flag edition; OffFirstSeen is when
- // THIS BOX first saw it — the ratchet clock, the box's own observation,
- // never the flag's timestamp (same argument as the bake clock itself).
- OffSetAt time.Time `json:"off_set_at,omitempty"`
- OffFirstSeen time.Time `json:"off_first_seen,omitempty"`
+ NewestSetAt time.Time `json:"newest_set_at,omitzero"`
+ // OffSetAt is the discriminator: non-zero means the newest verified word
+ // for this app is off (only ever read via IsZero — its VALUE isn't
+ // otherwise consulted; NewestSetAt already carries the replay floor).
+ // OffFirstSeen is when THIS BOX first saw that off-flag edition — the
+ // ratchet clock, the box's own observation, never the flag's own
+ // timestamp (same argument as the bake clock itself).
+ OffSetAt time.Time `json:"off_set_at,omitzero"`
+ OffFirstSeen time.Time `json:"off_first_seen,omitzero"`
}
-func prodKey(acct, app string) string { return acct + clockSep + app }
+// prodKey identifies one app's production memory. Joined with clockSep for
+// the same reason channelClockKey is (an app or account name can never
+// contain it, unlike a channel segment), and it folds "" and OpsAccount to
+// the same key — unlike channelClockKey there's no legacy bare shape to
+// keep separate, this memory is new, so RouteAccount(rt)'s either form
+// always resolves to the one record.
+func prodKey(acct, app string) string {
+ if acct == "" || acct == OpsAccount {
+ acct = OpsAccount
+ }
+ return acct + clockSep + app
+}
// ObserveProduction folds one VERIFIED flag read into this box's memory.
// nil (no object in the bucket) changes nothing — absence is not evidence.
@@ -378,6 +432,13 @@ func (st *FirstSeen) ObserveProduction(acct, app string, flag *ProductionFlag, n
// resolved this pass, an off-flag needs no ratchet — there is nothing at a
// baked head to protect.
func (st *FirstSeen) ProductionBake(acct, app string, flagErr error, hasBakeableHead bool, holds map[string]time.Duration, now time.Time) bool {
+ if st.tainted {
+ // This box's own clock file read but did not parse — untrustworthy
+ // in a way absence never is. Fail toward baking for every held
+ // head, same direction as flagErr, until the next Save overwrites
+ // it with something this binary wrote.
+ return true
+ }
if flagErr != nil {
return true
}
diff --git a/internal/release/bake_test.go b/internal/release/bake_test.go
index a4ca340c1..f597123a7 100644
--- a/internal/release/bake_test.go
+++ b/internal/release/bake_test.go
@@ -466,4 +466,129 @@ func TestProductionMemoryRatchet(t *testing.T) {
t.Fatal("production memory did not survive persistence")
}
})
+
+ // Task-4-review fix 2: prodKey must fold the ops account the same way
+ // channelClockKey does, or an app addressed as "" (the pre-accounts
+ // bare shape some callers still use) and the same app addressed as
+ // OpsAccount silently keep SEPARATE production memories.
+ t.Run("prodKey folds the ops account like channelClockKey", func(t *testing.T) {
+ st := newClock()
+ st.ObserveProduction(OpsAccount, "kass", flag(true, base), base)
+ if !st.ProductionBake("", "kass", nil, true, DefaultHolds, base) {
+ t.Fatal(`observed under OpsAccount, not found evaluating under ""`)
+ }
+
+ st2 := newClock()
+ st2.ObserveProduction("", "kass", flag(true, base), base)
+ if !st2.ProductionBake(OpsAccount, "kass", nil, true, DefaultHolds, base) {
+ t.Fatal(`observed under "", not found evaluating under OpsAccount`)
+ }
+ // No free ride for an unrelated account: "bab"/"kass" (the ops
+ // account) must not satisfy "zzz"/"kass".
+ if st2.ProductionBake("zzz", "kass", nil, true, DefaultHolds, base) {
+ t.Fatal("an unrelated account satisfied the ops account's production memory")
+ }
+ })
+
+ // Task-4-review fix 3a: re-reading the SAME off object (the every-pass
+ // re-read, not a fresh sighting) must not change anything — in
+ // particular must not push the ratchet clock forward from the box's
+ // FIRST sighting to whatever pass happens to re-read it.
+ t.Run("equal set_at re-read does not reset the ratchet clock", func(t *testing.T) {
+ st := newClock()
+ st.ObserveProduction("bab", "kass", flag(true, base), base)
+ off := flag(false, base.Add(time.Hour))
+ st.ObserveProduction("bab", "kass", off, base.Add(2*time.Hour)) // first sighting
+ if st.ObserveProduction("bab", "kass", off, base.Add(3*time.Hour)) {
+ t.Fatal("re-reading the identical off object reported a change")
+ }
+ hold := HoldFor(DefaultHolds, RungStable)
+ // Still measured from the FIRST sighting (base+2h), not the re-read
+ // (base+3h) — a re-read one hour later must not buy the flag one
+ // more hour of ratchet.
+ if !st.ProductionBake("bab", "kass", nil, true, DefaultHolds, base.Add(2*time.Hour).Add(hold-time.Minute)) {
+ t.Fatal("off honored before the ratchet from its first sighting elapsed")
+ }
+ if st.ProductionBake("bab", "kass", nil, true, DefaultHolds, base.Add(2*time.Hour).Add(hold+time.Minute)) {
+ t.Fatal("re-reading the same object pushed the ratchet clock forward")
+ }
+ })
+
+ // Task-4-review fix 3b: an in-progress off-ratchet (not just a bare
+ // "on" latch) must itself survive Save/LoadFirstSeen.
+ t.Run("off-state ratchet survives a save/load round trip", func(t *testing.T) {
+ st := newClock()
+ st.ObserveProduction("bab", "kass", flag(true, base), base)
+ st.ObserveProduction("bab", "kass", flag(false, base.Add(time.Hour)), base.Add(time.Hour))
+ if err := st.Save(); err != nil {
+ t.Fatal(err)
+ }
+ st2 := LoadFirstSeen(st.path)
+ hold := HoldFor(DefaultHolds, RungStable)
+ if !st2.ProductionBake("bab", "kass", nil, true, DefaultHolds, base.Add(time.Hour).Add(hold-time.Minute)) {
+ t.Fatal("off-state ratchet lost on reload: should still be baking mid-window")
+ }
+ if st2.ProductionBake("bab", "kass", nil, true, DefaultHolds, base.Add(time.Hour).Add(hold+time.Minute)) {
+ t.Fatal("off-state ratchet lost on reload: should be off past the window")
+ }
+ })
+
+ // Task-4-review fix 3c: the ratchet is jittered exactly like any other
+ // hold (HoldFor's formula), not a bare, un-jittered 72h.
+ t.Run("the ratchet is jittered like any other hold", func(t *testing.T) {
+ st := LoadFirstSeen(filepath.Join(t.TempDir(), "clock.json"))
+ st.Jitter = 0.2
+ st.ObserveProduction("bab", "kass", flag(true, base), base)
+ st.ObserveProduction("bab", "kass", flag(false, base.Add(time.Hour)), base.Add(time.Hour))
+ hold := HoldFor(DefaultHolds, RungStable) // 72h, unjittered
+ if !st.ProductionBake("bab", "kass", nil, true, DefaultHolds, base.Add(time.Hour).Add(hold).Add(time.Minute)) {
+ t.Fatal("jitter not applied: off honored right at the un-jittered window")
+ }
+ jittered := time.Duration(float64(hold) * 1.2)
+ if st.ProductionBake("bab", "kass", nil, true, DefaultHolds, base.Add(time.Hour).Add(jittered).Add(time.Minute)) {
+ t.Fatal("jitter not applied: off still honored past the jittered window")
+ }
+ })
+
+ // Task-4-review fix 3d: an app declared off and NEVER on has nothing to
+ // ratchet — it must never bake, immediately and forever.
+ t.Run("an app declared off and never on never bakes", func(t *testing.T) {
+ st := newClock()
+ st.ObserveProduction("bab", "kass", flag(false, base), base)
+ if st.ProductionBake("bab", "kass", nil, true, DefaultHolds, base) {
+ t.Fatal("an app declared off, never on, baked immediately")
+ }
+ if st.ProductionBake("bab", "kass", nil, true, DefaultHolds, base.Add(1000*time.Hour)) {
+ t.Fatal("an app declared off, never on, baked eventually")
+ }
+ })
+}
+
+// Task-4-review fix 1 (Critical): LoadFirstSeen must distinguish a file that
+// read but did not PARSE from a file that is simply absent. Before this fix
+// both collapsed to the same "empty clock" result, which for the CHANNEL
+// clock is fine (fails closed either way — everything re-bakes from now),
+// but for the PRODUCTION memory silently failed OPEN: a corrupt clock file
+// (mem == nil for every app) made ProductionBake return false, instantly
+// un-baking every production app on the box. A corrupt file must instead
+// fail TOWARD baking (st.tainted), while an absent file — a genuinely fresh
+// box — still gets the ordinary false default.
+func TestCorruptClockFailsTowardBakingNotAwayFromIt(t *testing.T) {
+ base := time.Date(2026, 8, 4, 12, 0, 0, 0, time.UTC)
+
+ path := filepath.Join(t.TempDir(), "clock.json")
+ if err := os.WriteFile(path, []byte("{ this is not json"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ st := LoadFirstSeen(path)
+ if !st.ProductionBake("bab", "kass", nil, true, DefaultHolds, base) {
+ t.Fatal("a corrupt clock file did not fail toward baking")
+ }
+
+ // A genuinely fresh box (no file at all) is a different case entirely —
+ // still the ordinary false default, not tainted.
+ fresh := LoadFirstSeen(filepath.Join(t.TempDir(), "absent.json"))
+ if fresh.ProductionBake("bab", "kass", nil, true, DefaultHolds, base) {
+ t.Fatal("a fresh box with no clock file baked by default")
+ }
}
diff --git a/internal/release/production.go b/internal/release/production.go
index d22c4f010..b4a679c49 100644
--- a/internal/release/production.go
+++ b/internal/release/production.go
@@ -62,15 +62,26 @@ func SetProduction(ctx context.Context, s Store, priv ed25519.PrivateKey, acct,
return fmt.Errorf("set production: invalid app %q", app)
}
key := ProductionKey(acct, app)
- _, etag, err := s.Get(ctx, key)
+ raw, etag, err := s.Get(ctx, key)
if errors.Is(err, os.ErrNotExist) {
etag = "" // first write: create-only
} else if err != nil {
return fmt.Errorf("set production: read %s: %w", key, err)
}
+ setAt := time.Now().UTC().Truncate(time.Second)
+ if err == nil {
+ // SetAt is truncated to the second; two flips inside one wall-clock
+ // second would otherwise share a value, and the box-side replay
+ // floor (bake.go's NewestSetAt) treats an equal set_at as nothing
+ // new — silently swallowing the second flip on every box. Bump
+ // strictly past whatever this key's own last signed write recorded.
+ if existing, verr := verifyProductionFlag(priv.Public().(ed25519.PublicKey), raw, acct, app); verr == nil && !existing.SetAt.Before(setAt) {
+ setAt = existing.SetAt.Add(time.Second)
+ }
+ }
f := &ProductionFlag{
Schema: 1, Account: acct, App: app, Production: production,
- Actor: actor, SetAt: time.Now().UTC().Truncate(time.Second),
+ Actor: actor, SetAt: setAt,
}
if err := validateProductionFlag(f, acct, app); err != nil {
return fmt.Errorf("set production: %w", err)
diff --git a/internal/release/production_test.go b/internal/release/production_test.go
index d37e79d12..50b86c9ab 100644
--- a/internal/release/production_test.go
+++ b/internal/release/production_test.go
@@ -205,3 +205,33 @@ func TestProductionFlagRefusesTamperedPayload(t *testing.T) {
// timeNowTestHelper is a stamped, truncated now — what signProductionFlag
// itself writes — for tests that hand-build payloads.
func timeNowTestHelper() time.Time { return time.Now().UTC().Truncate(time.Second) }
+
+// Task-4-review fix 5 (Minor, promoted): SetAt is truncated to the second,
+// so two flips within the same wall-clock second would otherwise share a
+// set_at — and the box-side replay floor (bake.go's NewestSetAt) treats an
+// equal set_at as nothing new, silently swallowing the second flip on every
+// box that already saw the first. Two back-to-back SetProduction calls (as
+// fast as a test loop runs them, i.e. very likely within one truncated
+// second) must still produce strictly increasing set_ats.
+func TestSetProductionMonotonicSetAtOnRapidFlip(t *testing.T) {
+ s := NewDirStore(t.TempDir())
+ ctx := context.Background()
+ priv, pub := testKeys(t)
+ if err := SetProduction(ctx, s, priv, "bab", "kass", true, "paul@keymail.dev"); err != nil {
+ t.Fatalf("SetProduction on: %v", err)
+ }
+ f1, err := GetProduction(ctx, s, pub, "bab", "kass")
+ if err != nil || f1 == nil {
+ t.Fatalf("GetProduction after first flip: %v, %v", f1, err)
+ }
+ if err := SetProduction(ctx, s, priv, "bab", "kass", false, "paul@keymail.dev"); err != nil {
+ t.Fatalf("SetProduction off: %v", err)
+ }
+ f2, err := GetProduction(ctx, s, pub, "bab", "kass")
+ if err != nil || f2 == nil {
+ t.Fatalf("GetProduction after second flip: %v, %v", f2, err)
+ }
+ if !f2.SetAt.After(f1.SetAt) {
+ t.Fatalf("second flip's set_at %v not strictly after first's %v (same-second collision not bumped)", f2.SetAt, f1.SetAt)
+ }
+}