amadan

carlos / platform Public

zdd plan 3 final fix: every claims-backed swap is refused before its raise or stop (F9)

claimsSwapRefusal guards both modes whenever the selected work is not
hostwrite.Legacy: errClaimsNoGenerations without Generations and Pins,
and the closed isolated launch (claimAdmission of
ErrClaimIsolationUnavailable, NoOwnership) with both. Nothing is admitted
inside the work, raised, stopped or spawned. The booleans stay for plan
5 and never open the guard alone. The overlap's one exception is the
claimSpawn test seam; TestClaimSpawnIsATestSeam fails if any production
file assigns it. A hold has no exception.

Tests: every runtime combination (nil, neither, each, both) in both
modes refuses with no side effect; the closed-launch tests now expect no
raise and no With call; TestSwapClaimsOn's rows use the seam so each
reaches the refusal it names.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude b19a0f016df770f0e42366eef676f0adce438fb1 parent 6fcc08e
6 files changed, +165 −40
  • internal/activator/activator.go +3 −2
  • internal/activator/claimgen.go +26 −0
  • internal/activator/claimgen_test.go +103 −11
  • internal/activator/swap.go +12 −10
  • internal/activator/swap_overlap_test.go +13 −12
  • internal/activator/swaphold.go +8 −5
diff --git a/internal/activator/activator.go b/internal/activator/activator.go
index 96bee789a..8e2a7a49c 100644
--- a/internal/activator/activator.go
+++ b/internal/activator/activator.go
@@ -923,8 +923,9 @@ type Service struct {
genHealth func(ctx context.Context, g Gen) error
// claimSpawn, when set, replaces a claims-on successor's spawn. It is a
// test seam: a claims-on spawn needs a delegated cgroup tree and an open
- // isolated launch. Nil in production, where every claims-on launch is
- // closed on this base and spawnSuccessor refuses it.
+ // isolated launch. Nil in production (no production file assigns it,
+ // TestClaimSpawnIsATestSeam), where every claims-on launch is closed on
+ // this base and claimsSwapRefusal refuses the swap before its raise.
claimSpawn func(ctx context.Context, e *actEntry, rt registry.Route, work hostwrite.Work, g Gen) (Gen, Proc, error)
}
diff --git a/internal/activator/claimgen.go b/internal/activator/claimgen.go
index fe8fe390f..60c329c0d 100644
--- a/internal/activator/claimgen.go
+++ b/internal/activator/claimgen.go
@@ -11,6 +11,32 @@ import (
// returned before anything is raised, stopped or spawned.
var errClaimsNoGenerations = errors.New("swap: claims are on and the claim runtime cannot start a generation of its own")
+// claimsSwapRefusal is the guard every claims-backed swap (work that is not
+// hostwrite.Legacy) meets before its side effects: before the admission
+// runs inside the work, before an overlap raises the parent, before a hold
+// stops its incumbent (final review F9, index P2-9). On this base it always
+// refuses. A runtime without Generations and Pins gets
+// errClaimsNoGenerations, as before. One with both still meets the closed
+// isolated launch (ErrClaimIsolationUnavailable, as the admission's
+// refusal: NoOwnership, so the attempt is abandoned, never failed). The
+// booleans stay for plan 5; they alone never open this guard.
+//
+// overlap names the one exception, and it is a test seam: claimSpawn, the
+// stand-in for a working isolated launch, lets an overlap through to its
+// claims-on spawn. Only this package's tests set it (no production file
+// assigns it; TestClaimSpawnIsATestSeam checks that), so production always
+// refuses here. A hold has no stand-in: its pinned wake would run the
+// closed claims launch itself, so it is refused even then.
+func (a *Service) claimsSwapRefusal(overlap bool) error {
+ if r := a.cfg.Claims; r == nil || !r.Generations || !r.Pins {
+ return errClaimsNoGenerations
+ }
+ if overlap && a.claimSpawn != nil {
+ return nil
+ }
+ return claimAdmission(ErrClaimIsolationUnavailable, false)
+}
+
// claimGenAt is generation n's handle for a claims-on spawn: the generation
// sockets where the row has them, and the canonical socket (with a real
// number) where it does not.
diff --git a/internal/activator/claimgen_test.go b/internal/activator/claimgen_test.go
index 0ab304ca0..5e49ec4bf 100644
--- a/internal/activator/claimgen_test.go
+++ b/internal/activator/claimgen_test.go
@@ -4,6 +4,10 @@ import (
"context"
"errors"
"fmt"
+ "go/ast"
+ "go/parser"
+ "go/token"
+ "io/fs"
"os/exec"
"strings"
"testing"
@@ -145,10 +149,10 @@ func TestAClaimsRefusalAtTheExecIsNoFailure(t *testing.T) {
t.Fatalf("%v at the exec: Swap = %+v, want abandoned", refusal, res)
}
}
- // No seam: the successor's spawn is the closed launch itself. Its
- // refusal comes before anything runs, so it is the admission's
- // (claimAdmission), and the attempt is abandoned. The runtime is never
- // read or prepared: there is no launch to prepare for.
+ // No seam: the closed launch refuses the swap before anything runs
+ // (claimsSwapRefusal), as the admission's refusal (claimAdmission), and
+ // the attempt is abandoned. The runtime is never read or prepared:
+ // there is no launch to prepare for.
f := newSwapFixture(t)
f.a.cfg.HostWrites = hostwrite.Config{Enabled: true, Selector: &swapSelector{work: &swapWork{}}}
f.a.cfg.Claims = &ClaimRuntime{Pins: true, Generations: true,
@@ -164,11 +168,11 @@ func TestAClaimsRefusalAtTheExecIsNoFailure(t *testing.T) {
}
}
-// Index P2-9: a claims-on overlap whose runtime has Generations and Pins,
-// with no test seam standing in for the launch, passes every check, raises
-// the parent, and is refused at the spawn by the closed launch. It is
-// abandoned with nothing spawned, the raise undone, the incumbent still
-// serving and no failure recorded (SwapFailed would halt the cohort).
+// Index P2-9, final review F9: a claims-on overlap whose runtime has
+// Generations and Pins, with no test seam standing in for the launch, is
+// refused by the closed launch before its admission and its raise. It is
+// abandoned with nothing raised or spawned, the incumbent still serving and
+// no failure recorded (SwapFailed would halt the cohort).
func TestAClaimsOnOverlapOnAClosedLaunchIsAbandoned(t *testing.T) {
f := newSwapFixture(t)
w := &swapWork{}
@@ -182,8 +186,11 @@ func TestAClaimsOnOverlapOnAClosedLaunchIsAbandoned(t *testing.T) {
t.Fatalf("Reason %q: the closed launch is the admission's refusal", res.Reason)
}
gens, raises, restores := f.b.calls()
- if gens != 0 || raises != 1 || restores != 1 {
- t.Fatalf("StartStaged %d, raises %d, restores %d: want nothing spawned and the parent restored", gens, raises, restores)
+ if gens != 0 || raises != 0 || restores != 0 {
+ t.Fatalf("StartStaged %d, raises %d, restores %d: want nothing raised or spawned", gens, raises, restores)
+ }
+ if n := w.withCalls(); n != 0 {
+ t.Fatalf("With calls = %d: the refusal comes before the admission", n)
}
if f.b.lastProc().isStopped() {
t.Fatal("the incumbent was stopped")
@@ -354,3 +361,88 @@ func TestARetiringClaimedGenerationDrainsBeforeItsKill(t *testing.T) {
}
}
}
+
+// Final review F9: on this base every claims-backed swap (work that is not
+// hostwrite.Legacy) is refused before its side effects, whatever the
+// runtime's Generations and Pins say: no admission inside the work, no
+// parent raise, no stop of the incumbent, no spawn. The guard opens only
+// with a working isolated launch, which this base does not have.
+func TestEveryClaimsBackedSwapIsRefusedBeforeItsSideEffects(t *testing.T) {
+ runtimes := []struct {
+ name string
+ r *ClaimRuntime
+ }{
+ {"no runtime", nil},
+ {"neither", &ClaimRuntime{}},
+ {"generations only", &ClaimRuntime{Generations: true}},
+ {"pins only", &ClaimRuntime{Pins: true}},
+ {"generations and pins", &ClaimRuntime{Generations: true, Pins: true}},
+ }
+ for _, mode := range []SwapMode{ModeOverlap, ModeHold} {
+ for _, rc := range runtimes {
+ t.Run(string(mode)+"/"+rc.name, func(t *testing.T) {
+ f := newSwapFixture(t)
+ w := &swapWork{}
+ f.a.cfg.HostWrites = hostwrite.Config{Enabled: true, Selector: &swapSelector{work: w}}
+ f.a.cfg.Claims = rc.r
+ res := f.a.Swap(context.Background(), f.attempt(mode), f.newest)
+ if res.Outcome != SwapAbandoned {
+ t.Fatalf("Swap = %+v, want abandoned", res)
+ }
+ if !strings.Contains(res.Reason, errClaimsNoGenerations.Error()) && !strings.Contains(res.Reason, ErrClaimIsolationUnavailable.Error()) {
+ t.Fatalf("Reason %q: want the claims guard's refusal", res.Reason)
+ }
+ gens, raises, restores := f.b.calls()
+ if gens != 0 || raises != 0 || restores != 0 {
+ t.Fatalf("StartStaged %d, raises %d, restores %d: want no side effect", gens, raises, restores)
+ }
+ if f.b.lastProc().isStopped() {
+ t.Fatal("the incumbent was stopped")
+ }
+ if ev := f.b.events(); indexPrefix(ev, "stop:") >= 0 || indexPrefix(ev, "raise:") >= 0 {
+ t.Fatalf("events %v: a refused claims-backed swap acted", ev)
+ }
+ if n := w.withCalls(); n != 0 {
+ t.Fatalf("With calls = %d: a refused swap entered the selected work", n)
+ }
+ f.a.mu.Lock()
+ e := f.a.entries[f.host]
+ state, held, members := e.state, e.holdNext, len(e.ops.members)
+ f.a.mu.Unlock()
+ if state != actAwake || held != nil || members != 0 {
+ t.Fatalf("entry state %v, holdNext %v, members %d: want awake, untouched", state, held, members)
+ }
+ })
+ }
+ }
+}
+
+// claimSpawn stands in for an open isolated launch, and only tests may set
+// it: production must always meet claimsSwapRefusal's refusal. No
+// non-test file of the package assigns the field.
+func TestClaimSpawnIsATestSeam(t *testing.T) {
+ fset := token.NewFileSet()
+ pkgs, err := parser.ParseDir(fset, ".", func(fi fs.FileInfo) bool { return !strings.HasSuffix(fi.Name(), "_test.go") }, 0)
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, pkg := range pkgs {
+ for name, file := range pkg.Files {
+ ast.Inspect(file, func(n ast.Node) bool {
+ switch x := n.(type) {
+ case *ast.AssignStmt:
+ for _, l := range x.Lhs {
+ if sel, ok := l.(*ast.SelectorExpr); ok && sel.Sel.Name == "claimSpawn" {
+ t.Errorf("%s: %s assigns claimSpawn", name, fset.Position(x.Pos()))
+ }
+ }
+ case *ast.KeyValueExpr:
+ if id, ok := x.Key.(*ast.Ident); ok && id.Name == "claimSpawn" {
+ t.Errorf("%s: %s sets claimSpawn in a literal", name, fset.Position(x.Pos()))
+ }
+ }
+ return true
+ })
+ }
+ }
+}
diff --git a/internal/activator/swap.go b/internal/activator/swap.go
index 6431aa318..bee8dd5b0 100644
--- a/internal/activator/swap.go
+++ b/internal/activator/swap.go
@@ -295,12 +295,12 @@ func (a *Service) genHealthy(ctx context.Context, g Gen) error {
// hook (plan 3 Task 10).
//
// Every claims-on launch is closed on this base (index P2-9): the isolated
-// root launcher cannot carry claim reservations. So a claims-on spawn is
-// refused before anything runs, as a claims admission (NoOwnership, the
-// attempt abandoned), and is never sent down the claims-off StartStaged.
-// swapOverlap has already refused a runtime without Generations and Pins
-// (errClaimsNoGenerations); one with both reaches this refusal. claimSpawn
-// is the test seam that stands in for the open launch.
+// root launcher cannot carry claim reservations. swapOverlap refuses every
+// claims-backed attempt before its raise (claimsSwapRefusal), so production
+// never reaches the claims-on arm here; only the claimSpawn test seam, the
+// stand-in for an open launch, does. Without the seam the arm still refuses,
+// as a claims admission (NoOwnership, the attempt abandoned), and a
+// claims-on spawn is never sent down the claims-off StartStaged.
func (a *Service) spawnSuccessor(ctx context.Context, gb SwapBackend, e *actEntry, rt registry.Route, work hostwrite.Work, g Gen, gate func() error) (Gen, Proc, error) {
if hostwrite.Legacy(work) {
if err := gate(); err != nil {
@@ -333,10 +333,12 @@ func (a *Service) swapOverlap(ctx context.Context, gb SwapBackend, at Attempt, n
if err != nil {
return abandon(fmt.Errorf("admission: %w", err))
}
- if r := a.cfg.Claims; !hostwrite.Legacy(work) && (r == nil || !r.Generations || !r.Pins) {
- // Claims on, and the runtime cannot start a generation of its own:
- // refused before the admission and the raise (Task 10).
- return abandon(errClaimsNoGenerations)
+ if !hostwrite.Legacy(work) {
+ // Claims on: refused before the admission and the raise, whatever
+ // the runtime's capabilities (claimsSwapRefusal).
+ if err := a.claimsSwapRefusal(true); err != nil {
+ return abandon(err)
+ }
}
mctx, cancel := a.attemptCtx(ctx)
defer cancel()
diff --git a/internal/activator/swap_overlap_test.go b/internal/activator/swap_overlap_test.go
index 3b2f1d63d..0d646cbcb 100644
--- a/internal/activator/swap_overlap_test.go
+++ b/internal/activator/swap_overlap_test.go
@@ -416,9 +416,10 @@ func TestSwapClaimsOn(t *testing.T) {
// A runtime that can start a generation of its own, so each row
// is refused where it names, not by errClaimsNoGenerations.
f.a.cfg.Claims = &ClaimRuntime{Pins: true, Generations: true}
- if tc.wantGens > 0 {
- useClaimSpawn(f)
- }
+ // The seam stands in for an open launch, so each row reaches the
+ // refusal it names; without it, claimsSwapRefusal refuses every
+ // row first (the "closed launch, no seam" case below).
+ useClaimSpawn(f)
res := f.a.Swap(context.Background(), f.attempt(ModeOverlap), f.newest)
if res.Outcome != SwapAbandoned {
t.Fatalf("Swap = %+v, want abandoned", res)
@@ -448,11 +449,11 @@ func TestSwapClaimsOn(t *testing.T) {
t.Fatalf("With calls = %d, want 3 (the admission, the exec and the switch)", w.withCalls())
}
})
- // Every claims-on launch is closed on this base (index P2-9). A
- // claims-on swap that passes every check reaches the spawn and is
- // refused there: abandoned (never failed, which would halt the cohort
- // for a build that never ran), nothing spawned, and never sent down the
- // claims-off StartStaged. The raised parent is restored.
+ // Every claims-on launch is closed on this base (index P2-9). Without
+ // the seam a claims-on swap is refused before its admission and its
+ // raise (claimsSwapRefusal, final review F9): abandoned (never failed,
+ // which would halt the cohort for a build that never ran), nothing
+ // raised or spawned, and never sent down the claims-off StartStaged.
t.Run("closed launch, no seam", func(t *testing.T) {
f := newSwapFixture(t)
w := &swapWork{}
@@ -463,14 +464,14 @@ func TestSwapClaimsOn(t *testing.T) {
t.Fatalf("Swap = %+v, want abandoned: %v", res, ErrClaimIsolationUnavailable)
}
gens, raises, restores := f.b.calls()
- if gens != 0 || raises != 1 || restores != 1 {
- t.Fatalf("StartStaged %d, raises %d, restores %d: want nothing spawned and the raise undone", gens, raises, restores)
+ if gens != 0 || raises != 0 || restores != 0 {
+ t.Fatalf("StartStaged %d, raises %d, restores %d: want nothing raised or spawned", gens, raises, restores)
}
if ev := f.b.events(); indexPrefix(ev, "startGen:") >= 0 {
t.Fatalf("events %v: a claims-on swap went down the claims-off spawn", ev)
}
- if w.withCalls() != 2 {
- t.Fatalf("With calls = %d, want 2 (the admission and the exec)", w.withCalls())
+ if w.withCalls() != 0 {
+ t.Fatalf("With calls = %d, want 0 (refused before the admission)", w.withCalls())
}
if got := f.appSocket(t); got != f.incumbent.Socket {
t.Fatalf("AppSocket = %q, want the incumbent", got)
diff --git a/internal/activator/swaphold.go b/internal/activator/swaphold.go
index 40877c47c..5878a98b8 100644
--- a/internal/activator/swaphold.go
+++ b/internal/activator/swaphold.go
@@ -79,11 +79,14 @@ func (a *Service) swapHold(ctx context.Context, at Attempt, newest func(string)
if err != nil {
return abandon(fmt.Errorf("admission: %w", err))
}
- if r := a.cfg.Claims; !hostwrite.Legacy(work) && (r == nil || !r.Generations || !r.Pins) {
- // Claims on, and the runtime cannot start a pinned generation of its
- // own: refused before anything stops (Task 10). A stop whose start
- // cannot happen would only cost the instance its running process.
- return abandon(errClaimsNoGenerations)
+ if !hostwrite.Legacy(work) {
+ // Claims on: refused before anything stops, whatever the runtime's
+ // capabilities (claimsSwapRefusal). The pinned wake would meet the
+ // closed claims launch, and a stop whose start cannot happen would
+ // only cost the instance its running process.
+ if err := a.claimsSwapRefusal(false); err != nil {
+ return abandon(err)
+ }
}
sctx, scancel := context.WithCancel(ctx)
defer scancel()