carlos / platform Public

release: merge fixes — D1 refusals survive authority loss; one authority selection per sibling

Two fixes to the merge resolution in 0d8bad7d, from review:

- Authority lost before the rollback start no longer returns early past
  writeGroupRefusals. The start (and every process mutation after it) is
  skipped, and each group host is still refused with Recovered=false and
  the authority error as RecoveryError, so the next pass, with fresh
  authority and an empty poisoned map, does not re-adopt the rejected
  build.
- D1's sibling rollback takes ONE release work per sibling for both its
  repoint and its stamp: the pass's original selection (now carried as
  passConfig.selected) where it has one, else a single Select. A selection
  that goes stale refuses the remaining write, which is logged, rather
  than a second Select capturing a successor and stamping it.

Tests: TestClaimAuthorityLostBeforeRollbackStartStillRefusesTheGroup and
TestClaimSiblingStampRefusedWhenItsSelectionGoesStale; both fail against
the previous resolution (checked by mutation).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude agent, operated by paul@keymail.dev c6dfb4a749c983e050acdaf1d6c977790b87f4ca parent 0d8bad7
3 files changed, +230 −15
  • internal/release/adopt.go +31 −9
  • internal/release/rollback.go +14 −6
  • internal/release/rollback_test.go +185 −0

CI passed — run details

diff --git a/internal/release/adopt.go b/internal/release/adopt.go
index 029141b58..6df5d88df 100644
--- a/internal/release/adopt.go
+++ b/internal/release/adopt.go
@@ -36,6 +36,12 @@ type PassOption func(*passConfig)
type passConfig struct {
hostWrites hostwrite.Config
selectedRestart, selectedStart hostwrite.Callback
+ // selected is this pass's release work per adoptable host, captured once
+ // in PassFilter. D1's group rollback reuses a sibling's entry, so the
+ // sibling's rollback writes are checked against the same selection its
+ // forward adoption ran under, never a fresh one that could be a
+ // successor's.
+ selected map[string]hostwrite.Work
// nag is where the canary-TTL nag (spec §5) is printed; nil (the
// default — every call site that does not pass NagExpiredCanaries)
// means the nag never fires at all, not even to a discarded writer.
@@ -353,6 +359,7 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551
}
selected[rt.Host] = work
}
+ cfg.selected = selected
// Resolve every distinct (Account, App, Channel) once. Verification
// failures kill the group (nil entry), not the pass.
@@ -1402,31 +1409,44 @@ func adoptHost(ctx context.Context, work hostwrite.Work, reg *registry.Registry,
}
// D1's group (rollback.go): siblings follow bound onto the
// restored build only when bound's own rollback happened, each
- // under its own release work.
+ // under ONE release work of its own: the pass's original
+ // selection where it has one (the one the sibling's forward
+ // adoption ran under), else a single Select made here. Never a
+ // Select per write: a second one could capture a successor's
+ // installation and stamp it.
var group []string
if isNotUp {
restored := ""
if rolledBack {
restored = cur
}
- group = rollbackGroup(reg, optDir, rt, ver, restored, func(h string, fn func() error) error {
- w, err := cfg.hostWrites.Select(ctx, h, "release")
- if err != nil {
- return err
+ group = rollbackGroup(reg, optDir, rt, ver, restored, func(h string) (func(func() error) error, error) {
+ w, ok := cfg.selected[h]
+ if !ok || w == nil {
+ var err error
+ if w, err = cfg.hostWrites.Select(ctx, h, "release"); err != nil {
+ return nil, err
+ }
}
- return withReleaseWork(ctx, w, id, fn)
+ return func(fn func() error) error { return withReleaseWork(ctx, w, id, fn) }, nil
})
if notUp.Journal != "" {
fmt.Printf(" %s: %s's last lines:\n%s\n", rt.Host, notUp.Unit, indent(notUp.Journal, " "))
}
}
started := false
- var serr error
+ var serr, authErr error
if start != nil && rolledBack && !daemonRestart {
+ // Authority lost here skips the start, and every process
+ // mutation after it, but NOT the refusal records below: the
+ // next pass has fresh authority and an empty poisoned map, and
+ // without a record it would adopt the rejected build again.
if guardErr := withReleaseWork(ctx, work, id, func() error { return nil }); guardErr != nil {
- fmt.Printf(" %s: release authority changed before rollback start: %v\n", rt.Host, guardErr)
- return
+ fmt.Printf(" %s: release authority changed before rollback start: %v — not started\n", rt.Host, guardErr)
+ authErr = guardErr
}
+ }
+ if start != nil && rolledBack && !daemonRestart && authErr == nil {
started = true
if cfg.hostWrites.Enabled {
if cfg.selectedStart == nil {
@@ -1453,6 +1473,8 @@ func adoptHost(ctx context.Context, work hostwrite.Work, reg *registry.Registry,
rec = recovery{err: "there is no previous version to roll back to"}
case !rolledBack:
rec = recovery{err: fmt.Sprintf("could not roll the symlink back to %s: %v", cur, rollbackErr)}
+ case authErr != nil:
+ rec = recovery{restored: cur, err: "release authority changed before rollback start: " + authErr.Error()}
case !started:
rec = recovery{restored: cur, err: "nothing on this pass restarts the unit on " + cur}
case serr != nil:
diff --git a/internal/release/rollback.go b/internal/release/rollback.go
index 502560fd3..ab95da19d 100644
--- a/internal/release/rollback.go
+++ b/internal/release/rollback.go
@@ -74,10 +74,13 @@ func processGroup(reg *registry.Registry, bound registry.Route) []registry.Route
// pass it was stamped rejected over restored, so its own PrevVersion now
// names restored; bound, never stamped, still holds the history both share.
//
-// with runs each sibling's host mutation under that sibling's own release
-// work (hostwrite), the way adoptHost guards bound's: a sibling whose
-// authority cannot be selected or has moved is left alone and still refused.
-func rollbackGroup(reg *registry.Registry, optDir string, bound registry.Route, rejected, restored string, with func(host string, fn func() error) error) []string {
+// workFor is asked ONCE per sibling it would write, and the guard it returns
+// runs both of that sibling's writes (hostwrite): the repoint and the stamp
+// are checked against one selection, so an installation that changes between
+// them refuses the stamp instead of handing it to the successor. A sibling
+// whose work cannot be had, or whose selection has gone stale, is left alone
+// and still refused.
+func rollbackGroup(reg *registry.Registry, optDir string, bound registry.Route, rejected, restored string, workFor func(host string) (func(func() error) error, error)) []string {
hosts := []string{bound.Host}
for _, g := range processGroup(reg, bound) {
hosts = append(hosts, g.Host)
@@ -85,11 +88,16 @@ func rollbackGroup(reg *registry.Registry, optDir string, bound registry.Route,
if err != nil || v != rejected || restored == "" {
continue
}
- if err := with(g.Host, func() error { return PointLive(optDir, g.Host, RouteAccount(g), restored, g.App) }); err != nil {
+ with, err := workFor(g.Host)
+ if err != nil {
+ fmt.Printf(" %s: not rolled back with its process: release work: %v\n", g.Host, err)
+ continue
+ }
+ if err := with(func() error { return PointLive(optDir, g.Host, RouteAccount(g), restored, g.App) }); err != nil {
fmt.Printf(" %s: could not roll the symlink back to %s with its process: %v\n", g.Host, restored, err)
continue
}
- if err := with(g.Host, func() error { return reg.SetVersions(g.Host, restored, bound.PrevVersion) }); err != nil {
+ if err := with(func() error { return reg.SetVersions(g.Host, restored, bound.PrevVersion) }); err != nil {
fmt.Printf(" %s: rolled back to %s but recording the version failed: %v\n", g.Host, restored, err)
}
}
diff --git a/internal/release/rollback_test.go b/internal/release/rollback_test.go
index f1bd8d8e1..5bbf68104 100644
--- a/internal/release/rollback_test.go
+++ b/internal/release/rollback_test.go
@@ -10,6 +10,7 @@ import (
"strings"
"testing"
+ "github.com/carlosframework/platform/internal/hostwrite"
"github.com/carlosframework/platform/internal/registry"
)
@@ -299,3 +300,187 @@ func TestRollbackGroupLeavesOtherUpstreamAlone(t *testing.T) {
t.Fatalf("another process's route was refused: %+v", r)
}
}
+
+// scriptedWork is one host's release selection under lifecycle host writes.
+// It counts With calls, refuses every effect once stale, and lets a test go
+// stale right after a chosen call's effect has landed.
+type scriptedWork struct {
+ name string
+ calls int
+ stale bool
+ after func(call int)
+}
+
+func (w *scriptedWork) With(_ context.Context, id hostwrite.Identity, fn func() error) error {
+ w.calls++
+ if id.Release == "" {
+ return errors.New("missing release identity")
+ }
+ if w.stale {
+ return errors.New(w.name + ": release selection changed")
+ }
+ err := fn()
+ if w.after != nil {
+ w.after(w.calls)
+ }
+ return err
+}
+
+// scriptedSelector hands out each host's works in order, one per Select, and
+// keeps the last for any later Select.
+type scriptedSelector struct {
+ works map[string][]*scriptedWork
+ selects map[string]int
+}
+
+func (s *scriptedSelector) Select(_ context.Context, host, _ string) (hostwrite.Work, error) {
+ list := s.works[host]
+ if len(list) == 0 {
+ return nil, errors.New("no work for " + host)
+ }
+ i := min(s.selects[host], len(list)-1)
+ s.selects[host]++
+ return list[i], nil
+}
+
+func claimWrites(sel *scriptedSelector) PassOption {
+ return WithHostWrites(hostwrite.Config{Enabled: true, Selector: sel})
+}
+
+// boundNotUp is the selected restart for notUpGroupFixture: the bound route's
+// unit does not come up, the sibling's restart is the shared process's no-op.
+func boundNotUp(restarts *int) hostwrite.Callback {
+ return func(_ context.Context, s hostwrite.Selection) error {
+ *restarts++
+ if s.Host == "platform.example.com" {
+ return &UnitNotUpError{Unit: "console.service", Reason: "exited: status=2"}
+ }
+ return nil
+ }
+}
+
+// Merge review P1: authority lost after the group rolled back, before the
+// rollback start. The start is skipped, but every group host is still refused
+// (not recovered, carrying the authority error), so the next pass, with fresh
+// authority and an empty poisoned map, does not adopt the rejected build.
+func TestClaimAuthorityLostBeforeRollbackStartStillRefusesTheGroup(t *testing.T) {
+ ctx, reg, s, pub, opt := notUpGroupFixture(t)
+ bake := t.TempDir()
+ bound := &scriptedWork{name: "platform"}
+ // Bound's calls: 1 forward repoint, 2 the pre-restart check, 3 the
+ // rollback repoint, 4 the pre-start check. Stale after 3 has landed.
+ bound.after = func(call int) {
+ if call == 3 {
+ bound.stale = true
+ }
+ }
+ sibling := &scriptedWork{name: "console"}
+ sel := &scriptedSelector{selects: map[string]int{}, works: map[string][]*scriptedWork{
+ "platform.example.com": {bound}, "console.example.com": {sibling}}}
+ restarts, starts := 0, 0
+ start := func(ctx context.Context, s hostwrite.Selection) error { starts++; return nil }
+ var out bytes.Buffer
+ withCapturedStdout(t, &out, func() {
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { t.Fatal("legacy restart"); return nil },
+ BakeDir(bake), claimWrites(sel), WithSelectedRestarts(boundNotUp(&restarts), start),
+ WithRollbackRestart(func(string) error { t.Fatal("legacy rollback restart"); return nil })); err != nil {
+ t.Fatalf("Pass: %v", err)
+ }
+ })
+ if !bound.stale {
+ t.Fatalf("the fixture never reached the rollback repoint:\n%s", out.String())
+ }
+ if starts != 0 {
+ t.Fatalf("rollback start ran %d times after authority changed", starts)
+ }
+ for _, h := range []string{"console.example.com", "platform.example.com"} {
+ if v, _ := LiveVersion(opt, h); v != "old" {
+ t.Fatalf("%s live = %q, want old (rolled back before authority changed)", h, v)
+ }
+ r, err := ReadRefusal(bake, h)
+ if err != nil || r == nil || r.Version != "new" || r.Recovered || r.RestoredVersion != "old" ||
+ !strings.Contains(r.RecoveryError, "release authority changed before rollback start") {
+ t.Fatalf("%s refusal = %+v, %v; want new refused, not recovered, with the authority error\n%s", h, r, err, out.String())
+ }
+ }
+
+ // The next pass: fresh, valid authority for both hosts, nothing poisoned.
+ sel2 := &scriptedSelector{selects: map[string]int{}, works: map[string][]*scriptedWork{
+ "platform.example.com": {{name: "platform2"}}, "console.example.com": {{name: "console2"}}}}
+ restarts = 0
+ out.Reset()
+ withCapturedStdout(t, &out, func() {
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { t.Fatal("legacy restart"); return nil },
+ BakeDir(bake), claimWrites(sel2), WithSelectedRestarts(boundNotUp(&restarts), start),
+ WithRollbackRestart(func(string) error { t.Fatal("legacy rollback restart"); return nil })); err != nil {
+ t.Fatalf("second Pass: %v", err)
+ }
+ })
+ if restarts != 0 || starts != 0 {
+ t.Fatalf("second pass restarted %d / started %d: it re-adopted the rejected build\n%s", restarts, starts, out.String())
+ }
+ for _, h := range []string{"console.example.com", "platform.example.com"} {
+ if v, _ := LiveVersion(opt, h); v != "old" {
+ t.Fatalf("%s live = %q after the second pass, want old", h, v)
+ }
+ if rt := mustRoute(t, reg, h); rt.Version == "new" {
+ t.Fatalf("%s stamped new by the second pass", h)
+ }
+ }
+}
+
+// Merge review P1: the sibling's installation changes between its rollback
+// repoint and its stamp. The stamp is refused under the sibling's one
+// selection (the pass's original), never re-selected and handed to the
+// successor.
+func TestClaimSiblingStampRefusedWhenItsSelectionGoesStale(t *testing.T) {
+ ctx, reg, s, pub, opt := notUpGroupFixture(t)
+ bake := t.TempDir()
+ bound := &scriptedWork{name: "platform"}
+ sibling := &scriptedWork{name: "console"}
+ // Sibling's calls: 1 forward repoint, 2 the pre-restart check, 3 the
+ // forward stamp, 4 the rollback repoint. Stale after 4 has landed.
+ sibling.after = func(call int) {
+ if call == 4 {
+ sibling.stale = true
+ }
+ }
+ successor := &scriptedWork{name: "console-successor"}
+ sel := &scriptedSelector{selects: map[string]int{}, works: map[string][]*scriptedWork{
+ "platform.example.com": {bound}, "console.example.com": {sibling, successor}}}
+ restarts, starts := 0, 0
+ start := func(ctx context.Context, s hostwrite.Selection) error {
+ return s.With(ctx, func() error { starts++; return nil })
+ }
+ var out bytes.Buffer
+ withCapturedStdout(t, &out, func() {
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { t.Fatal("legacy restart"); return nil },
+ BakeDir(bake), claimWrites(sel), WithSelectedRestarts(boundNotUp(&restarts), start),
+ WithRollbackRestart(func(string) error { t.Fatal("legacy rollback restart"); return nil })); err != nil {
+ t.Fatalf("Pass: %v", err)
+ }
+ })
+ if n := sel.selects["console.example.com"]; n != 1 || successor.calls != 0 {
+ t.Fatalf("sibling selected %d times, successor used %d times: want the pass's one selection only", n, successor.calls)
+ }
+ if !sibling.stale {
+ t.Fatalf("the fixture never reached the sibling's rollback repoint:\n%s", out.String())
+ }
+ if v, _ := LiveVersion(opt, "console.example.com"); v != "old" {
+ t.Fatalf("sibling live = %q, want old (the repoint landed before the change)", v)
+ }
+ if rt := mustRoute(t, reg, "console.example.com"); rt.Version != "new" {
+ t.Fatalf("sibling stamped %q over %q: the stamp should have been refused", rt.Version, rt.PrevVersion)
+ }
+ if !strings.Contains(out.String(), "console: release selection changed") {
+ t.Fatalf("the refused stamp was not logged:\n%s", out.String())
+ }
+ if starts != 1 {
+ t.Fatalf("bound's rollback start ran %d times, want 1", starts)
+ }
+ for _, h := range []string{"console.example.com", "platform.example.com"} {
+ if r, _ := ReadRefusal(bake, h); r == nil || r.Version != "new" {
+ t.Fatalf("%s refusal = %+v, want one naming new", h, r)
+ }
+ }
+}