carlos / platform Public

activator: a strict handle owns its host's group until its cleanup succeeds (Task 7b fix round 2)

- strictOwners replaces strictPending: a strict spawn registers its handle
  as the host's owner, live while the child runs and pending while its
  cleanup is retried. While a host has an owner every spawn of it is
  refused, ordinary ones included (ErrStrictGroupInUse,
  ErrStrictCleanupPending); the check and an ordinary spawn's start share
  strictMu. With Strict nil the spawn is still main's exactly.
- The exit path's CleanupHost is skipped while cleanup is pending; the
  retry owner removes the group directory, under strictMu, once the handle
  closes, so the retained handle's identity is never removed under it.
- Tests: a fake handle whose identity is the group directory under a
  temp-dir cgroup fixture covers a drain timeout then an empty group, and a
  transient close failure on an empty group; both recover, the directory
  goes after cleanup, an ordinary spawn during pending cleanup is refused
  and never runs, and strict spawns work afterwards. A live strict child
  refuses an ordinary sibling.
- copy: MV-N111, MV-N112, pending.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude agent, operated by paul@keymail.dev 5832a73fabc4ca09078cb52085777a04a3ee841b parent dfeee62
3 files changed, +338 −46
  • docs/superpowers/plans/2026-10-06-moves-phase1-copy.md +2 −0
  • internal/activator/backend_exec.go +112 −45
  • internal/activator/backend_exec_test.go +224 −1
diff --git a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
index 38cb3aee4..9c0d3c7b3 100644
--- a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
+++ b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
@@ -150,6 +150,8 @@ Columns: id, file, text, status.
| MV-N108 | internal/activator/backend_exec.go | S2: the agent is not placed in its delegated cgroup subtree, so it cannot spawn strictly | pending |
| MV-N109 | internal/activator/backend_exec.go | S2: this host's last strict group has not been cleaned up yet | pending |
| MV-N110 | internal/release/materializeheld.go | release: a held publication needs its authority | pending |
+| MV-N111 | internal/activator/backend_exec.go | activator: a strict child of this host is still running in its group | pending |
+| MV-N112 | internal/activator/backend_exec.go | activator: this host's last strict group is still being cleaned up | pending |
## Plan 2
diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go
index 16b8fe122..e8515f3f5 100644
--- a/internal/activator/backend_exec.go
+++ b/internal/activator/backend_exec.go
@@ -177,13 +177,25 @@ type ExecBackend struct {
// preflight. Nil: Cgroups.StrictPreflight, the very function
// PrepareStrict runs, so S2 and the spawn cannot disagree.
strictPreflight func() error
- // strictPending holds, per host, a strict group whose drain or close
- // failed on exit, with its handle, until a retry cleans it up. While a
- // host has one, its strict spawns are refused.
- strictMu sync.Mutex
- strictPending map[string]*pendingStrict
+ // strictOwners holds, per host, the strict handle that owns the host's
+ // group: from the strict spawn until its cleanup succeeds, including
+ // any retries. While a host has an owner, no other spawn of that host,
+ // strict or ordinary, may use the group, and the exit path may not
+ // remove it (moves design §5.3).
+ strictMu sync.Mutex
+ strictOwners map[string]*strictOwner
}
+// ErrStrictGroupInUse is a spawn refused because a strict child of the same
+// host is alive in the host's group: an ordinary child placed beside it
+// would be killed by its exit drain.
+var ErrStrictGroupInUse = errors.New("activator: a strict child of this host is still running in its group")
+
+// ErrStrictCleanupPending is a spawn refused because the host's last strict
+// group has not been cleaned up: its retained handle still owns the group,
+// and a retry's drain would kill anything placed in it.
+var ErrStrictCleanupPending = errors.New("activator: this host's last strict group is still being cleaned up")
+
// ErrStrictRequired is a spawn refused because its host is in a move and
// the child could not be started under strict containment (moves design
// §5.3): no delegated cgroup subtree, a populated or unverifiable group, or
@@ -260,24 +272,54 @@ func (b *ExecBackend) tier(rt registry.Route) cgroup.Limit {
// no child shape can start a host's process outside the strict group. The
// returned settle runs after the child has been reaped: for a strict spawn
// it drains and closes the group, for a plain one it does nothing.
+//
+// A strict handle owns the host's group from the spawn until its cleanup
+// succeeds (strictOwners). While it does, every other spawn of the host is
+// refused, an ordinary one included: the check and an ordinary spawn's
+// start happen under strictMu, so no strict group can be prepared for the
+// host in between. With Strict nil (a box with no moves wiring) none of
+// this runs: the spawn is main's exactly.
func (b *ExecBackend) spawnInCgroup(rt registry.Route, build func() *exec.Cmd) (*exec.Cmd, func(), error) {
- if b.Strict == nil || !b.Strict(rt.Host) {
+ if b.Strict == nil {
cmd, err := b.Cgroups.Start(rt.Host, b.tier(rt), build)
return cmd, func() {}, err
}
- if b.strictCleanupPending(rt.Host) {
- return nil, nil, fmt.Errorf("%s: %w: its last strict group has not been cleaned up yet", rt.Host, ErrStrictRequired)
+ strict := b.Strict(rt.Host)
+ b.strictMu.Lock()
+ if o := b.strictOwners[rt.Host]; o != nil {
+ b.strictMu.Unlock()
+ cause := ErrStrictGroupInUse
+ if o.pending {
+ cause = ErrStrictCleanupPending
+ }
+ if strict {
+ return nil, nil, fmt.Errorf("%s: %w: %w", rt.Host, ErrStrictRequired, cause)
+ }
+ return nil, nil, fmt.Errorf("%s: %w", rt.Host, cause)
}
+ if !strict {
+ cmd, err := b.Cgroups.Start(rt.Host, b.tier(rt), build)
+ b.strictMu.Unlock()
+ return cmd, func() {}, err
+ }
+ o := &strictOwner{}
+ if b.strictOwners == nil {
+ b.strictOwners = map[string]*strictOwner{}
+ }
+ b.strictOwners[rt.Host] = o
+ b.strictMu.Unlock()
sp, err := b.prepareStrictProc(rt.Host, b.tier(rt))
if err != nil {
+ b.disown(rt.Host, o, false)
return nil, nil, fmt.Errorf("%s: %w: %v", rt.Host, ErrStrictRequired, err)
}
+ o.sp = sp
cmd := build()
if err := sp.Start(cmd); err != nil {
- b.settleStrict(rt.Host, sp)
+ b.settleStrict(rt.Host, o)
return nil, nil, fmt.Errorf("%s: %w: %v", rt.Host, ErrStrictRequired, err)
}
- return cmd, func() { b.settleStrict(rt.Host, sp) }, nil
+ return cmd, func() { b.settleStrict(rt.Host, o) }, nil
}
// prepareStrictProc is prepareStrict with its default applied.
@@ -295,76 +337,101 @@ func (b *ExecBackend) prepareStrictProc(host string, limit cgroup.Limit) (strict
return p, nil
}
-// pendingStrict is a strict group whose cleanup has not succeeded yet.
+// strictOwner is the strict handle that owns a host's group. pending is
+// set once its first cleanup attempt failed and retries own the group.
// drained records a KillAndEmpty that already succeeded: the handle refuses
// a second one, so a retry after a failed Close only closes.
-type pendingStrict struct {
+type strictOwner struct {
sp strictProc
+ pending bool
drained bool
}
// attempt is one cleanup attempt: drain (unless already drained), then
// close.
-func (p *pendingStrict) attempt(host string) error {
- if !p.drained {
+func (o *strictOwner) attempt(host string) error {
+ if !o.drained {
ctx, cancel := context.WithTimeout(context.Background(), strictDrainTimeout)
- err := p.sp.KillAndEmpty(ctx)
+ err := o.sp.KillAndEmpty(ctx)
cancel()
if err != nil {
log.Printf("activator: %s: strict group did not drain: %v", host, err)
return err
}
- p.drained = true
+ o.drained = true
}
- if err := p.sp.Close(); err != nil {
+ if err := o.sp.Close(); err != nil {
log.Printf("activator: %s: strict group not closed: %v", host, err)
return err
}
return nil
}
-// settleStrict drains a strict child's group and closes the handle. On a
-// failure the handle is kept as the host's pending cleanup, retried on
-// strictCleanupBackoff, and the host's strict spawns are refused until a
-// retry succeeds.
-func (b *ExecBackend) settleStrict(host string, sp strictProc) {
- p := &pendingStrict{sp: sp}
- if p.attempt(host) == nil {
+// settleStrict drains a strict child's group and closes the handle. On
+// success the owner lets go and the exit path's cleanupHost removes the
+// group as before. On a failure the owner stays, marked pending, retried on
+// strictCleanupBackoff: until a retry succeeds every spawn of the host is
+// refused and the exit path leaves the group directory alone, because the
+// handle's identity is that directory and removing it would strand the
+// handle for ever.
+func (b *ExecBackend) settleStrict(host string, o *strictOwner) {
+ if o.attempt(host) == nil {
+ b.disown(host, o, false)
return
}
b.strictMu.Lock()
- if b.strictPending == nil {
- b.strictPending = map[string]*pendingStrict{}
- }
- b.strictPending[host] = p
+ o.pending = true
b.strictMu.Unlock()
- go b.retryStrictCleanup(host, p, append([]time.Duration(nil), strictCleanupBackoff...))
+ go b.retryStrictCleanup(host, o, append([]time.Duration(nil), strictCleanupBackoff...))
+}
+
+// disown ends o's ownership of host's group. removeGroup removes the group
+// directory first, under the same lock, so no spawn can recreate it in
+// between: the retry owner's last act.
+func (b *ExecBackend) disown(host string, o *strictOwner, removeGroup bool) {
+ b.strictMu.Lock()
+ defer b.strictMu.Unlock()
+ if b.strictOwners[host] != o {
+ return
+ }
+ if removeGroup {
+ b.Cgroups.CleanupHost(host)
+ }
+ delete(b.strictOwners, host)
}
-// retryStrictCleanup retries p's cleanup once after each wait in schedule,
-// a copy taken when the cleanup was queued.
-func (b *ExecBackend) retryStrictCleanup(host string, p *pendingStrict, schedule []time.Duration) {
+// cleanupHost is the exit path's CleanupHost, skipped while a strict
+// handle's cleanup is pending: the retry owner removes the group once the
+// handle is closed.
+func (b *ExecBackend) cleanupHost(host string) {
+ if b.strictCleanupPending(host) {
+ return
+ }
+ b.Cgroups.CleanupHost(host)
+}
+
+// retryStrictCleanup retries o's cleanup once after each wait in schedule,
+// a copy taken when the cleanup was queued, and removes the group directory
+// when one succeeds.
+func (b *ExecBackend) retryStrictCleanup(host string, o *strictOwner, schedule []time.Duration) {
for i, wait := range schedule {
time.Sleep(wait)
- if p.attempt(host) == nil {
- b.strictMu.Lock()
- if b.strictPending[host] == p {
- delete(b.strictPending, host)
- }
- b.strictMu.Unlock()
+ if o.attempt(host) == nil {
+ b.disown(host, o, true)
log.Printf("activator: %s: strict group cleaned up on retry %d", host, i+1)
return
}
}
- log.Printf("activator: %s: strict group still not cleaned up after %d retries; strict spawns of this host stay refused until the agent restarts", host, len(schedule))
+ log.Printf("activator: %s: strict group still not cleaned up after %d retries; spawns of this host stay refused until the agent restarts", host, len(schedule))
}
-// strictCleanupPending reports whether host has a strict group awaiting
-// cleanup.
+// strictCleanupPending reports whether host's group is owned by a strict
+// handle whose cleanup is being retried.
func (b *ExecBackend) strictCleanupPending(host string) bool {
b.strictMu.Lock()
defer b.strictMu.Unlock()
- return b.strictPending[host] != nil
+ o := b.strictOwners[host]
+ return o != nil && o.pending
}
// NextSpawnStrict is the source's S2 input (moves design §7.3, §5.3): will
@@ -1172,7 +1239,7 @@ func (b *ExecBackend) startChild(ctx context.Context, rt registry.Route) (Proc,
settle()
// Last exiter wins: rmdir refuses while the sidecar (same host
// cgroup) is still alive, and that failure is tolerated.
- b.Cgroups.CleanupHost(host)
+ b.cleanupHost(host)
close(p.done)
}()
@@ -1581,7 +1648,7 @@ func (b *ExecBackend) startSidecarChild(_ context.Context, rt registry.Route) (P
go func() {
p.err = cmd.Wait()
settle()
- b.Cgroups.CleanupHost(rt.Host)
+ b.cleanupHost(rt.Host)
close(p.done)
}()
return p, nil
@@ -1647,7 +1714,7 @@ func (b *ExecBackend) StartWorker(ctx context.Context, rt registry.Route, extra
emitEvent(b.Logs, rt, "info", "worker exited (exit 0)")
}
settle()
- b.Cgroups.CleanupHost(rt.Host)
+ b.cleanupHost(rt.Host)
close(p.done)
}()
return p, spawnVer, nil
diff --git a/internal/activator/backend_exec_test.go b/internal/activator/backend_exec_test.go
index 8f57b4525..51b3d0f2c 100644
--- a/internal/activator/backend_exec_test.go
+++ b/internal/activator/backend_exec_test.go
@@ -10,6 +10,7 @@ import (
"runtime"
"strings"
"sync"
+ "sync/atomic"
"testing"
"time"
@@ -738,7 +739,8 @@ func TestStrictCleanupGivesUpButStaysRefused(t *testing.T) {
host := "stuck.example.com"
b := testExecBackend(t)
h := &flakyStrict{recordingStrict: newRecordingStrict(), killFails: 100}
- b.settleStrict(host, h)
+ b.Strict = func(string) bool { return true }
+ b.settleStrict(host, b.ownStrictForTest(host, h))
time.Sleep(200 * time.Millisecond)
if got := strings.Count(strings.Join(h.seen(), ","), "kill-and-empty failed"); got != 3 {
t.Fatalf("drain attempts = %d, want the first and two retries", got)
@@ -755,3 +757,224 @@ func TestStrictCleanupGivesUpButStaysRefused(t *testing.T) {
t.Fatalf("spawn: %v, want ErrStrictRequired", err)
}
}
+
+// ownStrictForTest registers sp as host's strict owner, as a strict spawn
+// does, so a test can drive settleStrict directly.
+func (b *ExecBackend) ownStrictForTest(host string, sp strictProc) *strictOwner {
+ b.strictMu.Lock()
+ defer b.strictMu.Unlock()
+ if b.strictOwners == nil {
+ b.strictOwners = map[string]*strictOwner{}
+ }
+ o := &strictOwner{sp: sp}
+ b.strictOwners[host] = o
+ return o
+}
+
+// dirStrict is a strict handle whose identity is a real directory: the
+// host's group directory under a temp-dir cgroup fixture. Like
+// cgroup.Prepared's identityOK, once that directory is removed (or
+// replaced) the handle can neither drain nor close again, for ever. It
+// models what an exit-path CleanupHost would do to a retained handle.
+type dirStrict struct {
+ *recordingStrict
+ dir string
+ id os.FileInfo
+ killFails, closeFails int
+}
+
+func newDirStrict(t *testing.T, dir string) *dirStrict {
+ t.Helper()
+ if err := os.MkdirAll(dir, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ id, err := os.Stat(dir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return &dirStrict{recordingStrict: newRecordingStrict(), dir: dir, id: id}
+}
+
+func (d *dirStrict) identityOK() bool {
+ now, err := os.Stat(d.dir)
+ return err == nil && os.SameFile(now, d.id)
+}
+
+func (d *dirStrict) KillAndEmpty(ctx context.Context) error {
+ if !d.identityOK() {
+ d.note("kill refused: group gone")
+ return cgroup.ErrStrictRefused
+ }
+ d.mu.Lock()
+ fail := d.killFails > 0
+ if fail {
+ d.killFails--
+ }
+ d.mu.Unlock()
+ if fail {
+ d.note("kill-and-empty timed out")
+ return cgroup.ErrStrictDeadline
+ }
+ return d.recordingStrict.KillAndEmpty(ctx)
+}
+
+func (d *dirStrict) Close() error {
+ if !d.identityOK() {
+ d.note("close refused: group gone")
+ return cgroup.ErrStrictRefused
+ }
+ d.mu.Lock()
+ fail := d.closeFails > 0
+ if fail {
+ d.closeFails--
+ }
+ d.mu.Unlock()
+ if fail {
+ d.note("close failed")
+ return cgroup.ErrStrictRefused
+ }
+ return d.recordingStrict.Close()
+}
+
+// TestPendingStrictCleanupOwnsTheGroup is the invariant: while a host has
+// pending strict cleanup, nothing else may use or remove that host's
+// cgroup. The exit path does not remove the group directory under a
+// retained handle (with a handle whose identity is that directory, a
+// removal would strand it for ever), so a drain that timed out before the
+// group emptied, and a transient Close failure on an empty group, both
+// recover on retry; the directory goes only once the handle is closed, and
+// strict spawns work afterwards. Meanwhile any spawn of the host, an
+// ordinary one included (Strict switched off), is refused and never runs.
+func TestPendingStrictCleanupOwnsTheGroup(t *testing.T) {
+ old := spawnHealthTimeout
+ spawnHealthTimeout = 300 * time.Millisecond
+ t.Cleanup(func() { spawnHealthTimeout = old })
+ host := "owned.example.com"
+ for name, tc := range map[string]struct {
+ killFails, closeFails int
+ want string
+ }{
+ "a drain timeout, then the group empties": {1, 0, "start,kill-and-empty timed out,kill-and-empty,close"},
+ "a transient close failure on an empty group": {0, 1, "start,kill-and-empty,close failed,close"},
+ } {
+ t.Run(name, func(t *testing.T) {
+ oldBackoff := strictCleanupBackoff
+ strictCleanupBackoff = []time.Duration{500 * time.Millisecond, 500 * time.Millisecond}
+ t.Cleanup(func() { strictCleanupBackoff = oldBackoff })
+ b := testExecBackend(t)
+ b.Cgroups = delegatedManager(t)
+ b.strictPreflight = func() error { return nil }
+ groupDir := filepath.Join(b.Cgroups.Root(), cgroup.ExecName(host))
+ argvPath, _ := stageRecordingLive(t, b, host, "v1")
+ var strict atomic.Bool
+ strict.Store(true)
+ b.Strict = func(string) bool { return strict.Load() }
+ var mu sync.Mutex
+ var handles []*dirStrict
+ b.prepareStrict = func(string, cgroup.Limit) (strictProc, error) {
+ mu.Lock()
+ defer mu.Unlock()
+ h := newDirStrict(t, groupDir)
+ if len(handles) == 0 {
+ h.killFails, h.closeFails = tc.killFails, tc.closeFails
+ }
+ handles = append(handles, h)
+ return h, nil
+ }
+ startOnce := func() error {
+ rt := registry.Route{Host: host, Kind: "instance", Socket: filepath.Join(t.TempDir(), "i.sock")}
+ ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+ defer cancel()
+ proc, _, err := b.Start(ctx, rt)
+ if proc != nil {
+ _ = proc.Stop(context.Background())
+ }
+ return err
+ }
+
+ _ = startOnce()
+ waitForFile(t, argvPath, time.Second)
+ deadline := time.Now().Add(2 * time.Second)
+ for !b.strictCleanupPending(host) {
+ if time.Now().After(deadline) {
+ t.Fatalf("no cleanup pending: %v", handles[0].seen())
+ }
+ time.Sleep(5 * time.Millisecond)
+ }
+ // The child has exited and its exit path has run: the group
+ // directory is still there, owned by the retained handle.
+ time.Sleep(50 * time.Millisecond)
+ if _, err := os.Stat(groupDir); err != nil {
+ t.Fatalf("the exit path removed the group under a pending handle: %v", err)
+ }
+ // An ordinary spawn of the host is refused while cleanup is
+ // pending, and nothing runs.
+ strict.Store(false)
+ if err := os.Remove(argvPath); err != nil {
+ t.Fatal(err)
+ }
+ if err := startOnce(); !errors.Is(err, ErrStrictCleanupPending) {
+ t.Fatalf("ordinary spawn during pending cleanup: %v, want ErrStrictCleanupPending", err)
+ }
+ time.Sleep(100 * time.Millisecond)
+ if _, err := os.Stat(argvPath); !os.IsNotExist(err) {
+ t.Fatalf("an ordinary child ran in a group awaiting cleanup: %v", err)
+ }
+ strict.Store(true)
+
+ deadline = time.Now().Add(4 * time.Second)
+ for b.strictCleanupPending(host) {
+ if time.Now().After(deadline) {
+ t.Fatalf("cleanup never succeeded: %v", handles[0].seen())
+ }
+ time.Sleep(5 * time.Millisecond)
+ }
+ if got := strings.Join(handles[0].seen(), ","); got != tc.want {
+ t.Fatalf("first handle's calls = %s, want %s", got, tc.want)
+ }
+ if _, err := os.Stat(groupDir); !os.IsNotExist(err) {
+ t.Fatalf("the group directory outlived its handle's cleanup: %v", err)
+ }
+ _ = startOnce()
+ waitForFile(t, argvPath, time.Second)
+ mu.Lock()
+ n := len(handles)
+ mu.Unlock()
+ if n != 2 {
+ t.Fatalf("strict groups prepared = %d, want a second one after cleanup", n)
+ }
+ })
+ }
+}
+
+// TestALiveStrictGroupRefusesOrdinarySpawns: while a strict child of a
+// host is alive, an ordinary spawn of the same host (Strict switched off
+// meanwhile) is refused rather than placed in the strict group, where the
+// strict child's exit drain would kill it.
+func TestALiveStrictGroupRefusesOrdinarySpawns(t *testing.T) {
+ host := "live.example.com"
+ b := testExecBackend(t)
+ var strict atomic.Bool
+ strict.Store(true)
+ b.Strict = func(string) bool { return strict.Load() }
+ rec := newRecordingStrict()
+ b.prepareStrict = func(string, cgroup.Limit) (strictProc, error) { return rec, nil }
+ rt := registry.Route{Host: host}
+ cmd, settle, err := b.spawnInCgroup(rt, func() *exec.Cmd { return exec.Command("sleep", "5") })
+ if err != nil {
+ t.Fatalf("strict spawn: %v", err)
+ }
+ strict.Store(false)
+ ran := false
+ if _, _, err := b.spawnInCgroup(rt, func() *exec.Cmd { ran = true; return exec.Command("/bin/true") }); !errors.Is(err, ErrStrictGroupInUse) || ran {
+ t.Fatalf("ordinary spawn beside a live strict child: %v (ran %v), want ErrStrictGroupInUse", err, ran)
+ }
+ _ = cmd.Process.Kill()
+ _ = cmd.Wait()
+ settle()
+ if _, settle2, err := b.spawnInCgroup(rt, func() *exec.Cmd { return exec.Command("/bin/true") }); err != nil {
+ t.Fatalf("ordinary spawn after the strict child settled: %v", err)
+ } else {
+ settle2()
+ }
+}