Final fix 3: held refusals get their own namespace
Held-refusal cleanup read refusals/<host>.json and then removed it; an adoption refusal written between the two (adoption is not serialised with the held stage across processes) was deleted with it. Held refusals now live at <bake>/moves-held/refusals/<host>.json, written, read, listed and removed with refusal.go's own functions over that base. The held writer no longer reads or skips adoption's record, adoption never sees a held one, and minting (proofroot.go) is back to main's code since it reads only adoption's directory. Status and alerts (refusalwatch.go) list both, held records keyed apart. A seam between the read and the removal makes the review's interleaving a deterministic regression.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
11 files changed,
+251
−52
approved_moves_copy_test.go+1 −1internal/host/heldstage.go+4 −3internal/host/movesstage_proof_test.go+15 −5internal/host/proofroot.go+1 −3internal/host/proofroot_test.go+3 −2internal/host/refusalwatch.go+14 −3internal/host/refusalwatch_test.go+34 −0internal/release/heldrefusal_test.go+103 −0internal/release/materializeheld.go+45 −20internal/release/materializeheld_proof_test.go+29 −14internal/release/refusal.go+2 −1
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go| index 5ae5a3fe1..249626389 100644 |
| --- a/approved_moves_copy_test.go |
| +++ b/approved_moves_copy_test.go |
| @@ -558,7 +558,7 @@ var movesScanFiles = []string{ |
| "internal/host/movesreport.go", |
| "internal/host/movesstage.go", |
| "internal/host/movesstore.go", |
| - "internal/host/proofroot.go", |
| + "internal/host/refusalwatch.go", |
| "internal/host/systemapply.go", |
| "internal/hostbox/writer.go", |
| "internal/hostclaim/move.go", |
diff --git a/internal/host/heldstage.go b/internal/host/heldstage.go| index 5bd1ca769..2db105344 100644 |
| --- a/internal/host/heldstage.go |
| +++ b/internal/host/heldstage.go |
| @@ -195,9 +195,10 @@ func heldStagePass(ctx context.Context, dataDir, optDir string, reg *registry.Re |
| } |
| // retireEndedHeldRefusals is root's half of a held refusal's lifecycle |
| -// (D3-held): every refusal record a move's held stage left (MoveOp set) |
| -// whose move the host claim no longer names as moving or landing into this |
| -// box is removed, so an aborted or abandoned move leaves nothing behind. |
| +// (D3-held): every refusal record a move's held stage left (the held |
| +// namespace, release.HeldRefusalBase) whose move the host claim no longer |
| +// names as moving or landing into this box is removed, so an aborted or |
| +// abandoned move leaves nothing behind. |
| // The agent retires the ones it can (movesWorker.retireHeldRefusals), but |
| // not a record root wrote. A claim that cannot be read keeps the record for |
| // the next pass. Adoption refusals are never touched, and no claim is read |
diff --git a/internal/host/movesstage_proof_test.go b/internal/host/movesstage_proof_test.go| index 9bc7186e0..df364096e 100644 |
| --- a/internal/host/movesstage_proof_test.go |
| +++ b/internal/host/movesstage_proof_test.go |
| @@ -10,6 +10,7 @@ import ( |
| "os" |
| "path/filepath" |
| "runtime" |
| + "sort" |
| "strings" |
| "testing" |
| "time" |
| @@ -148,32 +149,41 @@ func TestHeldReleaseReadyAppliesTheProofGate(t *testing.T) { |
| } |
| // heldRefusalFixture writes three refusal records under bake: a held one |
| -// for hsOp at hsHost, a held one for another move at another host, and an |
| -// adoption refusal (no MoveOp) at a third host. |
| +// for hsOp at hsHost, a held one for another move at another host (both in |
| +// the held namespace), and an adoption refusal (no MoveOp) at a third host. |
| func heldRefusalFixture(t *testing.T, bake string) { |
| t.Helper() |
| at := time.Date(2026, 10, 7, 0, 0, 0, 0, time.UTC) |
| for _, r := range []release.Refusal{ |
| {Host: hsHost, App: "titogo", Version: "v7", RefusedAt: at, Kind: release.RefusalNotProven, MoveOp: hsOp}, |
| {Host: "other.example.com", App: "titogo", Version: "v7", RefusedAt: at, Kind: release.RefusalNotProven, MoveOp: hsOther}, |
| - {Host: "adopted.example.com", App: "titogo", Version: "v7", RefusedAt: at, Kind: release.RefusalNotProven}, |
| } { |
| - if err := release.WriteRefusal(bake, r); err != nil { |
| + if err := release.WriteRefusal(release.HeldRefusalBase(bake), r); err != nil { |
| t.Fatal(err) |
| } |
| } |
| + if err := release.WriteRefusal(bake, release.Refusal{Host: "adopted.example.com", App: "titogo", Version: "v7", RefusedAt: at, Kind: release.RefusalNotProven}); err != nil { |
| + t.Fatal(err) |
| + } |
| } |
| +// refusalHosts lists the hosts with a refusal record in either namespace, |
| +// sorted. |
| func refusalHosts(t *testing.T, bake string) string { |
| t.Helper() |
| recs, err := release.ListRefusals(bake) |
| if err != nil { |
| t.Fatal(err) |
| } |
| + held, err := release.ListHeldRefusals(bake) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| var hosts []string |
| - for _, r := range recs { |
| + for _, r := range append(recs, held...) { |
| hosts = append(hosts, r.Host) |
| } |
| + sort.Strings(hosts) |
| return strings.Join(hosts, ",") |
| } |
diff --git a/internal/host/proofroot.go b/internal/host/proofroot.go| index 263bd08fd..5f2c1fb1e 100644 |
| --- a/internal/host/proofroot.go |
| +++ b/internal/host/proofroot.go |
| @@ -232,9 +232,7 @@ func (r proofRoot) mint(ctx context.Context, pol *release.ProofPolicy, rt regist |
| return |
| } |
| for _, ref := range refusals { |
| - // A move's held refusal (MoveOp set, D3-held) is about that move's |
| - // held stage, and is retired with it; it never stops a mint. |
| - if ref.MoveOp == "" && ref.SHA256 == b.SHA256 { |
| + if ref.SHA256 == b.SHA256 { |
| r.logf("proof: %s: not minting %s: this box refused it (%s)", rt.Host, b.Version, ref.Reason) |
| return |
| } |
diff --git a/internal/host/proofroot_test.go b/internal/host/proofroot_test.go| index d950c1f2d..1baa41cf6 100644 |
| --- a/internal/host/proofroot_test.go |
| +++ b/internal/host/proofroot_test.go |
| @@ -136,10 +136,11 @@ func TestProofRootDoesNotMintARefusedBuild(t *testing.T) { |
| // D3-held fix round 1: a move's held refusal (MoveOp set) is scoped to |
| // that move's held stage and never stops this box minting a proof for a |
| -// build it runs. |
| +// build it runs. Since final review finding 3 it lives in the held |
| +// namespace, which minting never reads. |
| func TestProofRootMintsPastAMovesHeldRefusal(t *testing.T) { |
| e := newProofRootEnv(t, provePolicy) |
| - if err := release.WriteRefusal(e.bake, release.Refusal{Host: "held.example.com", App: "console", Version: "v1", SHA256: e.sha, |
| + if err := release.WriteRefusal(release.HeldRefusalBase(e.bake), release.Refusal{Host: "held.example.com", App: "console", Version: "v1", SHA256: e.sha, |
| Kind: release.RefusalNotProven, MoveOp: "mv_abcdefghijklmnopqrstuvwxyz"}); err != nil { |
| t.Fatal(err) |
| } |
diff --git a/internal/host/refusalwatch.go b/internal/host/refusalwatch.go| index 30a77ac69..0b84adba8 100644 |
| --- a/internal/host/refusalwatch.go |
| +++ b/internal/host/refusalwatch.go |
| @@ -27,17 +27,28 @@ func (w *refusalWatch) tick(ctx context.Context, bakeDir string, sinks *alertSin |
| log.Printf("agent: refusals: %v", err) |
| return nil |
| } |
| + // A move's held-stage refusals (D3-held) live in their own namespace |
| + // (final review finding 3) and are surfaced as they were when they |
| + // shared adoption's: a row and an alert each, seen under their own key. |
| + held, err := release.ListHeldRefusals(bakeDir) |
| + if err != nil { |
| + log.Printf("agent: held refusals: %v", err) |
| + } |
| var rows []status.ReleaseRefusal |
| - for _, r := range recs { |
| + for i, r := range append(recs, held...) { |
| row := status.ReleaseRefusal{Host: r.Host, Version: r.Version, Reason: r.Reason, RefusedAt: r.RefusedAt, Kind: r.Kind} |
| if !r.Recovered { |
| row.RecoveryError = recoveryError(r) |
| } |
| rows = append(rows, row) |
| - if last, ok := w.seen[r.Host]; ok && !r.RefusedAt.After(last) { |
| + key := r.Host |
| + if i >= len(recs) { |
| + key = "held/" + r.Host |
| + } |
| + if last, ok := w.seen[key]; ok && !r.RefusedAt.After(last) { |
| continue |
| } |
| - w.seen[r.Host] = r.RefusedAt |
| + w.seen[key] = r.RefusedAt |
| if now.Sub(r.RefusedAt) <= refusalAlertWindow { |
| sinks.deliverRelease(ctx, r) |
| } |
diff --git a/internal/host/refusalwatch_test.go b/internal/host/refusalwatch_test.go| index 8aa25967a..899bfdb4a 100644 |
| --- a/internal/host/refusalwatch_test.go |
| +++ b/internal/host/refusalwatch_test.go |
| @@ -159,3 +159,37 @@ func TestRefusalWatchReportsANotProven(t *testing.T) { |
| t.Fatalf("rows = %+v", rows) |
| } |
| } |
| + |
| +// Final review finding 3: held refusals moved to their own namespace, and |
| +// status and alerts still surface them as they did when they shared |
| +// adoption's directory: one row each, beside an adoption refusal for the |
| +// same host, and one alert each, keyed apart from adoption's. |
| +func TestRefusalWatchReadsHeldRefusals(t *testing.T) { |
| + bake := t.TempDir() |
| + now := time.Date(2026, 10, 7, 9, 0, 0, 0, time.UTC) |
| + host := "held.example.com" |
| + if err := release.WriteRefusal(bake, release.Refusal{Host: host, App: "console", Version: "v1", |
| + RefusedAt: now.Add(-time.Minute), Reason: "exited: status=2"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := release.WriteRefusal(release.HeldRefusalBase(bake), release.Refusal{Host: host, App: "console", Version: "v2", |
| + RefusedAt: now.Add(-2 * time.Minute), Reason: "not proven: staged sha256 bb", Kind: release.RefusalNotProven, |
| + Recovered: true, MoveOp: "mv_abcdefghijklmnopqrstuvwxyz"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + var posts []string |
| + sinks := newAlertSinks("https://hook.example", func(_ string, b []byte) error { posts = append(posts, string(b)); return nil }, "", nil, &alertCounters{}) |
| + w := &refusalWatch{} |
| + rows := w.tick(context.Background(), bake, sinks, now) |
| + w.tick(context.Background(), bake, sinks, now.Add(time.Minute)) |
| + var versions []string |
| + for _, r := range rows { |
| + versions = append(versions, r.Host+"@"+r.Version) |
| + } |
| + if strings.Join(versions, ",") != host+"@v1,"+host+"@v2" { |
| + t.Fatalf("status rows %v, want the adoption refusal and the held one", versions) |
| + } |
| + if len(posts) != 2 { |
| + t.Fatalf("%d alerts, want one per record: %v", len(posts), posts) |
| + } |
| +} |
diff --git a/internal/release/heldrefusal_test.go b/internal/release/heldrefusal_test.go| new file mode 100644 |
| index 000000000..fcba71671 |
| --- /dev/null |
| +++ b/internal/release/heldrefusal_test.go |
| @@ -0,0 +1,103 @@ |
| +package release |
| + |
| +import ( |
| + "bytes" |
| + "os" |
| + "path/filepath" |
| + "testing" |
| + "time" |
| +) |
| + |
| +const ( |
| + nsHost = "held.example.com" |
| + nsOp = "mv_abcdefghijklmnopqrstuvwxyz" |
| +) |
| + |
| +func adoptionRefusalFor(host string) Refusal { |
| + return Refusal{Host: host, App: "console", Version: "v0", SHA256: "aa", Reason: "did not come up", |
| + PointerPromotedAt: time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC), RefusedAt: time.Date(2026, 10, 1, 0, 1, 0, 0, time.UTC)} |
| +} |
| + |
| +func heldRefusalFor(host string) Refusal { |
| + return Refusal{Host: host, App: "console", Version: "v1", SHA256: "bb", Reason: "not proven: staged sha256 bb", |
| + RefusedAt: time.Date(2026, 10, 7, 0, 0, 0, 0, time.UTC), Kind: RefusalNotProven, Recovered: true} |
| +} |
| + |
| +// Final review finding 3: held cleanup reads a record and then removes a |
| +// path. While held and adoption refusals shared refusals/<host>.json, an |
| +// adoption refusal written between the two (adoption does not take the |
| +// root adopt lock) was removed with it. Held refusals now live in their own |
| +// namespace, so the adoption refusal survives. The hook makes the |
| +// interleaving deterministic. |
| +func TestHeldCleanupLeavesAConcurrentAdoptionRefusal(t *testing.T) { |
| + bake := t.TempDir() |
| + if err := heldRefusalWriter(bake, nsOp)(heldRefusalFor(nsHost)); err != nil { |
| + t.Fatal(err) |
| + } |
| + prev := heldRetireBeforeClear |
| + t.Cleanup(func() { heldRetireBeforeClear = prev }) |
| + raced := false |
| + heldRetireBeforeClear = func(host string) { |
| + raced = true |
| + if err := WriteRefusal(bake, adoptionRefusalFor(host)); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + retired, err := RetireHeldRefusals(bake, func(Refusal) bool { return false }) |
| + if err != nil || len(retired) != 1 || !raced { |
| + t.Fatalf("retired %v, %v (raced %v)", retired, err, raced) |
| + } |
| + r, err := ReadRefusal(bake, nsHost) |
| + if err != nil || r == nil || r.MoveOp != "" || r.Reason != "did not come up" { |
| + t.Fatalf("the adoption refusal written during held cleanup: %+v, %v; want it kept", r, err) |
| + } |
| + if held, _ := ListHeldRefusals(bake); len(held) != 0 { |
| + t.Fatalf("held refusals left: %+v", held) |
| + } |
| +} |
| + |
| +// The held writer never touches adoption's record: with an adoption |
| +// refusal standing for the host, the held one is still recorded, beside |
| +// it, and the adoption file is byte-identical. Adoption's readers |
| +// (ReadRefusal, ListRefusals) never see a held record. |
| +func TestHeldRefusalsHaveTheirOwnNamespace(t *testing.T) { |
| + bake := t.TempDir() |
| + if err := WriteRefusal(bake, adoptionRefusalFor(nsHost)); err != nil { |
| + t.Fatal(err) |
| + } |
| + path := filepath.Join(RefusalDir(bake), nsHost+".json") |
| + before, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := heldRefusalWriter(bake, nsOp)(heldRefusalFor(nsHost)); err != nil { |
| + t.Fatalf("the held refusal was not recorded beside an adoption one: %v", err) |
| + } |
| + if after, _ := os.ReadFile(path); !bytes.Equal(before, after) { |
| + t.Fatalf("the adoption refusal changed:\n%s\nwant\n%s", after, before) |
| + } |
| + held, err := ListHeldRefusals(bake) |
| + if err != nil || len(held) != 1 || held[0].MoveOp != nsOp || held[0].Host != nsHost { |
| + t.Fatalf("held refusals %+v, %v", held, err) |
| + } |
| + // Adoption's readers see only adoption's record, on this host and on |
| + // one with only a held refusal. |
| + other := "only-held.example.com" |
| + if err := heldRefusalWriter(bake, nsOp)(heldRefusalFor(other)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if r, _ := ReadRefusal(bake, other); r != nil { |
| + t.Fatalf("adoption reads a held refusal: %+v", r) |
| + } |
| + recs, _ := ListRefusals(bake) |
| + if len(recs) != 1 || recs[0].MoveOp != "" { |
| + t.Fatalf("adoption's list holds %+v", recs) |
| + } |
| + // Retiring the held one leaves adoption's. |
| + if err := RetireHeldRefusal(bake, nsHost, nsOp); err != nil { |
| + t.Fatal(err) |
| + } |
| + if after, _ := os.ReadFile(path); !bytes.Equal(before, after) { |
| + t.Fatal("retiring the held refusal touched the adoption one") |
| + } |
| +} |
diff --git a/internal/release/materializeheld.go b/internal/release/materializeheld.go| index 219f13c99..2c24a2ed9 100644 |
| --- a/internal/release/materializeheld.go |
| +++ b/internal/release/materializeheld.go |
| @@ -150,8 +150,8 @@ func PublishHeld(ctx context.Context, s Store, optDir string, rt registry.Route, |
| // (or away, when there was none) and the stage is ErrHeldNotProven. |
| // |
| // A binding refusal is recorded scoped to the move (rt.HoldOp, Refusal's |
| -// MoveOp), never over an adoption refusal for the host; a publication that |
| -// succeeds retires its move's earlier refusal. |
| +// MoveOp) in the held namespace (HeldRefusalBase), never in adoption's; a |
| +// publication that succeeds retires its move's earlier refusal. |
| func publishHeld(optDir string, rt registry.Route, version string, proof *ProofGate, ptr *ChannelPointer, v proofVerdict, guard func(publish func() error) error) error { |
| if err := heldStageable(rt, version); err != nil { |
| return err |
| @@ -206,42 +206,67 @@ func restoreLive(optDir string, rt registry.Route, cur string) error { |
| return PointLive(optDir, rt.Host, RouteAccount(rt), cur, rt.App) |
| } |
| +// Held refusals (final review finding 3) live in their own namespace, |
| +// HeldRefusalBase(bakeDir)/refusals/<host>.json, the same record shape as |
| +// adoption's refusals/<host>.json beside it, written and read with the same |
| +// functions (WriteRefusal, ReadRefusal, ListRefusals, ClearRefusal) over the |
| +// other base. Adoption never reads that directory and the held stage never |
| +// writes, reads or removes adoption's, so neither can clobber or delete the |
| +// other's record whatever the interleaving: adoption runs in the agent and |
| +// in root, and nothing serialises the two across processes. Minting |
| +// (internal/host proofroot.go) reads only adoption's records; status and |
| +// alerts read both (internal/host refusalwatch.go). |
| +// |
| +// Within the namespace one host has one record, the latest held stage's. |
| +// Retirement reads it and removes it only when it names the ended move; a |
| +// newer move's record replacing it in between would be removed too. That |
| +// race is between two moves of one host into one box, and costs a status |
| +// row: a held refusal blocks nothing, and the next held stage of the live |
| +// move writes it again. |
| + |
| +// HeldRefusalBase is the bake directory the held namespace is rooted at: |
| +// its records are at RefusalDir(HeldRefusalBase(bakeDir)). |
| +func HeldRefusalBase(bakeDir string) string { return filepath.Join(bakeDir, "moves-held") } |
| + |
| // heldRefusalWriter is a held stage's refusal writer for bindRecording: the |
| -// record carries the move's op, and an adoption refusal standing for the |
| -// host (MoveOp "") is left as it is rather than overwritten. |
| +// record carries the move's op and goes to the held namespace. |
| func heldRefusalWriter(bakeDir, op string) func(Refusal) error { |
| return func(r Refusal) error { |
| - old, err := ReadRefusal(bakeDir, r.Host) |
| - if err != nil { |
| - return err |
| - } |
| - if old != nil && old.MoveOp == "" { |
| - return fmt.Errorf("an adoption refusal for %s stands, so move %s's is not recorded over it", r.Host, op) |
| - } |
| r.MoveOp = op |
| - return WriteRefusal(bakeDir, r) |
| + return WriteRefusal(HeldRefusalBase(bakeDir), r) |
| } |
| } |
| -// RetireHeldRefusal removes host's refusal record when it is move op's held |
| -// refusal, and leaves any other record (another move's, or adoption's). |
| +// RetireHeldRefusal removes host's held refusal when it is move op's, and |
| +// leaves another move's. Adoption's records are never read or touched. |
| func RetireHeldRefusal(bakeDir, host, op string) error { |
| if op == "" { |
| return nil |
| } |
| - r, err := ReadRefusal(bakeDir, host) |
| + base := HeldRefusalBase(bakeDir) |
| + r, err := ReadRefusal(base, host) |
| if err != nil || r == nil || r.MoveOp != op { |
| return err |
| } |
| - return ClearRefusal(bakeDir, host) |
| + heldRetireBeforeClear(host) |
| + return ClearRefusal(base, host) |
| +} |
| + |
| +// heldRetireBeforeClear runs between RetireHeldRefusal's read and its |
| +// removal. A test seam: the window a concurrent writer lands in. |
| +var heldRetireBeforeClear = func(host string) {} |
| + |
| +// ListHeldRefusals is every held refusal record, sorted by host. With no |
| +// held namespace yet it reads nothing. |
| +func ListHeldRefusals(bakeDir string) ([]Refusal, error) { |
| + return ListRefusals(HeldRefusalBase(bakeDir)) |
| } |
| -// RetireHeldRefusals removes every held refusal (MoveOp set) whose move |
| -// live says is over, and returns the hosts it removed. Adoption refusals |
| -// are never read past their MoveOp. With no refusal directory it reads |
| +// RetireHeldRefusals removes every held refusal whose move live says is |
| +// over, and returns the hosts it removed. With no held namespace it reads |
| // nothing. Errors from individual removals are joined; the rest go ahead. |
| func RetireHeldRefusals(bakeDir string, live func(Refusal) bool) ([]string, error) { |
| - recs, err := ListRefusals(bakeDir) |
| + recs, err := ListHeldRefusals(bakeDir) |
| if err != nil { |
| return nil, err |
| } |
diff --git a/internal/release/materializeheld_proof_test.go b/internal/release/materializeheld_proof_test.go| index 5b5ce10b5..fb88470bf 100644 |
| --- a/internal/release/materializeheld_proof_test.go |
| +++ b/internal/release/materializeheld_proof_test.go |
| @@ -125,8 +125,10 @@ func TestHeldStageProofCoveredUnprovenIsRefused(t *testing.T) { |
| t.Fatalf("v1 was staged for an unproven held stage: %v", err) |
| } |
| // A missing proof is not a refusal record, as in adoption. |
| - if r, _ := ReadRefusal(e.bake, heldTestHost); r != nil { |
| - t.Fatalf("a missing proof wrote a refusal: %+v", r) |
| + for _, base := range []string{e.bake, HeldRefusalBase(e.bake)} { |
| + if r, _ := ReadRefusal(base, heldTestHost); r != nil { |
| + t.Fatalf("a missing proof wrote a refusal: %+v", r) |
| + } |
| } |
| } |
| @@ -224,7 +226,7 @@ func TestHeldStageProofBindingRefusesOtherStagedBytes(t *testing.T) { |
| if !strings.Contains(out, "not pointed live") { |
| t.Fatalf("printed %q, want the binding refusal", out) |
| } |
| - r, _ := ReadRefusal(e.bake, heldTestHost) |
| + r, _ := ReadRefusal(HeldRefusalBase(e.bake), heldTestHost) |
| if r == nil || r.Kind != RefusalNotProven || !strings.Contains(r.Reason, etagOf(stale)) || r.RestoredVersion != "v0" { |
| t.Fatalf("refusal = %+v, want a not-proven record naming the staged hash, still on v0", r) |
| } |
| @@ -379,14 +381,19 @@ func TestHeldStageProofRefusalCarriesTheMove(t *testing.T) { |
| t.Fatal(err) |
| } |
| e.materialize(t, e.store) |
| - r, _ := ReadRefusal(e.bake, heldTestHost) |
| + r, _ := ReadRefusal(HeldRefusalBase(e.bake), heldTestHost) |
| if r == nil || r.MoveOp != heldTestOp || r.Kind != RefusalNotProven { |
| t.Fatalf("refusal = %+v, want a not-proven record naming op %s", r, heldTestOp) |
| } |
| + // Final review finding 3: in the held namespace, never adoption's. |
| + if r, _ := ReadRefusal(e.bake, heldTestHost); r != nil { |
| + t.Fatalf("the held refusal reached adoption's records: %+v", r) |
| + } |
| } |
| // Fix round 1, finding 2: an adoption refusal already standing for the host |
| -// is never overwritten by a held one. |
| +// is never overwritten by a held one. Since final review finding 3 the held |
| +// one is recorded beside it, in its own namespace. |
| func TestHeldStageProofLeavesAnAdoptionRefusal(t *testing.T) { |
| e := newHeldProofEnv(t) |
| e.require(t, "console") |
| @@ -408,6 +415,9 @@ func TestHeldStageProofLeavesAnAdoptionRefusal(t *testing.T) { |
| if !bytes.Equal(before, after) { |
| t.Fatalf("the adoption refusal changed:\n%s\nwant\n%s", after, before) |
| } |
| + if r, _ := ReadRefusal(HeldRefusalBase(e.bake), heldTestHost); r == nil || r.MoveOp != heldTestOp { |
| + t.Fatalf("the held refusal = %+v, want it recorded beside the adoption one", r) |
| + } |
| } |
| // Fix round 1, finding 2: a later held stage of the same move that |
| @@ -420,7 +430,7 @@ func TestHeldStageProofSuccessRetiresTheMovesRefusal(t *testing.T) { |
| t.Fatal(err) |
| } |
| e.materialize(t, e.store) |
| - if r, _ := ReadRefusal(e.bake, heldTestHost); r == nil { |
| + if r, _ := ReadRefusal(HeldRefusalBase(e.bake), heldTestHost); r == nil { |
| t.Fatal("no refusal to retire") |
| } |
| if err := StageArtifact(e.opt, "bab", "v1", "console", []byte("pinned build")); err != nil { |
| @@ -429,7 +439,7 @@ func TestHeldStageProofSuccessRetiresTheMovesRefusal(t *testing.T) { |
| if err, _, _ := e.materialize(t, e.store); err != nil || e.live() != "v1" { |
| t.Fatalf("retry: %v, live %q", err, e.live()) |
| } |
| - if r, _ := ReadRefusal(e.bake, heldTestHost); r != nil { |
| + if r, _ := ReadRefusal(HeldRefusalBase(e.bake), heldTestHost); r != nil { |
| t.Fatalf("the move's refusal outlived its successful retry: %+v", r) |
| } |
| } |
| @@ -440,21 +450,26 @@ func TestRetireHeldRefusals(t *testing.T) { |
| bake := t.TempDir() |
| at := time.Date(2026, 10, 7, 0, 0, 0, 0, time.UTC) |
| for _, r := range []Refusal{ |
| - {Host: "live.example.com", App: "a", Version: "v1", RefusedAt: at, Kind: RefusalNotProven, MoveOp: "mv_live"}, |
| - {Host: "over.example.com", App: "a", Version: "v1", RefusedAt: at, Kind: RefusalNotProven, MoveOp: "mv_over"}, |
| - {Host: "adopted.example.com", App: "a", Version: "v1", RefusedAt: at, Kind: RefusalNotProven}, |
| + {Host: "live.example.com", App: "a", Version: "v1", RefusedAt: at, Kind: RefusalNotProven}, |
| + {Host: "over.example.com", App: "a", Version: "v1", RefusedAt: at, Kind: RefusalNotProven}, |
| } { |
| - if err := WriteRefusal(bake, r); err != nil { |
| + if err := heldRefusalWriter(bake, "mv_"+strings.TrimSuffix(r.Host, ".example.com"))(r); err != nil { |
| t.Fatal(err) |
| } |
| } |
| + if err := WriteRefusal(bake, Refusal{Host: "adopted.example.com", App: "a", Version: "v1", RefusedAt: at, Kind: RefusalNotProven}); err != nil { |
| + t.Fatal(err) |
| + } |
| retired, err := RetireHeldRefusals(bake, func(r Refusal) bool { return r.MoveOp == "mv_live" }) |
| if err != nil || len(retired) != 1 || retired[0] != "over.example.com" { |
| t.Fatalf("retired %v, %v; want over.example.com", retired, err) |
| } |
| - left, _ := ListRefusals(bake) |
| - if len(left) != 2 || left[0].Host != "adopted.example.com" || left[1].Host != "live.example.com" { |
| - t.Fatalf("left %+v", left) |
| + left, _ := ListHeldRefusals(bake) |
| + if len(left) != 1 || left[0].Host != "live.example.com" { |
| + t.Fatalf("held left %+v", left) |
| + } |
| + if adopted, _ := ListRefusals(bake); len(adopted) != 1 || adopted[0].Host != "adopted.example.com" { |
| + t.Fatalf("adoption's records %+v", adopted) |
| } |
| // With no refusal directory it reads nothing and fails nothing. |
| if retired, err := RetireHeldRefusals(t.TempDir(), func(Refusal) bool { return false }); err != nil || len(retired) != 0 { |
diff --git a/internal/release/refusal.go b/internal/release/refusal.go| index 87c06b394..7d61a2afc 100644 |
| --- a/internal/release/refusal.go |
| +++ b/internal/release/refusal.go |
| @@ -70,7 +70,8 @@ type Refusal struct { |
| // ruling D3-held): the op of the move whose held stage was refused. It |
| // scopes the record to that move, which retires it (a later held stage |
| // of the move that succeeds, or the move ending), and it is never set on |
| - // an adoption refusal, so adoption's records are byte-identical. |
| + // an adoption refusal, so adoption's records are byte-identical. Held |
| + // records live under HeldRefusalBase, never in RefusalDir(bakeDir). |
| MoveOp string `json:"move_op,omitempty"` |
| } |