carlos / platform Public

release: the rollback fires on every failure, and a daemon cycles once

Review of c847b0b: one critical, four important, one ruling.

C1. A converge verb that fails AFTER its daemon-reload leaves the NEW
version's unit text on disk, loaded, under the OLD binary — Converge
deliberately does not undo that and says the version-change caller owns it,
and the caller never did. Permanently wrong, not transiently: the next pass
finds no diff, snapshots those bytes as its own Prev, and every later
"restore" restores the wrong shape. adoptHost now calls caps.Restore on that
branch, gated on the signal Converge actually gives — Changed together with
the error. NOT unconditional, contrary to the review note: the refusals that
fire earliest fire before the drop-ins are read, so Prev is empty, and
Restore honours an empty snapshot by REMOVING what it finds. Unconditional
would delete the shape the running version serves under. Both directions are
pinned by tests, and both mutants are caught.

I1. Converge succeeded, PointLive failed: the symlink never moved, so the box
still serves the old version while the new version's ports sit bound in
public. The drop-ins now follow the symlink, which stayed put.

I2. The rollback summary no longer claims a restore that errored; it says so
instead. The line is the only thing most operators read.

I4. A daemon version change cycled the unit twice, and the FIRST cycle ran
the old binary under the new unit shape — a combination nobody shipped, on a
mail server. caps.Plan.DeferRestart (set only by adoptHost's version-change
converge, and only when a restart callback exists to honour it) suppresses
every verb that would bring a daemon up: step 8's start, step 9's restart,
socketSequence's trailing start. The service is down across the symlink swap
and comes up exactly once, on the new binary, through caps.Restart. The
socket unit's own verbs are never deferred — a listener the caller will not
re-bind has to be re-bound here. Task 8's drift pass leaves the flag false,
where restarting the live binary is the whole point.

Poison ruling: the converge-refusal branch no longer poisons the version. A
refusal bounces nothing, so there is no bad build to keep off the next host —
and a host-shaped refusal (a port another route already claims) would
otherwise freeze every other host on the box out of a version that is fine
for them. The restart-failure poison is untouched.

Also: the rollback's symlink-first ordering is now pinned (the recorder logs
the live version at EVERY call, not just each verb's first, so the restore's
own service start is observed at the old version); ensureStaged's
sibling-first read has a test of its own; and a first adoption whose restart
fails keeps its converged drop-ins rather than being handed an empty snapshot
that would strip the live version's shape.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 72be15c82ac524d1c9b8d65598d0ed762d356f85 parent c847b0b
3 files changed, +680 −26
  • internal/caps/converge.go +69 −7
  • internal/release/adopt.go +75 −17
  • internal/release/adopt_test.go +536 −2
diff --git a/internal/caps/converge.go b/internal/caps/converge.go
index fed039d8e..c185e0b55 100644
--- a/internal/caps/converge.go
+++ b/internal/caps/converge.go
@@ -57,6 +57,28 @@ type Plan struct {
Set *Set // NIL means the manifest did not resolve — LEAVE THE DROP-INS ALONE
Grants Grants
Hibernate bool
+
+ // DeferRestart says the CALLER will start this route's service itself,
+ // after this converge returns, so nothing here may bring it up.
+ //
+ // It exists for exactly one caller: release.adoptHost's version-change
+ // converge, which runs BEFORE the live symlink moves. Without it a daemon
+ // version change cycles the unit twice, and the first cycle runs the OLD
+ // binary under the NEW unit shape — a boot of a combination nobody
+ // shipped, on a mail server, for the width of a symlink swap. With it the
+ // service stays down across the swap and comes up exactly once, on the
+ // new binary, through Restart.
+ //
+ // It only ever affects a DAEMON plan's service verbs (step 8's start,
+ // step 9's restart, and socketSequence's trailing start). A non-daemon
+ // route's service is not started here with or without it, and the socket
+ // unit's own verbs are never deferred — a listener the caller is not
+ // going to re-bind must be re-bound here.
+ //
+ // The grant-drift pass (host.capabilityPass) leaves this FALSE and must:
+ // restarting the live binary to pick up a new unit shape is the entire
+ // point of that pass, and nothing there follows up with a Restart.
+ DeferRestart bool
}
// Snapshot is what was on disk before Converge wrote, enough to put it back.
@@ -139,6 +161,21 @@ func DropInPaths(etcDir, host string) (service, socket string) {
// version-change caller already owns that case (it rolls back the symlink and
// calls Restore).
//
+// That hand-off has a signal, and a caller that ignores it leaves the NEW
+// version's unit shape on disk under the OLD binary forever — the next pass
+// finds no diff, snapshots those bytes as Prev, and "restores" them:
+//
+// Changed == true together with a non-nil error means BYTES ARE STILL MOVED
+// and this function is not going to move them back.
+//
+// A version-change caller must answer that with Restore(o, res.Prev). Changed
+// == false with an error is a refusal (or a roll-back this function already
+// performed) and needs nothing: Prev is then either empty — because the
+// refusal came before the drop-ins were even read — or already on disk. Do
+// not call Restore unconditionally on a refusal; an empty Prev says "this
+// route had no generated drop-ins", and Restore honours that by REMOVING the
+// ones it finds.
+//
// Concurrency, stated rather than solved: two converges in one process (the
// hourly RunAdopt and the version-change converge inside Pass) are
// sequential. `carlos adopt` run by hand while carlos-adopt.timer fires is
@@ -387,6 +424,13 @@ type unitState struct {
// applyUnitState is steps 7-9: the socket restart ordering, the enable/start
// a daemon route's drift repair needs, and the service restart. Every verb
// here runs AFTER Converge's daemon-reload, never before it.
+//
+// p.DeferRestart suppresses every verb that would BRING A DAEMON UP — step
+// 8's start, step 9's restart, and socketSequence's trailing start — and
+// nothing else. The socket unit's own verbs, the enables, and the service
+// STOP still happen: a listener has to be re-bound here because the caller
+// will not do it, and leaving the service down across the caller's symlink
+// swap is the entire point. See Plan.DeferRestart.
func (o Ops) applyUnitState(p Plan, sockLive, svcChanged, sockChanged bool) unitState {
svcUnit, sockUnit := ServiceUnit(p.Host), SocketUnit(p.Host)
var st unitState
@@ -407,7 +451,7 @@ func (o Ops) applyUnitState(p Plan, sockLive, svcChanged, sockChanged bool) unit
// has to actually drop :25 — a rewrite nobody reloads into a running
// socket unit is a revoke that did nothing.
if sockChanged {
- err := o.socketSequence(p.Host, p.Set.Daemon, "restart")
+ err := o.socketSequence(p.Host, p.Set.Daemon, p.DeferRestart, "restart")
note(err)
if err != nil {
st.diverged = true
@@ -430,7 +474,7 @@ func (o Ops) applyUnitState(p Plan, sockLive, svcChanged, sockChanged bool) unit
// the service may be holding the port itself today (keymail binds
// :25 until its LISTEN_FDS change lands), and a bare socket start
// under it is the EADDRINUSE race again.
- note(o.socketSequence(p.Host, p.Set.Daemon, "start"))
+ note(o.socketSequence(p.Host, p.Set.Daemon, p.DeferRestart, "start"))
restartedService = true
}
}
@@ -438,10 +482,14 @@ func (o Ops) applyUnitState(p Plan, sockLive, svcChanged, sockChanged bool) unit
if !o.query("is-enabled", svcUnit) {
note(o.systemctl("enable", svcUnit))
}
- if !o.query("is-active", svcUnit) {
+ if !o.query("is-active", svcUnit) && !p.DeferRestart {
// enable+start, not try-restart: a daemon that crashed into a
// stopped state comes back rather than being left down by a
// try-restart no-op (spec §4d).
+ //
+ // Deferred, this is the start that would run the OLD binary: the
+ // service is inactive here precisely because step 7 just stopped
+ // it, and the caller's Restart is one symlink swap away.
note(o.systemctl("start", svcUnit))
restartedService = true
}
@@ -449,7 +497,10 @@ func (o Ops) applyUnitState(p Plan, sockLive, svcChanged, sockChanged bool) unit
// Step 9. A non-daemon route's restart is not this function's business:
// the caller's existing try-restart owns it, byte-identically to today.
- if svcChanged && p.Set.Daemon && !restartedService {
+ // Nor is a DEFERRED daemon's: restarting it here would cycle the old
+ // binary under the new unit text, and the caller restarts it once, after
+ // the symlink moves.
+ if svcChanged && p.Set.Daemon && !restartedService && !p.DeferRestart {
note(o.systemctl("restart", svcUnit))
restartedService = true
}
@@ -487,7 +538,15 @@ func (o Ops) applyUnitState(p Plan, sockLive, svcChanged, sockChanged bool) unit
//
// A route that is neither a daemon nor currently running is neither stopped
// nor started: a hibernating tenant's drop-ins are unit text, not a wake.
-func (o Ops) socketSequence(host string, daemon bool, verb string) error {
+//
+// deferStart drops the trailing service start for a DAEMON whose caller is
+// about to start it itself (Plan.DeferRestart) — the service is then stopped
+// here and stays stopped until the caller's Restart, which is what keeps the
+// old binary from ever running under the new unit shape. It is deliberately
+// ignored for a non-daemon route: there the caller's follow-up is a
+// try-restart, which is a no-op on a stopped unit, so skipping the start
+// would leave the app down.
+func (o Ops) socketSequence(host string, daemon, deferStart bool, verb string) error {
svcUnit, sockUnit := ServiceUnit(host), SocketUnit(host)
shouldRun := daemon || o.query("is-active", svcUnit)
@@ -500,7 +559,7 @@ func (o Ops) socketSequence(host string, daemon bool, verb string) error {
if err := o.systemctl(verb, sockUnit); err != nil {
errs = append(errs, err)
}
- if shouldRun {
+ if shouldRun && !(daemon && deferStart) {
if err := o.systemctl("start", svcUnit); err != nil {
errs = append(errs, err)
}
@@ -847,7 +906,10 @@ func Restore(o Ops, prev Snapshot) error {
o.logf(" %s: capability drop-ins rolled back to the previous version's", prev.Host)
}
if sockChanged {
- if err := o.socketSequence(prev.Host, prev.Daemon, "restart"); err != nil {
+ // Never deferred: a restore is the end of the line, and its caller
+ // has already rolled the symlink back to the binary this shape
+ // belongs to.
+ if err := o.socketSequence(prev.Host, prev.Daemon, false, "restart"); err != nil {
o.logf(" %s: rebinding the previous version's listeners: %v", prev.Host, err)
errs = append(errs, err)
}
diff --git a/internal/release/adopt.go b/internal/release/adopt.go
index 0abefec13..1eba14e32 100644
--- a/internal/release/adopt.go
+++ b/internal/release/adopt.go
@@ -413,9 +413,13 @@ func adoptHost(ctx context.Context, reg *registry.Registry, s Store, optDir stri
converged bool
)
if cfg.capsOps != nil && capabilityRoute(rt) {
- plan, err = capabilityPlan(rt, m)
+ // restart != nil is what makes the deferral safe: it says somebody is
+ // going to start this daemon after the symlink moves. With no restart
+ // callback at all (a caller with no proc manager) converge keeps
+ // today's behaviour and brings the unit up itself.
+ plan, err = capabilityPlan(rt, m, restart != nil)
+ var res caps.Result
if err == nil {
- var res caps.Result
res, err = caps.Converge(*cfg.capsOps, plan)
prev = res.Prev
}
@@ -427,12 +431,31 @@ func adoptHost(ctx context.Context, reg *registry.Registry, s Store, optDir stri
// declined to write. Converge itself has already written nothing
// and logged the operator's fix.
//
- // The poison is deliberately pass-wide even though some refusals
- // are host-shaped (a port another route's drop-in already claims,
- // a state-directory flip): the refusal this is most likely to be
- // is version-shaped — a capability block this binary does not
- // understand — and one skipped pass is the cheap direction.
- poisoned[ver] = true
+ // The version is deliberately NOT poisoned here (2026-08-03
+ // review). A refusal bounces nothing — no symlink moved, no unit
+ // cycled — so there is no bad build to keep off the next host,
+ // which is the only thing the poison exists for. And the refusal
+ // is often HOST-shaped: a port another route's drop-in already
+ // claims, a state-directory flip mid-migration. Poisoning on one
+ // of those would let a single misconfigured route freeze every
+ // other host on the box out of a version that is fine for them,
+ // pass after pass, until an operator noticed.
+ //
+ // Converge's own contract: bytes are still moved only when it
+ // reports Changed with the error — a failure AFTER its
+ // daemon-reload, which it deliberately does not undo because the
+ // version-change caller owns it. That is here. Without this the
+ // NEW version's unit shape stays on disk under the OLD binary
+ // permanently: the next pass finds no diff, snapshots those bytes
+ // as its Prev, and every later "restore" restores the wrong
+ // shape. Restore is NOT called for an ordinary refusal, where
+ // Prev is empty and would be read as "this route had no drop-ins"
+ // — that is a deletion, not a restore.
+ if res.Changed {
+ if err := caps.Restore(*cfg.capsOps, prev); err != nil {
+ fmt.Printf(" %s: restoring the previous version's capability drop-ins: %v\n", rt.Host, err)
+ }
+ }
fmt.Printf(" %s: capabilities for %s were refused: %v — not pointed live\n", rt.Host, ver, err)
return
}
@@ -440,7 +463,21 @@ func adoptHost(ctx context.Context, reg *registry.Registry, s Store, optDir stri
}
if err := PointLive(optDir, rt.Host, RouteAccount(rt), ver, rt.App); err != nil {
- fmt.Printf(" %s: repoint failed: %v\n", rt.Host, err)
+ // The symlink did not move, so the box is still serving cur — and the
+ // unit shape on disk is the new version's. Put the old one back, or a
+ // port the running binary never asked for stays publicly bound.
+ // Unlike the restart-failure branch below there is no cur == ""
+ // exception: nothing is live either way, so "restore what the symlink
+ // names" means removing what this pass just wrote.
+ rolled := ""
+ if converged {
+ if rerr := caps.Restore(*cfg.capsOps, prev); rerr != nil {
+ fmt.Printf(" %s: restoring the previous version's capability drop-ins: %v\n", rt.Host, rerr)
+ } else {
+ rolled = " — restored the capability drop-ins the symlink still names"
+ }
+ }
+ fmt.Printf(" %s: repoint failed: %v%s\n", rt.Host, err, rolled)
return
}
@@ -485,15 +522,29 @@ func adoptHost(ctx context.Context, reg *registry.Registry, s Store, optDir stri
if cur != "" {
_ = PointLive(optDir, rt.Host, RouteAccount(rt), cur, rt.App)
}
+ // The rule is one line long: restore the unit shape that matches
+ // whatever the symlink NOW names.
+ //
+ // With cur == "" there was no previous version and the symlink
+ // above could not be rolled back, so it still names ver — and the
+ // drop-ins already match it. Restoring here would hand Restore an
+ // empty snapshot, which it reads as "this route had no generated
+ // drop-ins" and honours by REMOVING the ones this pass wrote,
+ // leaving the live version running with no capability shape at
+ // all until the hourly grant-drift pass rebuilds it.
rolled := ""
- if converged {
+ if converged && cur != "" {
// Bytes are not a binding: Restore re-runs the socket
// sequence itself when the socket half moves, or the MX
// stays off :25 after a rollback.
if err := caps.Restore(*cfg.capsOps, prev); err != nil {
fmt.Printf(" %s: restoring the previous version's capability drop-ins: %v\n", rt.Host, err)
+ rolled = " but could not restore the previous version's capability drop-ins"
+ } else {
+ rolled = " and restored the previous version's capability drop-ins"
}
- rolled = " and restored the previous version's capability drop-ins"
+ } else if converged {
+ rolled = "; there is no previous version to roll back to, so its capability drop-ins stand as converged"
}
fmt.Printf(" %s: update to %s failed: %v — rolled the symlink back%s\n", rt.Host, ver, rerr, rolled)
return
@@ -537,7 +588,13 @@ func adoptHost(ctx context.Context, reg *registry.Registry, s Store, optDir stri
// grants: "no grants" is a real instruction that revokes :25 and drops a
// mail server's listener, and a column this binary cannot read is not
// evidence that an operator asked for that.
-func capabilityPlan(rt registry.Route, m *Manifest) (caps.Plan, error) {
+//
+// canRestart arms caps.Plan.DeferRestart: this converge runs BEFORE the
+// symlink moves, so a daemon it started would be the OLD binary under the NEW
+// unit shape, and the caller would then cycle it a second time. Deferring is
+// only honest when somebody is actually going to do the restart — with no
+// restart callback at all, converge keeps bringing the unit up itself.
+func capabilityPlan(rt registry.Route, m *Manifest, canRestart bool) (caps.Plan, error) {
if m == nil {
return caps.Plan{}, fmt.Errorf("the manifest for this version could not be read or fetched, so its unit shape is unknown")
}
@@ -550,11 +607,12 @@ func capabilityPlan(rt registry.Route, m *Manifest) (caps.Plan, error) {
set = &caps.Set{V: caps.Version}
}
return caps.Plan{
- Host: rt.Host,
- App: rt.App,
- Set: set,
- Grants: g,
- Hibernate: rt.Hibernate,
+ Host: rt.Host,
+ App: rt.App,
+ Set: set,
+ Grants: g,
+ Hibernate: rt.Hibernate,
+ DeferRestart: canRestart,
}, nil
}
diff --git a/internal/release/adopt_test.go b/internal/release/adopt_test.go
index e59e9d29b..ad5bd5702 100644
--- a/internal/release/adopt_test.go
+++ b/internal/release/adopt_test.go
@@ -1016,10 +1016,12 @@ func TestPassBackfillsTheLiveVersionsSiblingOnAnUnchangedPass(t *testing.T) {
type capsRecorder struct {
opt, host string
calls []string
+ live []string // live[i] is LiveVersion when calls[i] ran
liveAt map[string]string // verb+unit -> LiveVersion at its FIRST call
active map[string]bool
enabled map[string]bool
- fail map[string]error // "verb unit" -> error
+ fail map[string]error // "verb unit" -> error
+ onCall func(call string) // optional: fires after a call is recorded
}
func newCapsRecorder(opt, host string) *capsRecorder {
@@ -1034,11 +1036,15 @@ func newCapsRecorder(opt, host string) *capsRecorder {
func (r *capsRecorder) run(args ...string) error {
call := strings.Join(args, " ")
+ v, _ := LiveVersion(r.opt, r.host)
r.calls = append(r.calls, call)
+ r.live = append(r.live, v)
if _, seen := r.liveAt[call]; !seen {
- v, _ := LiveVersion(r.opt, r.host)
r.liveAt[call] = v
}
+ if r.onCall != nil {
+ r.onCall(call)
+ }
if err := r.fail[call]; err != nil {
return err
}
@@ -1077,6 +1083,38 @@ func (r *capsRecorder) called(call string) bool {
return false
}
+// lastLiveAt is where the live symlink pointed at the LAST occurrence of a
+// call. liveAt records only the first, which is blind to everything a
+// rollback does after the same verb has already run once — the ordering
+// inside the rollback (symlink first, drop-ins second) is exactly what needs
+// pinning, so it needs the last one.
+func (r *capsRecorder) lastLiveAt(call string) (string, bool) {
+ for i := len(r.calls) - 1; i >= 0; i-- {
+ if r.calls[i] == call {
+ return r.live[i], true
+ }
+ }
+ return "", false
+}
+
+// serviceCycles is every verb that STARTS the service unit — the count that
+// says whether a version change cycled the app once or twice.
+func (r *capsRecorder) serviceCycles(host string) (n int, lastLive string) {
+ for i, c := range r.calls {
+ if c == "start "+caps.ServiceUnit(host) || c == "restart "+caps.ServiceUnit(host) {
+ n++
+ lastLive = r.live[i]
+ }
+ }
+ return n, lastLive
+}
+
+// reset clears the call log between passes.
+func (r *capsRecorder) reset() {
+ r.calls, r.live = nil, nil
+ r.liveAt = map[string]string{}
+}
+
// capsTestOps wires every caps seam at a temp dir, so a Pass in this package
// touches no systemd, no /etc, no /run and no /var/lib. Logf is left nil on
// purpose: converge's own lines then go to os.Stdout with every other adopt
@@ -1202,6 +1240,13 @@ func TestAdoptConvergesCapabilitiesBeforePointLive(t *testing.T) {
if v, seen := rec.liveAt[svcRestart]; !seen || v != "v2" {
t.Fatalf("%q observed live=%q (seen=%v), want v2 — the daemon restarts onto the NEW binary", svcRestart, v, seen)
}
+ // And exactly ONCE. Converge runs before the symlink moves, so any start
+ // it performs itself boots the OLD binary under the NEW unit shape — a
+ // combination nobody shipped — and then the caller cycles the app a
+ // second time. Plan.DeferRestart is what collapses that to one cycle.
+ if n, last := rec.serviceCycles(host); n != 1 || last != "v2" {
+ t.Fatalf("service started/restarted %d time(s), last at live=%q; want exactly 1, at v2 (calls: %v)", n, last, rec.calls)
+ }
}
// TestRestartFailureRestoresBothTheSymlinkAndTheDropIns is the rollback that
@@ -1277,6 +1322,15 @@ func TestRestartFailureRestoresBothTheSymlinkAndTheDropIns(t *testing.T) {
if !rec.called("restart " + caps.SocketUnit(host)) {
t.Fatalf("calls = %v, want the socket unit re-bound during the restore", rec.calls)
}
+ // The ordering inside the rollback is load-bearing and easy to get
+ // backwards: the symlink goes back FIRST, so the service the restore
+ // starts comes up on the OLD binary. Restoring first would start v2's
+ // binary under v1's unit text — the failure this whole branch exists to
+ // undo. lastLiveAt, not liveAt: the same verb already ran during the
+ // forward converge.
+ if v, seen := rec.lastLiveAt("start " + caps.ServiceUnit(host)); !seen || v != "v1" {
+ t.Fatalf("the restore's service start observed live=%q (seen=%v), want v1 — the symlink must be rolled back BEFORE the drop-ins are", v, seen)
+ }
if v, _ := LiveVersion(opt, "b.example.com"); v != "v1" {
t.Fatalf("b live = %q, want v1 — the failed version must be poisoned for the rest of the pass", v)
}
@@ -1692,3 +1746,483 @@ func TestAdoptRemovesDropInsWhenTheNewVersionDeclaresNothing(t *testing.T) {
t.Fatalf("calls = %v, want the socket unit re-bound when its drop-in was removed", rec.calls)
}
}
+
+// TestDaemonVersionChangeStartsTheServiceExactlyOnce covers the other half of
+// the deferred restart: a version change whose SOCKET shape is unchanged, so
+// nothing in converge's socket ordering runs and step 9's service restart is
+// the only thing that would have cycled the unit. Deferred, the app is
+// started exactly once, after the symlink moves — one cycle, on the new
+// binary, instead of two with the first one running the old.
+func TestDaemonVersionChangeStartsTheServiceExactlyOnce(t *testing.T) {
+ s := NewDirStore(t.TempDir())
+ ctx := context.Background()
+ priv, pub := testKeys(t)
+ opt := t.TempDir()
+ reg := testRegistry(t)
+ host := "keymail.dev"
+
+ v1 := &caps.Set{V: 1, Command: "relay", Daemon: true, Ports: []caps.Port{{Port: 25}}}
+ v2 := &caps.Set{V: 1, Command: "relay-two", Daemon: true, Ports: []caps.Port{{Port: 25}}}
+ shipCapRelease(t, s, "bab", "keymail", "v1", v1, []byte("relay one"))
+ shipCapRelease(t, s, "bab", "keymail", "v2", v2, []byte("relay two"))
+ if err := Promote(ctx, s, priv, "bab", "keymail", "canary/x", "v1"); err != nil {
+ t.Fatalf("Promote v1: %v", err)
+ }
+ addCapRoute(t, reg, host, "keymail", "canary/x", registry.BackingUnit, "port:25", false, false)
+
+ rec := newCapsRecorder(opt, host)
+ o, _ := capsTestOps(t, rec)
+ restart := func(string) error { return nil }
+ if err := Pass(ctx, reg, s, pub, opt, restart, WithCapabilities(o)); err != nil {
+ t.Fatalf("Pass v1: %v", err)
+ }
+ if n, _ := rec.serviceCycles(host); n != 1 {
+ t.Fatalf("first adoption started the service %d time(s), want 1 (calls: %v)", n, rec.calls)
+ }
+ rec.reset()
+
+ if err := Promote(ctx, s, priv, "bab", "keymail", "canary/x", "v2"); err != nil {
+ t.Fatalf("Promote v2: %v", err)
+ }
+ if err := Pass(ctx, reg, s, pub, opt, restart, WithCapabilities(o)); err != nil {
+ t.Fatalf("Pass v2: %v", err)
+ }
+
+ if v, _ := LiveVersion(opt, host); v != "v2" {
+ t.Fatalf("live = %q, want v2", v)
+ }
+ if n, last := rec.serviceCycles(host); n != 1 || last != "v2" {
+ t.Fatalf("service started/restarted %d time(s), last at live=%q; want exactly 1, at v2 (calls: %v)", n, last, rec.calls)
+ }
+}
+
+// TestConvergeFailureAfterItsReloadRestoresTheDropIns is the failure the
+// converge contract hands to this caller by name: a systemd verb that fails
+// AFTER the daemon-reload leaves the new version's unit text on disk, loaded,
+// and deliberately un-rolled-back (Converge reports Changed with the error
+// and says the version-change caller owns it). Nothing points live, so those
+// bytes describe a version this box is not running — and they are permanent
+// if nobody undoes them: the next pass finds no diff, snapshots them as its
+// own Prev, and every later "restore" restores the wrong shape.
+//
+// The failing verb here is `enable` on the socket unit, which is post-reload
+// and NOT one of the diverging ones Converge rolls back itself.
+func TestConvergeFailureAfterItsReloadRestoresTheDropIns(t *testing.T) {
+ s := NewDirStore(t.TempDir())
+ ctx := context.Background()
+ priv, pub := testKeys(t)
+ opt := t.TempDir()
+ reg := testRegistry(t)
+ host := "keymail.dev"
+
+ // The box is on v1, which declares nothing, so there are no drop-ins yet.
+ shipRelease(t, s, "bab", "keymail", "v1", adoptPlatform, []byte("old"))
+ if err := StageArtifact(opt, "bab", "v1", "keymail", []byte("old")); err != nil {
+ t.Fatal(err)
+ }
+ addCapRoute(t, reg, host, "keymail", "canary/x", registry.BackingUnit, "port:25", false, false)
+ if err := PointLive(opt, host, "bab", "v1", "keymail"); err != nil {
+ t.Fatal(err)
+ }
+ shipCapRelease(t, s, "bab", "keymail", "v2", relaySet(), []byte("new"))
+ if err := Promote(ctx, s, priv, "bab", "keymail", "canary/x", "v2"); err != nil {
+ t.Fatalf("Promote v2: %v", err)
+ }
+
+ rec := newCapsRecorder(opt, host)
+ o, etc := capsTestOps(t, rec)
+ rec.fail["enable "+caps.SocketUnit(host)] = errors.New("unit masked")
+
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }, WithCapabilities(o)); err != nil {
+ t.Fatalf("Pass: %v", err)
+ }
+
+ if v, _ := LiveVersion(opt, host); v != "v1" {
+ t.Fatalf("live = %q, want v1", v)
+ }
+ svcPath, sockPath := caps.DropInPaths(etc, host)
+ for _, p := range []string{svcPath, sockPath} {
+ if b, err := os.ReadFile(p); err == nil {
+ t.Fatalf("%s survives a converge that failed after its reload (%q) — v2's unit shape must not be left on disk under v1's binary", p, b)
+ }
+ }
+}
+
+// TestConvergeRefusalLeavesAnEarlierVersionsDropInsAlone is the other side of
+// that coin, and the reason the restore above is conditional. An ORDINARY
+// refusal writes nothing — and the refusals that fire earliest fire before
+// the drop-ins are even read, so the snapshot is EMPTY. caps.Restore reads an
+// empty snapshot as "this route had no generated drop-ins" and honours it by
+// REMOVING the ones it finds, so restoring unconditionally on a refusal would
+// delete the shape the running version is serving under.
+//
+// The refusal here is spec §4e's daemon + hibernate contradiction, which
+// Converge checks before it reads anything.
+func TestConvergeRefusalLeavesAnEarlierVersionsDropInsAlone(t *testing.T) {
+ s := NewDirStore(t.TempDir())
+ ctx := context.Background()
+ priv, pub := testKeys(t)
+ opt := t.TempDir()
+ reg := testRegistry(t)
+ host := "sleepy.example.com"
+
+ // v1 is a hibernating tenant with a granted port and no daemon: a
+ // perfectly legal shape, and it converges.
+ v1 := &caps.Set{V: 1, Command: "web", Ports: []caps.Port{{Port: 25}}}
+ shipCapRelease(t, s, "bab", "keymail", "v1", v1, []byte("web one"))
+ if err := Promote(ctx, s, priv, "bab", "keymail", "canary/x", "v1"); err != nil {
+ t.Fatalf("Promote v1: %v", err)
+ }
+ addCapRoute(t, reg, host, "keymail", "canary/x", registry.BackingUnit, "port:25", true, false)
+
+ rec := newCapsRecorder(opt, host)
+ o, etc := capsTestOps(t, rec)
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }, WithCapabilities(o)); err != nil {
+ t.Fatalf("Pass v1: %v", err)
+ }
+ svcPath, sockPath := caps.DropInPaths(etc, host)
+ wantSvc, err := os.ReadFile(svcPath)
+ if err != nil {
+ t.Fatalf("read v1 service drop-in: %v", err)
+ }
+ wantSock, err := os.ReadFile(sockPath)
+ if err != nil {
+ t.Fatalf("read v1 socket drop-in: %v", err)
+ }
+
+ // v2 adds `daemon` to a hibernating route — refused before anything is
+ // read or written.
+ shipCapRelease(t, s, "bab", "keymail", "v2", relaySet(), []byte("web two"))
+ if err := Promote(ctx, s, priv, "bab", "keymail", "canary/x", "v2"); err != nil {
+ t.Fatalf("Promote v2: %v", err)
+ }
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }, WithCapabilities(o)); err != nil {
+ t.Fatalf("Pass v2: %v", err)
+ }
+
+ if v, _ := LiveVersion(opt, host); v != "v1" {
+ t.Fatalf("live = %q, want v1 — the refusal must not point live", v)
+ }
+ gotSvc, _ := os.ReadFile(svcPath)
+ gotSock, _ := os.ReadFile(sockPath)
+ if !bytes.Equal(gotSvc, wantSvc) || !bytes.Equal(gotSock, wantSock) {
+ t.Fatalf("drop-ins after a refusal = %q / %q, want v1's %q / %q left exactly as they were",
+ gotSvc, gotSock, wantSvc, wantSock)
+ }
+}
+
+// TestConvergeRefusalDoesNotPoisonTheVersionForOtherHosts is the 2026-08-03
+// ruling on the poison. A refusal bounces nothing — no symlink moved, no unit
+// cycled — so there is no bad build to keep off the next host, which is the
+// only thing the poison exists for. And the refusal is often HOST-shaped: the
+// one here is a port a third route's drop-in already holds on this box.
+// Poisoning on that would let one misconfigured route freeze every other host
+// out of a version that is fine for them, every pass, until an operator
+// noticed.
+//
+// The ordering is the point: a.example.com is refused FIRST (routes are swept
+// in host order), so a poison set there is what b.example.com would trip over.
+func TestConvergeRefusalDoesNotPoisonTheVersionForOtherHosts(t *testing.T) {
+ s := NewDirStore(t.TempDir())
+ ctx := context.Background()
+ priv, pub := testKeys(t)
+ opt := t.TempDir()
+ reg := testRegistry(t)
+
+ shipCapRelease(t, s, "bab", "keymail", "v1", relaySet(), []byte("relay"))
+ if err := Promote(ctx, s, priv, "bab", "keymail", "canary/x", "v1"); err != nil {
+ t.Fatalf("Promote: %v", err)
+ }
+ // a holds the grant for :25 and is refused, because something else on
+ // this box already claims it. b declares the same port with NO grant, so
+ // it claims nothing, materializes only its service half, and is fine.
+ addCapRoute(t, reg, "a.example.com", "keymail", "canary/x", registry.BackingUnit, "port:25", false, false)
+ addCapRoute(t, reg, "b.example.com", "keymail", "canary/x", registry.BackingUnit, "", false, false)
+
+ rec := newCapsRecorder(opt, "a.example.com")
+ o, etc := capsTestOps(t, rec)
+ // A drop-in belonging to a route this pass never sees, holding :25.
+ incumbent := filepath.Join(etc, caps.SocketUnit("zz.example.com")+".d")
+ if err := os.MkdirAll(incumbent, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(filepath.Join(incumbent, caps.DropInName),
+ []byte(caps.Marker+"\n[Socket]\nListenStream=25\n"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+
+ var out bytes.Buffer
+ withCapturedStdout(t, &out, func() {
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }, WithCapabilities(o)); err != nil {
+ t.Fatalf("Pass: %v", err)
+ }
+ })
+
+ if v, _ := LiveVersion(opt, "a.example.com"); v != "" {
+ t.Fatalf("a live = %q, want unset — its own refusal keeps it off the version", v)
+ }
+ if v, _ := LiveVersion(opt, "b.example.com"); v != "v1" {
+ t.Fatalf("b live = %q, want v1 — another host's refusal must not freeze this one out of the version", v)
+ }
+ line := out.String()
+ if strings.Contains(line, "failed on another host this pass") {
+ t.Fatalf("printed %q, want no poison — a refusal is not a bad build", line)
+ }
+ if !strings.Contains(line, "a.example.com: capabilities for v1 were refused") {
+ t.Fatalf("printed %q, want a's own refusal named", line)
+ }
+}
+
+// TestPointLiveFailureRestoresTheDropIns: converge succeeded, the symlink did
+// not move. The box is still serving the previous version, and the unit shape
+// on disk is the new one — a port the running binary never asked for, bound
+// in public. The drop-ins have to follow the symlink, which stayed put.
+//
+// PointLive is failed by parking a non-empty directory at the temp-link path
+// it renames through, which is as close to a disk-level swap failure as this
+// package can get without a fake filesystem.
+func TestPointLiveFailureRestoresTheDropIns(t *testing.T) {
+ s := NewDirStore(t.TempDir())
+ ctx := context.Background()
+ priv, pub := testKeys(t)
+ opt := t.TempDir()
+ reg := testRegistry(t)
+ host := "keymail.dev"
+
+ v1 := &caps.Set{V: 1, Command: "relay", Daemon: true, Ports: []caps.Port{{Port: 25}}}
+ v2 := &caps.Set{V: 1, Command: "relay", Daemon: true, Ports: []caps.Port{{Port: 25}, {Port: 465}}}
+ shipCapRelease(t, s, "bab", "keymail", "v1", v1, []byte("relay one"))
+ shipCapRelease(t, s, "bab", "keymail", "v2", v2, []byte("relay two"))
+ if err := Promote(ctx, s, priv, "bab", "keymail", "canary/x", "v1"); err != nil {
+ t.Fatalf("Promote v1: %v", err)
+ }
+ addCapRoute(t, reg, host, "keymail", "canary/x", registry.BackingUnit, "port:25,port:465", false, false)
+
+ rec := newCapsRecorder(opt, host)
+ o, etc := capsTestOps(t, rec)
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }, WithCapabilities(o)); err != nil {
+ t.Fatalf("Pass v1: %v", err)
+ }
+ svcPath, sockPath := caps.DropInPaths(etc, host)
+ wantSvc, _ := os.ReadFile(svcPath)
+ wantSock, _ := os.ReadFile(sockPath)
+ if !strings.Contains(string(wantSock), "ListenStream=25") || strings.Contains(string(wantSock), "465") {
+ t.Fatalf("v1 socket drop-in = %q, want :25 only", wantSock)
+ }
+
+ // Block the symlink swap.
+ blocker := LiveLink(opt, host) + ".tmp"
+ if err := os.MkdirAll(filepath.Join(blocker, "occupied"), 0o755); err != nil {
+ t.Fatalf("park a blocker at %s: %v", blocker, err)
+ }
+ if err := Promote(ctx, s, priv, "bab", "keymail", "canary/x", "v2"); err != nil {
+ t.Fatalf("Promote v2: %v", err)
+ }
+
+ var out bytes.Buffer
+ withCapturedStdout(t, &out, func() {
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }, WithCapabilities(o)); err != nil {
+ t.Fatalf("Pass v2: %v", err)
+ }
+ })
+
+ if v, _ := LiveVersion(opt, host); v != "v1" {
+ t.Fatalf("live = %q, want v1 — the repoint was supposed to fail", v)
+ }
+ gotSvc, _ := os.ReadFile(svcPath)
+ gotSock, _ := os.ReadFile(sockPath)
+ if !bytes.Equal(gotSvc, wantSvc) || !bytes.Equal(gotSock, wantSock) {
+ t.Fatalf("drop-ins = %q / %q, want v1's %q / %q — :465 must not stay bound under a binary that never asked for it",
+ gotSvc, gotSock, wantSvc, wantSock)
+ }
+ if line := out.String(); !strings.Contains(line, "repoint failed") || !strings.Contains(line, "capability drop-ins") {
+ t.Fatalf("printed %q, want a line naming both the failed repoint and the restored drop-ins", line)
+ }
+}
+
+// TestArmedPassConvergesFromTheStagedSiblingWhenTheBucketCannotAnswer pins
+// the read ensureStaged does on its already-staged path. The binary is on
+// disk with its sibling beside it and the bucket object is gone: the sibling
+// IS the manifest in hand there, and dropping that read would send this
+// straight to a bucket fetch that cannot answer, then to a refusal of a
+// version whose capabilities are sitting on disk, readable, all along.
+func TestArmedPassConvergesFromTheStagedSiblingWhenTheBucketCannotAnswer(t *testing.T) {
+ dir := t.TempDir()
+ s := NewDirStore(dir)
+ ctx := context.Background()
+ priv, pub := testKeys(t)
+ opt := t.TempDir()
+ reg := testRegistry(t)
+ host := "keymail.dev"
+
+ shipRelease(t, s, "bab", "keymail", "v1", adoptPlatform, []byte("old"))
+ if err := StageArtifact(opt, "bab", "v1", "keymail", []byte("old")); err != nil {
+ t.Fatal(err)
+ }
+ addCapRoute(t, reg, host, "keymail", "canary/x", registry.BackingUnit, "port:25", false, false)
+ if err := PointLive(opt, host, "bab", "v1", "keymail"); err != nil {
+ t.Fatal(err)
+ }
+
+ set := relaySet()
+ shipCapRelease(t, s, "bab", "keymail", "v2", set, []byte("new"))
+ if err := Promote(ctx, s, priv, "bab", "keymail", "canary/x", "v2"); err != nil {
+ t.Fatalf("Promote v2: %v", err)
+ }
+ // v2 is staged the way a box that adopted it once already would have it:
+ // binary AND sibling on disk. Then the bucket object goes away.
+ if err := StageArtifact(opt, "bab", "v2", "keymail", []byte("new")); err != nil {
+ t.Fatal(err)
+ }
+ raw, _, err := s.Get(ctx, ManifestKey("bab", "keymail", "v2"))
+ if err != nil {
+ t.Fatalf("read v2 manifest: %v", err)
+ }
+ if err := StageManifest(opt, "bab", "v2", "keymail", raw); err != nil {
+ t.Fatalf("StageManifest: %v", err)
+ }
+ if err := os.Remove(filepath.Join(dir, filepath.FromSlash(ManifestKey("bab", "keymail", "v2")))); err != nil {
+ t.Fatalf("remove v2 manifest: %v", err)
+ }
+
+ rec := newCapsRecorder(opt, host)
+ o, etc := capsTestOps(t, rec)
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }, WithCapabilities(o)); err != nil {
+ t.Fatalf("Pass: %v", err)
+ }
+
+ if v, _ := LiveVersion(opt, host); v != "v2" {
+ t.Fatalf("live = %q, want v2 — the sibling answered, so nothing needed the bucket", v)
+ }
+ g := caps.Grants{Ports: map[int]bool{25: true}}
+ svcPath, _ := caps.DropInPaths(etc, host)
+ wantSvc, err := caps.RenderService("keymail", set, g)
+ if err != nil {
+ t.Fatalf("RenderService: %v", err)
+ }
+ gotSvc, err := os.ReadFile(svcPath)
+ if err != nil || !bytes.Equal(gotSvc, wantSvc) {
+ t.Fatalf("service drop-in = %q (%v), want the sibling's capabilities materialized: %q", gotSvc, err, wantSvc)
+ }
+}
+
+// TestRestartFailureSaysSoWhenTheRestoreAlsoFails: the summary line is the
+// only thing most operators read, and it must not claim a restore that did
+// not happen. A rollback that could not put the drop-ins back is the worst
+// state this path can end in — the symlink says v1, the unit text says v2 —
+// so it is exactly the case that must not be reported as a tidy rollback.
+//
+// The restore is failed by making the socket drop-in's directory unwritable
+// at the moment the daemon restart fails, i.e. after the forward converge has
+// written it and before the rollback tries to.
+func TestRestartFailureSaysSoWhenTheRestoreAlsoFails(t *testing.T) {
+ if os.Geteuid() == 0 {
+ t.Skip("running as root: directory permissions do not refuse a write")
+ }
+ s := NewDirStore(t.TempDir())
+ ctx := context.Background()
+ priv, pub := testKeys(t)
+ opt := t.TempDir()
+ reg := testRegistry(t)
+ host := "keymail.dev"
+
+ v1 := &caps.Set{V: 1, Command: "relay", Daemon: true, Ports: []caps.Port{{Port: 25}}}
+ v2 := &caps.Set{V: 1, Command: "relay", Daemon: true, Ports: []caps.Port{{Port: 25}, {Port: 465}}}
+ shipCapRelease(t, s, "bab", "keymail", "v1", v1, []byte("relay one"))
+ shipCapRelease(t, s, "bab", "keymail", "v2", v2, []byte("relay two"))
+ if err := Promote(ctx, s, priv, "bab", "keymail", "canary/x", "v1"); err != nil {
+ t.Fatalf("Promote v1: %v", err)
+ }
+ addCapRoute(t, reg, host, "keymail", "canary/x", registry.BackingUnit, "port:25,port:465", false, false)
+
+ rec := newCapsRecorder(opt, host)
+ o, etc := capsTestOps(t, rec)
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }, WithCapabilities(o)); err != nil {
+ t.Fatalf("Pass v1: %v", err)
+ }
+
+ _, sockPath := caps.DropInPaths(etc, host)
+ sockDir := filepath.Dir(sockPath)
+ t.Cleanup(func() { os.Chmod(sockDir, 0o755) })
+ rec.fail["restart "+caps.ServiceUnit(host)] = errors.New("relay failed to start")
+ rec.onCall = func(call string) {
+ if call == "restart "+caps.ServiceUnit(host) {
+ if err := os.Chmod(sockDir, 0o555); err != nil {
+ t.Errorf("chmod %s: %v", sockDir, err)
+ }
+ }
+ }
+
+ if err := Promote(ctx, s, priv, "bab", "keymail", "canary/x", "v2"); err != nil {
+ t.Fatalf("Promote v2: %v", err)
+ }
+ var out bytes.Buffer
+ withCapturedStdout(t, &out, func() {
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }, WithCapabilities(o)); err != nil {
+ t.Fatalf("Pass v2: %v", err)
+ }
+ })
+
+ line := out.String()
+ if !strings.Contains(line, "could not restore") {
+ t.Fatalf("printed %q, want the summary to say the restore failed", line)
+ }
+ if strings.Contains(line, "and restored the previous version's capability drop-ins") {
+ t.Fatalf("printed %q, want no claim of a restore that did not happen", line)
+ }
+ if v, _ := LiveVersion(opt, host); v != "v1" {
+ t.Fatalf("live = %q, want v1 — the symlink half of the rollback still has to happen", v)
+ }
+}
+
+// TestFirstAdoptionRestartFailureLeavesTheConvergedDropIns: with no previous
+// version there is no symlink to roll back — the box stays live on the
+// version whose restart just failed. Handing caps.Restore the empty snapshot
+// that describes "this route had no drop-ins" would then REMOVE the shape the
+// live version needs, leaving it unconfigured until the hourly grant-drift
+// pass rebuilds it. The honest state is the one the symlink names.
+func TestFirstAdoptionRestartFailureLeavesTheConvergedDropIns(t *testing.T) {
+ s := NewDirStore(t.TempDir())
+ ctx := context.Background()
+ priv, pub := testKeys(t)
+ opt := t.TempDir()
+ reg := testRegistry(t)
+ host := "keymail.dev"
+
+ set := relaySet()
+ shipCapRelease(t, s, "bab", "keymail", "v1", set, []byte("relay"))
+ if err := Promote(ctx, s, priv, "bab", "keymail", "canary/x", "v1"); err != nil {
+ t.Fatalf("Promote: %v", err)
+ }
+ addCapRoute(t, reg, host, "keymail", "canary/x", registry.BackingUnit, "port:25", false, false)
+
+ rec := newCapsRecorder(opt, host)
+ o, etc := capsTestOps(t, rec)
+ rec.fail["restart "+caps.ServiceUnit(host)] = errors.New("relay failed to start")
+
+ var out bytes.Buffer
+ withCapturedStdout(t, &out, func() {
+ if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }, WithCapabilities(o)); err != nil {
+ t.Fatalf("Pass: %v", err)
+ }
+ })
+
+ if v, _ := LiveVersion(opt, host); v != "v1" {
+ t.Fatalf("live = %q, want v1 — there is no previous version to roll back to", v)
+ }
+ g := caps.Grants{Ports: map[int]bool{25: true}}
+ svcPath, sockPath := caps.DropInPaths(etc, host)
+ wantSvc, _ := caps.RenderService("keymail", set, g)
+ wantSock, _ := caps.RenderSocket(set, g)
+ gotSvc, err := os.ReadFile(svcPath)
+ if err != nil || !bytes.Equal(gotSvc, wantSvc) {
+ t.Fatalf("service drop-in = %q (%v), want the converged shape the live version needs: %q", gotSvc, err, wantSvc)
+ }
+ gotSock, err := os.ReadFile(sockPath)
+ if err != nil || !bytes.Equal(gotSock, wantSock) {
+ t.Fatalf("socket drop-in = %q (%v), want %q", gotSock, err, wantSock)
+ }
+ if line := out.String(); !strings.Contains(line, "no previous version to roll back to") {
+ t.Fatalf("printed %q, want the line to say why nothing was rolled back", line)
+ }
+}