amadan

carlos / platform Public

provision: the removal phase judges a row's own key only under a known account, and Reconcile arms its own context

Fix round 1. A record under a retired or unmaterialised account is never in
the pass's set, so its row has always been removed as record gone; observing
its key valid and in scope must not keep it. The judged branch now also
requires the listing's known set to hold the row's account; otherwise
today's rules apply. Reconcile wraps its context with WithLifetimes of
cfg.Judge (identity under nil).

Tests: TestRetiredAccountRowIsRemovedUnderAJudge (red before the fix),
TestRemovalProtectsACorruptObservation,
TestDeleteIfHoldRefusalLeavesTheRowForTheNextPass, and
TestOutOfScopeValidRecordFollowsTodaysRemoval runs every case judged and
unjudged, with the trash rule paired with a keep view.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
claude 4ea911082fcf54b7553b2e33b567e027186198e0 parent 57212a4
2 files changed, +143 −17
  • internal/provision/reconcile.go +8 −2
  • internal/provision/reconcile_lifetime_test.go +135 −15
diff --git a/internal/provision/reconcile.go b/internal/provision/reconcile.go
index 73dc7ed9a..fc69eb11d 100644
--- a/internal/provision/reconcile.go
+++ b/internal/provision/reconcile.go
@@ -832,6 +832,9 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg
if cfg.Now == nil {
cfg.Now = time.Now
}
+ // A pass with a judge reads and writes records judged and sequenced,
+ // whoever called it; a nil judge leaves ctx as it is.
+ ctx = WithLifetimes(ctx, cfg.Judge)
known, readFailed, err := knownAccounts(ctx, s)
if err != nil {
@@ -1605,12 +1608,15 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg
continue
}
why = "app in the trash"
- case cfg.Judge != nil && rt.Account != "" && rt.App != "":
+ case cfg.Judge != nil && rt.Account != "" && rt.App != "" && known[rt.Account]:
// The pass holds no record for this row under the row's own
// key: none for the host, or another owner's (a legacy
// collision, or a successor lifetime minted after this row's
// owner retired). Validity and placement are separate
- // questions, so the row's own key is observed afresh.
+ // questions, so the row's own key is observed afresh. Only
+ // under an account the listing trusts (known, this pass's
+ // set): a retired or unmaterialised account owns no record
+ // in scope, so its rows keep today's rules, record gone.
obs := cfg.Judge.Observe(ctx, s, rt.Account, rt.App, rt.Host)
switch {
case obs.Verdict == LifetimeUnreadable || obs.Verdict == LifetimeCorrupt:
diff --git a/internal/provision/reconcile_lifetime_test.go b/internal/provision/reconcile_lifetime_test.go
index 45244cbb7..63afdcef0 100644
--- a/internal/provision/reconcile_lifetime_test.go
+++ b/internal/provision/reconcile_lifetime_test.go
@@ -3,6 +3,7 @@ package provision
import (
"context"
"errors"
+ "fmt"
"os"
"strings"
"sync"
@@ -508,27 +509,146 @@ func TestOutOfScopeValidRecordFollowsTodaysRemoval(t *testing.T) {
{name: "another region, kept by the view", move: func(r *Record) { r.Region = "uk" }, keep: true, kept: true},
{name: "another pool, kept by the view", move: func(r *Record) { r.Pool = "other" }, keep: true, kept: true},
{name: "another region, the app trashed", move: func(r *Record) { r.Region = "uk" }, trash: true},
+ // Today an out-of-scope record is not in the pass's set, so its
+ // row's reason is record gone and the trash marker is never asked:
+ // the keep view keeps it, trashed or not. Judged, the same.
+ {name: "another region, the app trashed, kept by the view", move: func(r *Record) { r.Region = "uk" }, keep: true, trash: true, kept: true},
} {
- t.Run(c.name, func(t *testing.T) {
+ for _, judged := range []bool{false, true} {
+ t.Run(fmt.Sprintf("%s, judged %v", c.name, judged), func(t *testing.T) {
+ x := newLifeScene(t)
+ rec := x.record("shop", 1)
+ x.putRecord(t, rec)
+ x.putSlot(t, "shop", 1, SlotLive)
+ x.minted(t, "shop")
+ c.move(&rec)
+ x.putRecord(t, rec)
+ if !judged {
+ x.cfg.Judge = nil
+ }
+ if c.keep {
+ x.cfg.KeepParticipant = func(string) bool { return true }
+ }
+ if c.trash {
+ if err := release.TrashApp(ctx, x.s, x.sqid, "shop", "op@example.com", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ }
+ x.pass(t)
+ if got := x.row(t) != nil; got != c.kept {
+ t.Fatalf("row kept = %v, want %v", got, c.kept)
+ }
+ })
+ }
+ }
+}
+
+// A record under a retired account is never in the pass's set (the listing
+// trusts only known accounts), so its row has always been removed as record
+// gone. A judge observing the row's own key must not bring it back.
+func TestRetiredAccountRowIsRemovedUnderAJudge(t *testing.T) {
+ for _, judged := range []bool{false, true} {
+ t.Run(fmt.Sprintf("judged %v", judged), func(t *testing.T) {
x := newLifeScene(t)
- rec := x.record("shop", 1)
- x.putRecord(t, rec)
- x.putSlot(t, "shop", 1, SlotLive)
+ x.putRecord(t, x.record("shop", 0))
x.minted(t, "shop")
- c.move(&rec)
- x.putRecord(t, rec)
- if c.keep {
- x.cfg.KeepParticipant = func(string) bool { return true }
- }
- if c.trash {
- if err := release.TrashApp(ctx, x.s, x.sqid, "shop", "op@example.com", time.Now()); err != nil {
- t.Fatal(err)
- }
+ // The account is retired: its manifest is tombstoned. The record
+ // beside the row is valid (no slot) and in scope.
+ putReplace(t, x.s, "accounts/"+x.sqid+"/account.json",
+ []byte(`{"id":`+fmt.Sprint(acctID)+`,"sqid":"`+x.sqid+`","name":"Shop","created_at":"2026-08-02T09:00:00Z","deleted_at":"2026-10-01T09:00:00Z"}`))
+ if !judged {
+ x.cfg.Judge = nil
}
x.pass(t)
- if got := x.row(t) != nil; got != c.kept {
- t.Fatalf("row kept = %v, want %v", got, c.kept)
+ if rt := x.row(t); rt != nil {
+ t.Fatalf("a retired account's row survived: %+v", rt)
}
})
}
}
+
+// The removal phase's own observation finds the row's record corrupt: the
+// row is protected, where a readable record would let the trash rule
+// remove it.
+func TestRemovalProtectsACorruptObservation(t *testing.T) {
+ x := newLifeScene(t)
+ x.putRecord(t, x.record("shop", 0))
+ x.minted(t, "shop")
+ x.putRecord(t, x.record("cart", 0)) // cart sorts first: the pass selects it
+ if err := release.TrashApp(ctx, x.s, x.sqid, "shop", "op@example.com", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ // The listing reads shop's body cleanly; the removal phase's read of it
+ // (the second) finds it corrupt.
+ s := &corruptOnSecondRead{Store: x.s, key: RecordKey(x.sqid, "shop", lifeHost)}
+ if err := Reconcile(ctx, s, x.reg, x.cfg); err != nil {
+ t.Fatal(err)
+ }
+ if rt := x.row(t); rt == nil || rt.App != "shop" {
+ t.Fatalf("a corrupt observation removed the row: %+v", rt)
+ }
+ x.pass(t)
+ if rt := x.row(t); rt != nil && rt.App == "shop" {
+ t.Fatalf("fixture: readable, the trashed owner's row survived: %+v", rt)
+ }
+}
+
+// corruptOnSecondRead answers key's second and later reads with a body that
+// does not decode.
+type corruptOnSecondRead struct {
+ release.Store
+ key string
+ mu sync.Mutex
+ reads int
+}
+
+func (s *corruptOnSecondRead) Get(ctx context.Context, key string) ([]byte, string, error) {
+ raw, tag, err := s.Store.Get(ctx, key)
+ if key == s.key {
+ s.mu.Lock()
+ s.reads++
+ n := s.reads
+ s.mu.Unlock()
+ if n > 1 && err == nil {
+ return []byte("{not json"), tag, nil
+ }
+ }
+ return raw, tag, err
+}
+
+// A held row whose record is gone is deleted only while its op still holds
+// it. When the hold changed since the listing the row stays this pass, and
+// the next pass, listing the new hold, removes it.
+func TestDeleteIfHoldRefusalLeavesTheRowForTheNextPass(t *testing.T) {
+ x := newLifeScene(t)
+ x.putRecord(t, x.record("shop", 0))
+ x.minted(t, "shop")
+ if ok, err := x.reg.SetHoldIf(lifeHost, "moving", "mv_1", ""); err != nil || !ok {
+ t.Fatalf("hold = %v, %v", ok, err)
+ }
+ x.putSlot(t, "shop", 1, SlotRetired)
+ var calls []string
+ first := true
+ x.cfg.DeleteIfHold = func(host, op string) (bool, error) {
+ calls = append(calls, op)
+ if first {
+ // Another op takes the row between the listing and the delete.
+ first = false
+ if err := x.reg.SetHold(host, "moving", "mv_2"); err != nil {
+ t.Fatal(err)
+ }
+ }
+ return x.reg.DeleteIfHoldWithTimers(host, op)
+ }
+ x.pass(t)
+ if rt := x.row(t); rt == nil || rt.HoldOp != "mv_2" {
+ t.Fatalf("after the refused delete: %+v, want the row held by mv_2", rt)
+ }
+ x.pass(t)
+ if rt := x.row(t); rt != nil {
+ t.Fatalf("the next pass left %+v", rt)
+ }
+ if len(calls) != 2 || calls[0] != "mv_1" || calls[1] != "mv_2" {
+ t.Fatalf("DeleteIfHold calls %q, want mv_1 then mv_2", calls)
+ }
+}