amadan

carlos / platform Public

Merge pull request #91 from carlosframework/worktree-metrics-instances-fixes

console + agent: instances page gets real readings for exec-backed apps, stops flapping, stops paying the sweep
Paul Campbell 9668d302316b38cf1283b7eaf63efd982db5c3ea parent 934414cparent d3b2b56
25 files changed, +3678 −553
  • docs/superpowers/plans/2026-08-08-exec-child-accounting.md +1530 −0
  • docs/superpowers/specs/2026-08-02-host-process-monitoring-design.md +14 −3
  • docs/superpowers/specs/2026-08-07-exec-child-accounting-design.md +170 −0
  • docs/superpowers/specs/2026-08-08-exec-instance-metrics-design.md +11 −0
  • internal/activator/activator.go +0 −29
  • internal/activator/activator_test.go +0 −73
  • internal/activator/backend_exec.go +83 −54
  • internal/activator/backend_unit.go +8 −0
  • internal/cgroup/cgroup.go +274 −0
  • internal/cgroup/cgroup_linux.go +38 −0
  • internal/cgroup/cgroup_linux_test.go +185 −0
  • internal/cgroup/cgroup_other.go +14 −0
  • internal/cgroup/cgroup_test.go +269 −0
  • internal/console/metricsview.go +252 −56
  • internal/console/metricsview_test.go +698 −0
  • internal/console/server.go +5 −3
  • internal/host/agent.go +10 −1
  • internal/host/host_test.go +13 −0
  • internal/host/metrics_loop.go +12 −22
  • internal/host/metrics_probes.go +41 −76
  • internal/host/metrics_probes_test.go +35 −49
  • internal/host/units/carlos-edge.service +12 −1
  • internal/metrics/pidparse.go +0 −101
  • internal/metrics/pidparse_test.go +0 −83
  • internal/metrics/sample.go +4 −2
diff --git a/docs/superpowers/plans/2026-08-08-exec-child-accounting.md b/docs/superpowers/plans/2026-08-08-exec-child-accounting.md
new file mode 100644
index 000000000..52aec680a
--- /dev/null
+++ b/docs/superpowers/plans/2026-08-08-exec-child-accounting.md
@@ -0,0 +1,1530 @@
+# Exec-Child Accounting Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Per-instance resource readings for exec-backed routes — each exec
+child gets its own cgroup under a subtree systemd delegates to the agent, the
+sampler reads it, the owner Instances page shows it.
+
+**Architecture:** A new `internal/cgroup` package owns the delegated subtree
+(delegation dance at agent start, per-host `carlos-exec@<host>` cgroups,
+clone3-into-cgroup spawn with a latched write-PID fallback). The activator
+spawns instance/sidecar/seed through it; the host sampler gains the subtree
+as a third scan root; the console allowlist gains a second key per host.
+Everything degrades to today's behavior when delegation is absent (nil
+manager) — fail toward absent readings, never toward a broken spawn path.
+
+**Tech Stack:** Go 1.26 (stdlib only — `syscall.SysProcAttr{UseCgroupFD}`
+for clone3 `CLONE_INTO_CGROUP`; no new dependencies). Module
+`github.com/carlosframework/platform`.
+
+**Spec:** `docs/superpowers/specs/2026-08-07-exec-child-accounting-design.md`
+— read it before starting a task.
+
+## Global Constraints
+
+- Build/test env (sandboxed shells): `GOCACHE=/tmp/claude-1001/gocache
+ GOFLAGS=-mod=mod GOPROXY=off`. Tests in `internal/activator`,
+ `internal/host`, `internal/edge` need the sandbox disabled (unix-socket
+ permission errors are the tell — pre-existing, not yours).
+- Before every commit: `go build ./...`, `go vet ./...`, and the touched
+ package's tests green; `gofmt -l` clean for files you touch.
+- Wire shape is additive only: `Sample.Units` gains new keys
+ (`carlos-exec@<host>` — no `.service` suffix, deliberately); nothing
+ already in a production bucket may stop parsing.
+- Probe convention (`internal/host/probes.go`): fail toward nil, never a
+ guessed zero. Thin OS glue is untested by convention; directory WALKS and
+ pure parsers are tested against fixtures. Tests never touch `/proc`,
+ live cgroups, or a live syscall.
+- The agent is NOT root (`User=carlos`). Nothing here may assume privilege;
+ cgroup writability comes from systemd delegation (`Delegate=`) only.
+- Every cgroup-manager failure degrades to a logged line + today's
+ behavior. A spawn must never fail because accounting failed.
+- Naming discipline: the string `carlos-exec@` is defined ONCE, in
+ `internal/cgroup` (`ExecName`/`IsExecName`); everyone else calls those.
+- Commit messages: repo style is `component: what changed` (lowercase), body
+ optional, ending with `Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>`.
+
+---
+
+### Task 1: `internal/cgroup` — names and the /proc/self/cgroup parser
+
+**Files:**
+- Create: `internal/cgroup/cgroup.go`
+- Create: `internal/cgroup/cgroup_test.go`
+
+**Interfaces:**
+- Consumes: nothing (new leaf package; stdlib only).
+- Produces: `ExecName(host string) string` (returns `"carlos-exec@" + host`),
+ `IsExecName(name string) bool`, `ParseSelfCgroup(data string) (string, bool)`
+ (relative v2 cgroup path, no leading slash). Tasks 2–5 and 7 use these.
+
+- [ ] **Step 1: Write the failing tests**
+
+Create `internal/cgroup/cgroup_test.go`:
+
+```go
+package cgroup
+
+import "testing"
+
+func TestExecNameAndIsExecName(t *testing.T) {
+ if got := ExecName("keymail.dev"); got != "carlos-exec@keymail.dev" {
+ t.Fatalf("ExecName = %q, want %q", got, "carlos-exec@keymail.dev")
+ }
+ for name, want := range map[string]bool{
+ "carlos-exec@keymail.dev": true,
+ "carlos-exec@": false, // no host — not one of ours
+ "supervise": false, // the agent's own leaf
+ "carlos-app@x.service": false,
+ "cgroup.procs": false,
+ } {
+ if got := IsExecName(name); got != want {
+ t.Errorf("IsExecName(%q) = %v, want %v", name, got, want)
+ }
+ }
+}
+
+// TestParseSelfCgroup pins the /proc/self/cgroup v2 line format: the "0::"
+// line carries the unified-hierarchy path. v1-only content (no 0:: line)
+// and the root cgroup ("0::/") both report not-ok — neither is a subtree
+// systemd could have delegated to us.
+func TestParseSelfCgroup(t *testing.T) {
+ cases := []struct {
+ name, data, want string
+ ok bool
+ }{
+ {"systemd service", "0::/system.slice/carlos-edge.service\n", "system.slice/carlos-edge.service", true},
+ {"hybrid box, v2 line among v1", "12:pids:/system.slice\n0::/system.slice/carlos-edge.service\n", "system.slice/carlos-edge.service", true},
+ {"v1 only", "12:pids:/system.slice\n11:cpu:/system.slice\n", "", false},
+ {"root cgroup", "0::/\n", "", false},
+ {"empty", "", "", false},
+ }
+ for _, c := range cases {
+ got, ok := ParseSelfCgroup(c.data)
+ if got != c.want || ok != c.ok {
+ t.Errorf("%s: ParseSelfCgroup = (%q, %v), want (%q, %v)", c.name, got, ok, c.want, c.ok)
+ }
+ }
+}
+```
+
+- [ ] **Step 2: Run tests to verify they fail**
+
+Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/cgroup/ -count=1`
+Expected: FAIL — package does not exist / `ExecName` undefined.
+
+- [ ] **Step 3: Write the implementation**
+
+Create `internal/cgroup/cgroup.go`:
+
+```go
+// Package cgroup gives each exec-backed child its own cgroup under the
+// subtree systemd delegates to carlos-edge.service (Delegate= in the unit
+// file), so the metrics sampler can report per-instance figures for routes
+// that run as exec children of the agent — today those live inside the
+// agent's own cgroup and can never show readings. Design:
+// docs/superpowers/specs/2026-08-07-exec-child-accounting-design.md.
+//
+// The agent is NOT root: everything here works through ownership of the
+// delegated subtree, never privilege. When delegation is absent (old unit
+// file, a test, darwin) Setup returns a nil *Manager and every method on it
+// degrades to today's behavior — fail toward absent readings, never toward
+// a broken spawn path.
+package cgroup
+
+import "strings"
+
+// execPrefix is the ONE definition of the per-host exec cgroup naming —
+// activator.ExecName and the sampler's directory match both come through
+// this package.
+const execPrefix = "carlos-exec@"
+
+// ExecName is the cgroup directory name — and therefore the metrics sample
+// key — for an exec-backed host's children. Deliberately NOT dressed up as
+// a fake ".service": an operator who takes a dashboard row to `systemctl
+// status` must not be told a unit exists that doesn't.
+func ExecName(host string) string { return execPrefix + host }
+
+// IsExecName reports whether a cgroup directory name is one of our per-host
+// exec cgroups: the carlos-exec@ prefix with a non-empty host after it.
+func IsExecName(name string) bool {
+ return strings.HasPrefix(name, execPrefix) && len(name) > len(execPrefix)
+}
+
+// ParseSelfCgroup extracts the cgroup v2 path from /proc/self/cgroup
+// content — the "0::" unified-hierarchy line — relative (no leading slash),
+// ready to join under /sys/fs/cgroup. Not-ok when there is no v2 line (a
+// v1-only box) or the process sits in the root cgroup: neither is a subtree
+// systemd could have delegated to us.
+func ParseSelfCgroup(data string) (string, bool) {
+ for _, line := range strings.Split(data, "\n") {
+ rest, found := strings.CutPrefix(line, "0::")
+ if !found {
+ continue
+ }
+ p := strings.TrimPrefix(rest, "/")
+ if p == "" {
+ return "", false
+ }
+ return p, true
+ }
+ return "", false
+}
+```
+
+- [ ] **Step 4: Run tests to verify they pass**
+
+Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/cgroup/ -count=1`
+Expected: PASS
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add internal/cgroup/cgroup.go internal/cgroup/cgroup_test.go
+git commit -m "cgroup: the exec-child cgroup names and the /proc/self/cgroup parser
+
+Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
+```
+
+---
+
+### Task 2: `internal/cgroup` — the delegation dance
+
+**Files:**
+- Modify: `internal/cgroup/cgroup.go` (append)
+- Modify: `internal/cgroup/cgroup_test.go` (append)
+
+**Interfaces:**
+- Consumes: `IsExecName` from Task 1.
+- Produces: `type Manager struct` (unexported fields `root string`,
+ `logf func(string, ...any)`, `fallback atomic.Bool`),
+ `SetupAt(root string, logf func(string, ...any)) (*Manager, error)`
+ (the testable core), `Setup(logf func(string, ...any)) *Manager`
+ (production entry, nil on any failure), and nil-safe
+ `func (m *Manager) Root() string` ("" for nil). Tasks 3, 4, 6 use these.
+
+- [ ] **Step 1: Write the failing tests**
+
+Append to `internal/cgroup/cgroup_test.go` (add imports `os`,
+`path/filepath`, `strconv` to the existing import block):
+
+```go
+// newDelegatedRoot builds the temp-dir shape of a delegated cgroup: the two
+// control files systemd chowns to the unit's user when Delegate= is set.
+// Everything the dance does is plain file I/O, so a temp dir is a faithful
+// fixture — the same seam listCarlosUnits uses in internal/host.
+func newDelegatedRoot(t *testing.T) string {
+ t.Helper()
+ root := t.TempDir()
+ for _, f := range []string{"cgroup.procs", "cgroup.subtree_control"} {
+ if err := os.WriteFile(filepath.Join(root, f), nil, 0o644); err != nil {
+ t.Fatal(err)
+ }
+ }
+ return root
+}
+
+// TestSetupAtRefusesUndelegatedRoot: without Delegate= the control files are
+// root-owned (here: absent) — SetupAt must error so Setup degrades to the
+// nil manager instead of half-arming.
+func TestSetupAtRefusesUndelegatedRoot(t *testing.T) {
+ if _, err := SetupAt(t.TempDir(), t.Logf); err == nil {
+ t.Fatal("SetupAt on an undelegated root succeeded, want error")
+ }
+}
+
+// TestSetupAtDance pins the dance's observable results: the agent's own PID
+// moved into the supervise/ leaf (cgroup v2 forbids enabling controllers on
+// a cgroup with member processes), then the four controllers enabled in
+// subtree_control, and Root reporting the subtree.
+func TestSetupAtDance(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if m.Root() != root {
+ t.Errorf("Root = %q, want %q", m.Root(), root)
+ }
+ pid, err := os.ReadFile(filepath.Join(root, "supervise", "cgroup.procs"))
+ if err != nil {
+ t.Fatalf("supervise leaf not populated: %v", err)
+ }
+ if string(pid) != strconv.Itoa(os.Getpid()) {
+ t.Errorf("supervise/cgroup.procs = %q, want our own pid %d", pid, os.Getpid())
+ }
+ ctrl, err := os.ReadFile(filepath.Join(root, "cgroup.subtree_control"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if string(ctrl) != "+cpu +memory +io +pids" {
+ t.Errorf("subtree_control = %q, want %q", ctrl, "+cpu +memory +io +pids")
+ }
+}
+
+// TestSetupAtSweepsStaleExecDirs: a previous agent's per-host dirs are empty
+// after its children died with the service; the dance removes them. A
+// non-empty one (something still alive in it — shouldn't happen, but rmdir
+// would fail anyway) and a foreign name are left alone.
+func TestSetupAtSweepsStaleExecDirs(t *testing.T) {
+ root := newDelegatedRoot(t)
+ stale := filepath.Join(root, "carlos-exec@old.example.com")
+ if err := os.Mkdir(stale, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ occupied := filepath.Join(root, "carlos-exec@busy.example.com")
+ if err := os.MkdirAll(filepath.Join(occupied, "child"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ foreign := filepath.Join(root, "not-ours")
+ if err := os.Mkdir(foreign, 0o755); err != nil {
+ t.Fatal(err)
+ }
+
+ if _, err := SetupAt(root, t.Logf); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := os.Stat(stale); !os.IsNotExist(err) {
+ t.Errorf("stale exec dir survived the sweep: %v", err)
+ }
+ if _, err := os.Stat(occupied); err != nil {
+ t.Errorf("non-empty exec dir must be tolerated, not force-removed: %v", err)
+ }
+ if _, err := os.Stat(foreign); err != nil {
+ t.Errorf("foreign dir must be untouched: %v", err)
+ }
+}
+
+func TestNilManagerRoot(t *testing.T) {
+ var m *Manager
+ if got := m.Root(); got != "" {
+ t.Fatalf("nil Root = %q, want \"\"", got)
+ }
+}
+```
+
+- [ ] **Step 2: Run tests to verify they fail**
+
+Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/cgroup/ -count=1`
+Expected: FAIL — `SetupAt`, `Manager` undefined.
+
+- [ ] **Step 3: Write the implementation**
+
+Append to `internal/cgroup/cgroup.go` (extend the import block to
+`fmt`, `os`, `path/filepath`, `strconv`, `strings`, `sync/atomic`):
+
+```go
+// Manager owns the delegated subtree. A nil *Manager is the documented
+// "no delegation" state: every method is nil-safe and degrades to today's
+// behavior.
+type Manager struct {
+ root string
+ logf func(string, ...any)
+ // fallback, once latched, means clone3-into-cgroup is refused here (a
+ // seccomp filter older than the syscall, an old kernel) and children are
+ // started normally then moved by PID. A filter is static for the life of
+ // the process, so retrying the syscall on every spawn buys nothing.
+ fallback atomic.Bool
+}
+
+// Root is the delegated subtree's path — the sampler's third scan root.
+// "" for a nil manager (no delegation): the sampler skips an empty root.
+func (m *Manager) Root() string {
+ if m == nil {
+ return ""
+ }
+ return m.root
+}
+
+// Setup arms exec-child accounting for the running agent: resolve our own
+// cgroup from /proc/self/cgroup and run the delegation dance there. Any
+// failure — v1-only box, old unit file without Delegate=, a test, darwin —
+// returns nil with one logged line, and every spawn behaves exactly as
+// today. Fail toward absent readings, never toward a broken spawn path.
+func Setup(logf func(string, ...any)) *Manager {
+ b, err := os.ReadFile("/proc/self/cgroup")
+ if err != nil {
+ logf("agent: exec-child accounting disabled: %v", err)
+ return nil
+ }
+ rel, ok := ParseSelfCgroup(string(b))
+ if !ok {
+ logf("agent: exec-child accounting disabled: no cgroup v2 path in /proc/self/cgroup")
+ return nil
+ }
+ m, err := SetupAt(filepath.Join("/sys/fs/cgroup", rel), logf)
+ if err != nil {
+ logf("agent: exec-child accounting disabled (no delegated subtree — unit file without Delegate=?): %v", err)
+ return nil
+ }
+ logf("agent: exec-child accounting armed (delegated subtree %s)", m.Root())
+ return m
+}
+
+// SetupAt is the delegation dance against an explicit subtree root — the
+// testable core Setup wraps, and the constructor activator tests use over a
+// temp-dir fixture. Order matters: probe, move self to a leaf, THEN enable
+// controllers — cgroup v2 refuses subtree_control writes while the cgroup
+// itself still has member processes ("no internal processes").
+func SetupAt(root string, logf func(string, ...any)) (*Manager, error) {
+ // 1. Probe live delegation: both control files must be writable by us.
+ // systemd chowns exactly these to User= when Delegate= is set; without
+ // it this open is the cheap, side-effect-free "no" that keeps an
+ // undelegated box on today's path.
+ for _, name := range []string{"cgroup.procs", "cgroup.subtree_control"} {
+ f, err := os.OpenFile(filepath.Join(root, name), os.O_WRONLY, 0)
+ if err != nil {
+ return nil, fmt.Errorf("probe delegation: %w", err)
+ }
+ f.Close()
+ }
+ // 2. Move ourselves into a supervise/ leaf, making step 3 legal.
+ sup := filepath.Join(root, "supervise")
+ if err := os.MkdirAll(sup, 0o755); err != nil {
+ return nil, fmt.Errorf("create supervise leaf: %w", err)
+ }
+ if err := os.WriteFile(filepath.Join(sup, "cgroup.procs"), []byte(strconv.Itoa(os.Getpid())), 0o644); err != nil {
+ return nil, fmt.Errorf("move self into supervise leaf: %w", err)
+ }
+ // 3. Enable the controllers per-child cgroups need for memory.current,
+ // io.stat and pids accounting (cpu.stat exists regardless, but +cpu
+ // keeps utilization attributable). One write, all four: the kernel
+ // applies subtree_control writes atomically.
+ if err := os.WriteFile(filepath.Join(root, "cgroup.subtree_control"), []byte("+cpu +memory +io +pids"), 0o644); err != nil {
+ return nil, fmt.Errorf("enable controllers: %w", err)
+ }
+ m := &Manager{root: root, logf: logf}
+ // 4. Sweep per-host dirs a previous agent left behind — empty after a
+ // restart killed their members. rmdir refuses a non-empty dir, which is
+ // exactly the tolerance we want.
+ m.sweepStale()
+ return m, nil
+}
+
+// sweepStale removes leftover carlos-exec@* dirs under the root. Failures
+// are logged and tolerated: a dir that won't rmdir has something alive in
+// it, and killing is the service manager's job, not ours.
+func (m *Manager) sweepStale() {
+ entries, err := os.ReadDir(m.root)
+ if err != nil {
+ return
+ }
+ for _, e := range entries {
+ if !e.IsDir() || !IsExecName(e.Name()) {
+ continue
+ }
+ if err := os.Remove(filepath.Join(m.root, e.Name())); err != nil {
+ m.logf("cgroup: stale %s not removed: %v", e.Name(), err)
+ }
+ }
+}
+```
+
+- [ ] **Step 4: Run tests to verify they pass**
+
+Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/cgroup/ -count=1`
+Expected: PASS
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add internal/cgroup/cgroup.go internal/cgroup/cgroup_test.go
+git commit -m "cgroup: the delegation dance — probe, supervise leaf, controllers, sweep
+
+Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
+```
+
+---
+
+### Task 3: `internal/cgroup` — spawning into per-host cgroups
+
+**Files:**
+- Modify: `internal/cgroup/cgroup.go` (append)
+- Create: `internal/cgroup/cgroup_linux.go`
+- Create: `internal/cgroup/cgroup_other.go`
+- Create: `internal/cgroup/cgroup_linux_test.go`
+- Modify: `internal/cgroup/cgroup_test.go` (append)
+
+**Interfaces:**
+- Consumes: `Manager`, `ExecName` from Tasks 1–2.
+- Produces:
+ `func (m *Manager) Start(host string, build func() *exec.Cmd) (*exec.Cmd, error)`
+ — nil-safe (nil manager = plain `build().Start()`); `build` must return a
+ fully configured, UNSTARTED Cmd and may be called twice (a Cmd is not
+ reusable after a failed Start). On error the returned Cmd may be nil —
+ callers must not touch it.
+ `func (m *Manager) CleanupHost(host string)` — nil-safe, rmdir-tolerant.
+ Task 4 uses both.
+
+- [ ] **Step 1: Write the failing tests**
+
+Append to `internal/cgroup/cgroup_test.go` (add `os/exec` and `errors` to
+imports):
+
+```go
+// fakeStart swaps the package's spawn seam for a recorder, restoring it on
+// cleanup. The fake never spawns anything: "success" fabricates cmd.Process
+// from our own PID so the PID-move path has a real number to write.
+func fakeStart(t *testing.T, fail func(cmd *exec.Cmd) error) *[]*exec.Cmd {
+ t.Helper()
+ var calls []*exec.Cmd
+ orig := startCmd
+ startCmd = func(cmd *exec.Cmd) error {
+ calls = append(calls, cmd)
+ if err := fail(cmd); err != nil {
+ return err
+ }
+ cmd.Process, _ = os.FindProcess(os.Getpid())
+ return nil
+ }
+ t.Cleanup(func() { startCmd = orig })
+ return &calls
+}
+
+func buildTrue() *exec.Cmd { return exec.Command("/bin/true") }
+
+// TestStartNilManagerSpawnsPlainly: the nil manager is the no-delegation
+// state — one plain start, no SysProcAttr, no filesystem writes.
+func TestStartNilManagerSpawnsPlainly(t *testing.T) {
+ calls := fakeStart(t, func(*exec.Cmd) error { return nil })
+ var m *Manager
+ cmd, err := m.Start("h.example.com", buildTrue)
+ if err != nil || cmd == nil {
+ t.Fatalf("Start = (%v, %v), want started cmd", cmd, err)
+ }
+ if len(*calls) != 1 || (*calls)[0].SysProcAttr != nil {
+ t.Fatalf("nil manager must start exactly once with no SysProcAttr: %d calls", len(*calls))
+ }
+}
+
+// TestStartGenuineSpawnFailureDoesNotLatch: when the retry-without-cgroup
+// ALSO fails, the cgroup was not the problem — the error propagates and the
+// fallback must not latch (the next spawn should still try clone3).
+func TestStartGenuineSpawnFailureDoesNotLatch(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ fakeStart(t, func(*exec.Cmd) error { return errors.New("no such binary") })
+ if _, err := m.Start("h.example.com", buildTrue); err == nil {
+ t.Fatal("Start with a failing spawn succeeded, want error")
+ }
+ if m.fallback.Load() {
+ t.Fatal("a genuine spawn failure latched the clone3 fallback")
+ }
+}
+
+// TestCleanupHost: removes an empty per-host dir, tolerates a non-empty
+// one, and is nil-safe.
+func TestCleanupHost(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ empty := filepath.Join(root, ExecName("gone.example.com"))
+ if err := os.Mkdir(empty, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ m.CleanupHost("gone.example.com")
+ if _, err := os.Stat(empty); !os.IsNotExist(err) {
+ t.Errorf("empty host dir not removed: %v", err)
+ }
+
+ busy := filepath.Join(root, ExecName("busy.example.com"))
+ if err := os.MkdirAll(filepath.Join(busy, "child"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ m.CleanupHost("busy.example.com") // must not panic or force-remove
+ if _, err := os.Stat(busy); err != nil {
+ t.Errorf("non-empty host dir must survive: %v", err)
+ }
+
+ var nilM *Manager
+ nilM.CleanupHost("h.example.com") // must not panic
+}
+```
+
+Create `internal/cgroup/cgroup_linux_test.go` — the clone3 paths only
+exist where `sysProcAttrInto` builds a real attr:
+
+```go
+//go:build linux
+
+package cgroup
+
+import (
+ "os"
+ "os/exec"
+ "path/filepath"
+ "syscall"
+ "testing"
+)
+
+// TestStartUsesCgroupFDFirst: with delegation live and no latch, the child
+// starts via clone3-into-cgroup — SysProcAttr set, host dir created, and NO
+// cgroup.procs write (the kernel placed it, no PID move needed).
+func TestStartUsesCgroupFDFirst(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ calls := fakeStart(t, func(*exec.Cmd) error { return nil })
+ if _, err := m.Start("h.example.com", buildTrue); err != nil {
+ t.Fatal(err)
+ }
+ if len(*calls) != 1 {
+ t.Fatalf("started %d times, want 1", len(*calls))
+ }
+ attr := (*calls)[0].SysProcAttr
+ if attr == nil || !attr.UseCgroupFD {
+ t.Fatalf("SysProcAttr = %+v, want UseCgroupFD", attr)
+ }
+ dir := filepath.Join(root, ExecName("h.example.com"))
+ if _, err := os.Stat(dir); err != nil {
+ t.Fatalf("host cgroup dir not created: %v", err)
+ }
+ if _, err := os.Stat(filepath.Join(dir, "cgroup.procs")); !os.IsNotExist(err) {
+ t.Errorf("clone3 path must not write cgroup.procs (the kernel places the child): %v", err)
+ }
+}
+
+// TestStartFallsBackAndLatches pins the refusal path: clone3 refused
+// (seccomp EPERM), the retry WITHOUT the cgroup fd succeeds — proving the
+// cgroup was the problem — the child's PID is moved by file write, and the
+// latch makes every later spawn skip clone3 entirely.
+func TestStartFallsBackAndLatches(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ calls := fakeStart(t, func(cmd *exec.Cmd) error {
+ if cmd.SysProcAttr != nil {
+ return syscall.EPERM // the filter refusing clone3
+ }
+ return nil
+ })
+ if _, err := m.Start("h.example.com", buildTrue); err != nil {
+ t.Fatal(err)
+ }
+ if len(*calls) != 2 {
+ t.Fatalf("first spawn: %d start calls, want 2 (clone3 attempt + plain retry)", len(*calls))
+ }
+ procs := filepath.Join(root, ExecName("h.example.com"), "cgroup.procs")
+ if _, err := os.Stat(procs); err != nil {
+ t.Fatalf("fallback did not move the PID: %v", err)
+ }
+ if !m.fallback.Load() {
+ t.Fatal("refusal did not latch the fallback")
+ }
+
+ if _, err := m.Start("h2.example.com", buildTrue); err != nil {
+ t.Fatal(err)
+ }
+ if len(*calls) != 3 {
+ t.Fatalf("latched spawn: %d total start calls, want 3 (no clone3 re-attempt)", len(*calls))
+ }
+ if (*calls)[2].SysProcAttr != nil {
+ t.Fatal("latched spawn still set SysProcAttr")
+ }
+}
+```
+
+- [ ] **Step 2: Run tests to verify they fail**
+
+Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/cgroup/ -count=1`
+Expected: FAIL — `startCmd`, `Start`, `CleanupHost` undefined.
+
+- [ ] **Step 3: Write the implementation**
+
+Create `internal/cgroup/cgroup_linux.go`:
+
+```go
+//go:build linux
+
+package cgroup
+
+import "syscall"
+
+// intoCgroupSupported gates the clone3 CLONE_INTO_CGROUP attempt to
+// platforms whose SysProcAttr has the fields — everywhere else Start goes
+// straight to the PID-move fallback.
+const intoCgroupSupported = true
+
+func sysProcAttrInto(fd int) *syscall.SysProcAttr {
+ return &syscall.SysProcAttr{UseCgroupFD: true, CgroupFD: fd}
+}
+```
+
+Create `internal/cgroup/cgroup_other.go`:
+
+```go
+//go:build !linux
+
+package cgroup
+
+import "syscall"
+
+const intoCgroupSupported = false
+
+func sysProcAttrInto(fd int) *syscall.SysProcAttr { return nil }
+```
+
+Append to `internal/cgroup/cgroup.go` (add `os/exec` to imports):
+
+```go
+// startCmd is Start's spawn seam — a package var so the clone3 refusal
+// path is testable without a seccomp filter in the loop.
+var startCmd = func(cmd *exec.Cmd) error { return cmd.Start() }
+
+// hostDir is the per-host cgroup: instance, sidecar and seed all share it —
+// a seed's first-boot cost and a sidecar's load ARE that instance's work.
+func (m *Manager) hostDir(host string) string { return filepath.Join(m.root, ExecName(host)) }
+
+// Start runs build() and starts the result inside host's cgroup. Nil-safe:
+// with no manager (or any accounting failure) the child starts exactly as
+// today — a spawn must never fail because accounting failed. build must
+// return a fully configured, unstarted Cmd; it may be called twice, because
+// a Cmd cannot be reused after a failed Start. On a non-nil error the
+// returned Cmd may be nil.
+//
+// Primary mechanism: clone3 with CLONE_INTO_CGROUP (SysProcAttr.UseCgroupFD)
+// — the child never spends a cycle in the wrong cgroup. If that start
+// fails, the cgroup fd may or may not be why (a seccomp filter older than
+// clone3 answers EPERM; so does a missing binary): retry WITHOUT the fd,
+// and only if the retry succeeds — proving the cgroup was the problem —
+// latch the fallback and move the child by PID. The window between start
+// and move charges the child's first moments to the agent: mis-accounting
+// in the safe direction.
+func (m *Manager) Start(host string, build func() *exec.Cmd) (*exec.Cmd, error) {
+ if m == nil {
+ cmd := build()
+ return cmd, startCmd(cmd)
+ }
+ dir := m.hostDir(host)
+ if err := os.MkdirAll(dir, 0o755); err != nil {
+ m.logf("cgroup: %s: create cgroup: %v — spawning unaccounted", host, err)
+ cmd := build()
+ return cmd, startCmd(cmd)
+ }
+ if intoCgroupSupported && !m.fallback.Load() {
+ f, err := os.Open(dir)
+ if err != nil {
+ m.logf("cgroup: %s: open cgroup dir: %v — using PID-move fallback for this spawn", host, err)
+ return m.startAndMove(host, build)
+ }
+ cmd := build()
+ cmd.SysProcAttr = sysProcAttrInto(int(f.Fd()))
+ err = startCmd(cmd)
+ f.Close()
+ if err == nil {
+ return cmd, nil
+ }
+ firstErr := err
+ retried, retryErr := m.startAndMove(host, build)
+ if retryErr != nil {
+ return nil, retryErr // failed both ways: not the cgroup's fault
+ }
+ m.fallback.Store(true)
+ m.logf("cgroup: clone3-into-cgroup refused (%v) — latching PID-move fallback for the life of this process", firstErr)
+ return retried, nil
+ }
+ return m.startAndMove(host, build)
+}
+
+// startAndMove is the fallback spawn: plain start, then write the child's
+// PID into the host cgroup. A failed move is logged and tolerated — the
+// child runs (unaccounted), which beats not running.
+func (m *Manager) startAndMove(host string, build func() *exec.Cmd) (*exec.Cmd, error) {
+ cmd := build()
+ if err := startCmd(cmd); err != nil {
+ return nil, err
+ }
+ procs := filepath.Join(m.hostDir(host), "cgroup.procs")
+ if err := os.WriteFile(procs, []byte(strconv.Itoa(cmd.Process.Pid)), 0o644); err != nil {
+ m.logf("cgroup: %s: move pid %d: %v — child stays unaccounted", host, cmd.Process.Pid, err)
+ }
+ return cmd, nil
+}
+
+// CleanupHost removes host's per-host cgroup dir after its children exit.
+// Nil-safe and rmdir-tolerant: while a sibling (instance vs sidecar) is
+// still alive the dir is non-empty and the remove fails — the LAST exiter's
+// call succeeds, and the next agent start's sweep catches anything missed.
+func (m *Manager) CleanupHost(host string) {
+ if m == nil {
+ return
+ }
+ _ = os.Remove(m.hostDir(host))
+}
+```
+
+- [ ] **Step 4: Run tests to verify they pass**
+
+Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/cgroup/ -count=1`
+Expected: PASS
+
+- [ ] **Step 5: gofmt + vet, then commit**
+
+Run: `gofmt -l internal/cgroup/ && GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go vet ./internal/cgroup/`
+
+```bash
+git add internal/cgroup/
+git commit -m "cgroup: spawn into per-host cgroups — clone3 first, latched PID-move fallback
+
+Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
+```
+
+---
+
+### Task 4: activator — spawn every exec child through the manager
+
+**Files:**
+- Modify: `internal/activator/backend_unit.go` (add `ExecName` next to `UnitName`, line ~66)
+- Modify: `internal/activator/backend_exec.go` (ExecBackend field + Start/StartSidecar/Seed paths)
+
+**Interfaces:**
+- Consumes: `cgroup.Manager.Start(host, build)`, `cgroup.Manager.CleanupHost(host)`,
+ `cgroup.ExecName(host)` from Tasks 1–3.
+- Produces: `activator.ExecName(host string) string` (Task 7 uses it);
+ `ExecBackend.Cgroups *cgroup.Manager` field (Task 6 wires it).
+
+- [ ] **Step 1: Add `ExecName` next to `UnitName`**
+
+In `internal/activator/backend_unit.go`, directly after the existing
+`UnitName` (line ~66), add (and import
+`github.com/carlosframework/platform/internal/cgroup`):
+
+```go
+// ExecName is the metrics sample key for an exec-backed tenant's children —
+// the delegated cgroup directory the activator spawns them into. Defined by
+// internal/cgroup (which creates the directories); surfaced here next to
+// UnitName so the console learns both of a host's legal metric keys from
+// the same place.
+func ExecName(host string) string { return cgroup.ExecName(host) }
+```
+
+- [ ] **Step 2: Add the manager field to ExecBackend**
+
+In `internal/activator/backend_exec.go`, add to the `ExecBackend` struct
+(after the `Cache` field, line ~58), and add the `cgroup` import:
+
+```go
+ // Cgroups, when non-nil, places every spawned child — instance, sidecar,
+ // seed — in a per-host cgroup under the agent's delegated subtree so the
+ // metrics sampler can account it (exec-child accounting spec). Nil — no
+ // delegation: old unit file, tests, darwin — spawns exactly as before.
+ Cgroups *cgroup.Manager
+```
+
+- [ ] **Step 3: Route the instance spawn through the manager**
+
+In `ExecBackend.Start` (line ~440), replace this block:
+
+```go
+ cmd := exec.Command(bin, "--socket", rt.Socket, "--db", dbPath)
+ cmd.Env = childEnv(childVars)
+ // The child logs through the agent's own stderr/stdout (journald under
+ // systemd); there's no per-instance unit to own a log stream anymore.
+ cmd.Stdout = os.Stdout
+ cmd.Stderr = os.Stderr
+ // Record the build we're about to exec — read the symlink immediately before
+ // Start() so it names the version the kernel will resolve. Reading it here,
+ // rather than back in wake() after the health wait, closes a real window: a
+ // concurrent ship's repoint during the up-to-30s wait would otherwise make
+ // us record the NEW target while this child runs the OLD binary, and the
+ // sweep would then think it's already converged and never cycle it. The
+ // residual sub-exec race (a repoint between this read and Start) is in the
+ // safe direction — we'd record the older version, so the sweep cycles once
+ // too often rather than never. Empty on a read error: the sweep won't cycle.
+ spawnVer, _ := b.LiveVersion(host)
+ if err := cmd.Start(); err != nil {
+ return nil, "", err
+ }
+```
+
+with:
+
+```go
+ // Record the build we're about to exec — read the symlink immediately before
+ // the start so it names the version the kernel will resolve. Reading it here,
+ // rather than back in wake() after the health wait, closes a real window: a
+ // concurrent ship's repoint during the up-to-30s wait would otherwise make
+ // us record the NEW target while this child runs the OLD binary, and the
+ // sweep would then think it's already converged and never cycle it. The
+ // residual sub-exec race (a repoint between this read and the start) is in
+ // the safe direction — we'd record the older version, so the sweep cycles
+ // once too often rather than never. Empty on a read error: the sweep won't
+ // cycle.
+ spawnVer, _ := b.LiveVersion(host)
+ // Spawned through the cgroup manager (nil-tolerant: no delegation means
+ // a plain start, exactly as before) so the child lands in
+ // carlos-exec@<host> and the sampler can account it. build may run
+ // twice — the manager's clone3-refusal fallback — so everything about
+ // the Cmd is configured inside it.
+ cmd, err := b.Cgroups.Start(host, func() *exec.Cmd {
+ cmd := exec.Command(bin, "--socket", rt.Socket, "--db", dbPath)
+ cmd.Env = childEnv(childVars)
+ // The child logs through the agent's own stderr/stdout (journald
+ // under systemd); there's no per-instance unit to own a log stream
+ // anymore.
+ cmd.Stdout = os.Stdout
+ cmd.Stderr = os.Stderr
+ return cmd
+ })
+ if err != nil {
+ return nil, "", err
+ }
+```
+
+Then in the wait goroutine a few lines below, add the cleanup:
+
+```go
+ go func() {
+ p.err = cmd.Wait()
+ // Last exiter wins: rmdir refuses while the sidecar (same host
+ // cgroup) is still alive, and that failure is tolerated.
+ b.Cgroups.CleanupHost(host)
+ close(p.done)
+ }()
+```
+
+- [ ] **Step 4: Route the sidecar spawn through the manager**
+
+In `StartSidecar` (line ~602), replace:
+
+```go
+ cmd := exec.Command(b.liveLink(rt.Host), "sidecar", "run")
+ cmd.Env = childEnv(fileEnv)
+ // Like the instance child: logs through the agent's own streams (journald
+ // under systemd) — no per-host unit owns a log stream anymore.
+ cmd.Stdout = os.Stdout
+ cmd.Stderr = os.Stderr
+ if err := cmd.Start(); err != nil {
+ return nil, err
+ }
+ p := &childProc{cmd: cmd, done: make(chan struct{})}
+ go func() {
+ p.err = cmd.Wait()
+ close(p.done)
+ }()
+```
+
+with:
+
+```go
+ // Same host cgroup as the instance: a sidecar's load is that instance's
+ // work, and attributing it elsewhere would be a lie in the other
+ // direction.
+ cmd, err := b.Cgroups.Start(rt.Host, func() *exec.Cmd {
+ cmd := exec.Command(b.liveLink(rt.Host), "sidecar", "run")
+ cmd.Env = childEnv(fileEnv)
+ // Like the instance child: logs through the agent's own streams
+ // (journald under systemd) — no per-host unit owns a log stream
+ // anymore.
+ cmd.Stdout = os.Stdout
+ cmd.Stderr = os.Stderr
+ return cmd
+ })
+ if err != nil {
+ return nil, err
+ }
+ p := &childProc{cmd: cmd, done: make(chan struct{})}
+ go func() {
+ p.err = cmd.Wait()
+ b.Cgroups.CleanupHost(rt.Host)
+ close(p.done)
+ }()
+```
+
+- [ ] **Step 5: Route the seed through the manager**
+
+In `Seed` (line ~213), replace the block from `ctx, cancel :=
+context.WithTimeout(ctx, seedTimeout)` through `if err := cmd.Run(); err !=
+nil {` — keeping the whole error-handling body — with:
+
+```go
+ ctx, cancel := context.WithTimeout(ctx, seedTimeout)
+ defer cancel()
+ argv := append(seedArgs, "--db", dbPath)
+ var stderr capBuffer
+ log.Printf("activator: seeding %s: %s %s", host, b.liveLink(host), strings.Join(argv, " "))
+ // The seed shares the instance's carlos-exec@<host> cgroup: first-boot
+ // cost is that instance's work. No CleanupHost here — the instance spawn
+ // reuses the dir moments later, and its exit path (or the next agent
+ // start's sweep) removes it.
+ cmd, err := b.Cgroups.Start(host, func() *exec.Cmd {
+ cmd := exec.CommandContext(ctx, b.liveLink(host), argv...)
+ // The purge-channel vars are agent-owned in BOTH directions here too
+ // (Start's discipline, see withoutVars): fileEnv is tenant-supplied, so
+ // without this a tenant's own env file could hand its seed a
+ // CARLOS_CONTROL_SOCKET the agent never opened and a CARLOS_PURGE for a
+ // channel that does not exist — and the seed has no serving cache to
+ // purge in any case. This is what makes the contract above true rather
+ // than merely intended.
+ cmd.Env = childEnv(withoutVars(fileEnv, "CARLOS_CONTROL_SOCKET", "CARLOS_PURGE"))
+ // Like the instance child, the seed logs through the agent's own streams
+ // (journald under systemd) — and its stderr is ALSO teed into a bounded
+ // buffer, because the wake error this returns is the whole diagnosis an
+ // operator gets for a host that refuses to come up.
+ cmd.Stdout = os.Stdout
+ cmd.Stderr = io.MultiWriter(os.Stderr, &stderr)
+ // WaitDelay is what makes seedTimeout real. Because Stderr is a plain
+ // io.Writer (the tee), os/exec pipes it and Wait waits for that pipe to
+ // close — and a seed that spawned its own grandchild holds the write end
+ // open long after the timeout killed the seed itself. Without this, a
+ // wedged seed would hold the lease until the wake's own 5m budget expired.
+ cmd.WaitDelay = seedDrainGrace
+ return cmd
+ })
+ if err == nil {
+ err = cmd.Wait()
+ }
+ if err != nil {
+```
+
+Inside the retained error-handling body, the `ErrWaitDelay` success check
+must guard against a nil cmd (a spawn that never started has no
+ProcessState):
+
+```go
+ if cmd != nil && errors.Is(err, exec.ErrWaitDelay) && cmd.ProcessState != nil && cmd.ProcessState.Success() {
+```
+
+Everything else in the body (`DeadlineExceeded` wrap, `seedOwnsDB` cleanup,
+stderr tail) stays byte-identical. Note the original `log.Printf("activator:
+seeding %s: ...")` line moved ABOVE the spawn (it was between cmd
+construction and Run; keep exactly one copy). Also delete the now-orphaned
+comment lines that referred to `cmd.Run` if any remain outside the closure.
+
+- [ ] **Step 6: Build, run the activator suite**
+
+Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && go vet ./internal/activator/`
+Then (sandbox disabled — the suite binds unix sockets):
+`GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/activator/ -count=1`
+Expected: PASS — every existing test runs with `Cgroups` nil, pinning that
+the nil manager is behavior-identical to today.
+
+- [ ] **Step 7: gofmt, commit**
+
+Run: `gofmt -l internal/activator/`
+
+```bash
+git add internal/activator/backend_unit.go internal/activator/backend_exec.go
+git commit -m "activator: instance, sidecar and seed spawn into the host's exec cgroup
+
+Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
+```
+
+---
+
+### Task 5: host sampler — the third scan root
+
+**Files:**
+- Modify: `internal/host/metrics_probes.go` (add `listExecCgroups`)
+- Modify: `internal/host/metrics_probes_test.go` (add discovery test)
+- Modify: `internal/host/metrics_loop.go` (`realUnitReadings`,
+ `realSampleProbes`, `metricsLoop` gain the exec root)
+- Modify: `internal/host/agent.go:164` (call-site signature only; real wiring is Task 6)
+
+**Interfaces:**
+- Consumes: `cgroup.IsExecName` from Task 1.
+- Produces: `metricsLoop(execRoot string)` — Task 6 passes the manager's
+ root. Sample keys `carlos-exec@<host>` flow into `metrics.Sample.Units`
+ verbatim (additive wire change; Task 7 consumes them in the console).
+
+- [ ] **Step 1: Write the failing test**
+
+Append to `internal/host/metrics_probes_test.go`:
+
+```go
+// TestListExecCgroupsFindsPerHostDirs pins the delegated subtree's scan:
+// carlos-exec@<host> dirs come out with their names VERBATIM as sample keys
+// (deliberately not dressed up as .service units — an operator who takes a
+// dashboard row to systemctl must not be told a unit exists that doesn't).
+// The agent's own supervise leaf and foreign names are invisible, and an
+// empty root — the no-delegation state — scans nothing.
+func TestListExecCgroupsFindsPerHostDirs(t *testing.T) {
+ root := t.TempDir()
+ mk := func(name string) string {
+ t.Helper()
+ dir := filepath.Join(root, name)
+ if err := os.MkdirAll(dir, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ return dir
+ }
+ execDir := mk("carlos-exec@keymail.dev")
+ mk("supervise")
+ mk("carlos-exec@") // no host — not one of ours
+ mk("random-dir")
+
+ got := listExecCgroups(root)
+ if len(got) != 1 {
+ t.Fatalf("discovered %d cgroups, want 1: %+v", len(got), got)
+ }
+ if got[0].name != "carlos-exec@keymail.dev" || got[0].dir != execDir {
+ t.Fatalf("got %+v, want name carlos-exec@keymail.dev dir %s", got[0], execDir)
+ }
+
+ if out := listExecCgroups(""); out != nil {
+ t.Fatalf("empty root must scan nothing, got %+v", out)
+ }
+}
+```
+
+- [ ] **Step 2: Run test to verify it fails**
+
+Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/host/ -run TestListExecCgroups -count=1`
+Expected: FAIL — `listExecCgroups` undefined.
+
+- [ ] **Step 3: Implement the scan**
+
+Append to `internal/host/metrics_probes.go` (add import
+`github.com/carlosframework/platform/internal/cgroup`):
+
+```go
+// listExecCgroups lists the per-host exec-child cgroups under the agent's
+// delegated subtree — the third scan root, alongside listCarlosUnits' two
+// levels. An empty root (no delegation armed) or a read failure -> nil. The
+// agent's own supervise leaf fails IsExecName and is skipped: that process
+// is already reported as carlos-edge.service. readUnitRaw reads these dirs
+// unchanged — same cgroup v2 files, same fail-toward-nil fields.
+func listExecCgroups(root string) []unitCgroup {
+ if root == "" {
+ return nil
+ }
+ entries, err := os.ReadDir(root)
+ if err != nil {
+ return nil
+ }
+ var units []unitCgroup
+ for _, entry := range entries {
+ if !entry.IsDir() || !cgroup.IsExecName(entry.Name()) {
+ continue
+ }
+ units = append(units, unitCgroup{name: entry.Name(), dir: filepath.Join(root, entry.Name())})
+ }
+ return units
+}
+```
+
+- [ ] **Step 4: Run test to verify it passes**
+
+Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/host/ -run TestListExecCgroups -count=1`
+Expected: PASS
+
+- [ ] **Step 5: Thread the exec root through the loop**
+
+In `internal/host/metrics_loop.go` (all three are thin glue, untested by
+the package's convention — the walk itself was tested in Step 1):
+
+`realSampleProbes` (line ~621) gains the root:
+
+```go
+func realSampleProbes(execRoot string) sampleProbes {
+ return sampleProbes{
+ procStat: readProcStat,
+ loadavg: readLoadAvg,
+ memFrac: readMemFrac,
+ diskFrac: readDiskFrac,
+ units: func() map[string]unitRaw { return realUnitReadings(execRoot) },
+ now: time.Now,
+ }
+}
+```
+
+`realUnitReadings` (line ~637) merges both discovery walks — update its doc
+comment's last sentence and the body:
+
+```go
+// realUnitReadings lists the box's currently active CARLOS units and reads
+// each one's cgroup usage — the production units func sampleProbes.units
+// wraps. Keyed by UNIT NAME (what the console matches against), read from the
+// unit's own cgroup DIRECTORY (which for a tenant instance is a level down;
+// see listCarlosUnits) — plus, when delegation is armed, the per-host
+// exec-child cgroups under execRoot (keyed carlos-exec@<host>; see
+// listExecCgroups).
+func realUnitReadings(execRoot string) map[string]unitRaw {
+ units := listCarlosUnits(cgroupSliceDir)
+ units = append(units, listExecCgroups(execRoot)...)
+ if len(units) == 0 {
+ return nil
+ }
+ out := make(map[string]unitRaw, len(units))
+ for _, u := range units {
+ out[u.name] = readUnitRaw(u.dir)
+ }
+ return out
+}
+```
+
+`metricsLoop` (line ~702) becomes `func metricsLoop(execRoot string)` and
+its `probes := realSampleProbes()` line becomes
+`probes := realSampleProbes(execRoot)`.
+
+In `internal/host/agent.go` line 164, change `go metricsLoop()` to
+`go metricsLoop("")` — the real root arrives in Task 6.
+
+- [ ] **Step 6: Build + full host suite (sandbox disabled), commit**
+
+Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && go vet ./internal/host/ && go test ./internal/host/ -count=1`
+Expected: PASS. Then `gofmt -l internal/host/`, and:
+
+```bash
+git add internal/host/metrics_probes.go internal/host/metrics_probes_test.go internal/host/metrics_loop.go internal/host/agent.go
+git commit -m "host: the sampler scans the delegated exec subtree as a third root
+
+Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
+```
+
+---
+
+### Task 6: agent wiring + the unit file's delegation
+
+**Files:**
+- Modify: `internal/host/agent.go` (RunAgent: Setup + wire manager)
+- Modify: `internal/host/units/carlos-edge.service`
+
+**Interfaces:**
+- Consumes: `cgroup.Setup`, `Manager.Root()` (Task 2),
+ `ExecBackend.Cgroups` (Task 4), `metricsLoop(execRoot)` (Task 5).
+- Produces: a fully armed production path. No new API.
+
+- [ ] **Step 1: Wire the manager in RunAgent**
+
+In `internal/host/agent.go`, add the import
+`github.com/carlosframework/platform/internal/cgroup`, then in `RunAgent`
+insert after the `cache := edge.NewCache(...)` line (~132):
+
+```go
+ // Exec-child accounting: claim the cgroup subtree the unit file
+ // delegates to us (Delegate= in carlos-edge.service). Nil when there is
+ // no delegation — an old unit file, a dev run outside systemd, darwin —
+ // in which case children spawn exactly as before and simply have no
+ // per-instance readings.
+ cgm := cgroup.Setup(log.Printf)
+```
+
+In the `activator.ExecBackend` literal (line ~146), add the field:
+
+```go
+ Exec: &activator.ExecBackend{
+ Cfg: *cfg,
+ OptDir: release.OptDir(),
+ DataDir: dd,
+ Cache: cache,
+ Cgroups: cgm,
+ },
+```
+
+And change the Task-5 placeholder `go metricsLoop("")` (line ~164) to:
+
+```go
+ go metricsLoop(cgm.Root())
+```
+
+- [ ] **Step 2: Delegate the subtree in the unit file**
+
+In `internal/host/units/carlos-edge.service`, two edits.
+
+First, replace the line `ProtectControlGroups=yes` with:
+
+```ini
+# ProtectControlGroups is deliberately ABSENT: it would mount /sys/fs/cgroup
+# read-only, and Delegate= (below) needs this unit's own subtree writable.
+# The hardening that line bought is retained by ownership instead — plain
+# DAC keeps everything OUTSIDE the chowned subtree read-only to this
+# unprivileged user.
+```
+
+Second, directly after the `TasksMax=1024` line, add:
+
+```ini
+# Exec-backed instances run as this unit's children. Delegation hands the
+# unit its own cgroup subtree — chowned to User=carlos, unmanaged by systemd
+# below this point — so the agent can give each child its own cgroup for
+# per-instance accounting (docs/superpowers/specs/
+# 2026-08-07-exec-child-accounting-design.md). The subtree still dies with
+# the service: restart semantics are unchanged.
+Delegate=cpu memory io pids
+```
+
+- [ ] **Step 3: Full gates**
+
+Run (sandbox disabled for the test step):
+
+```bash
+GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && \
+GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go vet ./... && \
+GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/host/ ./internal/activator/ ./internal/cgroup/ -count=1
+```
+
+Expected: all PASS. (`cgroup.Setup` in a test/dev process finds
+`/sys/fs/cgroup/...` unwritable and returns nil — the degradation path is
+what makes this safe to run anywhere.)
+
+- [ ] **Step 4: Commit**
+
+```bash
+git add internal/host/agent.go internal/host/units/carlos-edge.service
+git commit -m "host: the agent claims its delegated subtree; the unit file delegates it
+
+Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
+```
+
+---
+
+### Task 7: console — the second allowlist key
+
+**Files:**
+- Modify: `internal/console/metricsview.go:656-666` (OwnerAppMetrics allowlist + doc comment; also the stale comment at line ~630)
+- Modify: `internal/console/metricsview_test.go` (new test)
+
+**Interfaces:**
+- Consumes: `activator.ExecName(host)` from Task 4.
+- Produces: nothing new — exec-backed instances flow through the existing
+ row assembly (`Reporting()` turns true, sparklines render).
+
+- [ ] **Step 1: Write the failing test**
+
+Append to `internal/console/metricsview_test.go` (it already imports
+`metrics` and has `newMetricsFixture`, `p`, `pi64`):
+
+```go
+// TestOwnerAppMetricsSeesExecBackedInstances pins the second allowlist key:
+// an exec-backed host reports under carlos-exec@<host> (the delegated
+// cgroup's verbatim name, no .service), and its readings reach the owner
+// row exactly like a unit-backed tenant's. Another tenant's exec cgroup
+// stays invisible — the allowlist boundary holds for the new key form too.
+func TestOwnerAppMetricsSeesExecBackedInstances(t *testing.T) {
+ srv, rel := newMetricsFixture(t)
+ ctx := context.Background()
+ at := time.Now().UTC().Add(-time.Minute)
+ if err := metrics.WriteRaw(ctx, rel, metrics.RawChunk{
+ V: 1, Box: "box-a", Hour: metrics.HourOf(at),
+ Samples: []metrics.Sample{{
+ At: at,
+ Units: map[string]metrics.UnitSample{
+ "carlos-exec@keymail.example.com": {CPUFrac: p(0.22), MemBytes: pi64(2048)},
+ "carlos-exec@other.example.com": {CPUFrac: p(0.44), MemBytes: pi64(4096)},
+ },
+ }},
+ }); err != nil {
+ t.Fatal(err)
+ }
+
+ rows, err := srv.OwnerAppMetrics(ctx, []string{"keymail.example.com"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(rows) != 1 {
+ t.Fatalf("rows = %+v, want exactly the one allowlisted host", rows)
+ }
+ got := rows[0]
+ if got.Host != "keymail.example.com" || !got.Reporting() {
+ t.Fatalf("exec-backed host not reporting: %+v", got)
+ }
+ if got.CPUFrac == nil || *got.CPUFrac != 0.22 {
+ t.Fatalf("CPU = %v, want 0.22", got.CPUFrac)
+ }
+ if got.MemBytes == nil || *got.MemBytes != 2048 {
+ t.Fatalf("mem = %v, want 2048", got.MemBytes)
+ }
+ if got.SparkCPU == "" || got.SparkMem == "" {
+ t.Fatalf("sparklines empty: %+v", got)
+ }
+ if got.CPUFrac != nil && *got.CPUFrac == 0.44 {
+ t.Fatalf("row carries another tenant's exec reading: %+v", got)
+ }
+}
+
+// TestOwnerAppMetricsExecAndUnitKeysNeverDoubleReport pins the spec's
+// no-double-report property: a host can't legally report under both key
+// forms at once (one route, one backing), but if a sweep ever carries both
+// — a backing flip mid-window, two boxes — the existing freshest-box rule
+// picks ONE deterministically. No special case, still exactly one row.
+func TestOwnerAppMetricsExecAndUnitKeysNeverDoubleReport(t *testing.T) {
+ srv, rel := newMetricsFixture(t)
+ ctx := context.Background()
+ fresher := time.Now().UTC().Add(-time.Minute)
+ older := fresher.Add(-2 * time.Minute)
+ // The older box reports the host as a systemd unit, the fresher box as
+ // an exec cgroup — the flip-mid-window shape.
+ if err := metrics.WriteRaw(ctx, rel, metrics.RawChunk{
+ V: 1, Box: "box-old", Hour: metrics.HourOf(older),
+ Samples: []metrics.Sample{{
+ At: older,
+ Units: map[string]metrics.UnitSample{
+ "carlos-app@flip.example.com.service": {CPUFrac: p(0.99), MemBytes: pi64(9999)},
+ },
+ }},
+ }); err != nil {
+ t.Fatal(err)
+ }
+ if err := metrics.WriteRaw(ctx, rel, metrics.RawChunk{
+ V: 1, Box: "box-new", Hour: metrics.HourOf(fresher),
+ Samples: []metrics.Sample{{
+ At: fresher,
+ Units: map[string]metrics.UnitSample{
+ "carlos-exec@flip.example.com": {CPUFrac: p(0.11), MemBytes: pi64(1111)},
+ },
+ }},
+ }); err != nil {
+ t.Fatal(err)
+ }
+
+ rows, err := srv.OwnerAppMetrics(ctx, []string{"flip.example.com"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(rows) != 1 {
+ t.Fatalf("rows = %+v, want exactly one row for the host, never two", rows)
+ }
+ got := rows[0]
+ if got.Box != "box-new" {
+ t.Fatalf("Box = %q, want the freshest box-new: %+v", got.Box, got)
+ }
+ if got.CPUFrac == nil || *got.CPUFrac != 0.11 {
+ t.Fatalf("CPU = %v, want the freshest box's 0.11", got.CPUFrac)
+ }
+}
+```
+
+- [ ] **Step 2: Run test to verify it fails**
+
+Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -run TestOwnerAppMetricsSeesExecBacked -count=1`
+Expected: FAIL — the row is not Reporting (key unmatched).
+
+- [ ] **Step 3: Add the second key**
+
+In `internal/console/metricsview.go`, `OwnerAppMetrics` (line ~660),
+replace:
+
+```go
+ want := make(map[string]string, len(hosts))
+ for _, h := range hosts {
+ if h == "" {
+ continue
+ }
+ want[activator.UnitName(h)] = h
+ }
+```
+
+with:
+
+```go
+ want := make(map[string]string, 2*len(hosts))
+ for _, h := range hosts {
+ if h == "" {
+ continue
+ }
+ // Two legal keys per host: the systemd unit a unit-backed tenant
+ // reports under, and the delegated exec cgroup an exec-backed one
+ // does. One route, one backing — a host can't legally report under
+ // both at once; if both ever appear in a sweep, the freshest-box
+ // rule below picks deterministically, no special case.
+ want[activator.UnitName(h)] = h
+ want[activator.ExecName(h)] = h
+ }
+```
+
+In the same function's row assembly (line ~709), the loop currently
+iterates `want`'s VALUES — which was one-per-host when each host had one
+key, but with two keys per host would render every host twice. Replace:
+
+```go
+ rows := make([]InstanceMetrics, 0, len(want))
+ for _, host := range want {
+ if row, ok := found[host]; ok {
+ rows = append(rows, row)
+ } else {
+ rows = append(rows, InstanceMetrics{Host: host})
+ }
+ }
+```
+
+with:
+
+```go
+ // Iterate the callers' hosts, not want's values: want now carries TWO
+ // keys per host, and "every allowlisted host gets exactly one row" is a
+ // promise about hosts, not keys. seen collapses duplicate input hosts,
+ // which the old unique-keyed map did implicitly.
+ rows := make([]InstanceMetrics, 0, len(hosts))
+ seen := make(map[string]bool, len(hosts))
+ for _, h := range hosts {
+ if h == "" || seen[h] {
+ continue
+ }
+ seen[h] = true
+ if row, ok := found[h]; ok {
+ rows = append(rows, row)
+ } else {
+ rows = append(rows, InstanceMetrics{Host: h})
+ }
+ }
+```
+
+(the `sort.Slice` line after it stays unchanged).
+
+Update the function's doc comment first sentence (line ~646) from
+"...EXACTLY the units carlos-app@<host>.service for the given hosts" to:
+
+```go
+// OwnerAppMetrics filters the cached 48h sweep down to EXACTLY the given
+// hosts' own per-instance readings — carlos-app@<host>.service for a
+// unit-backed tenant, carlos-exec@<host> for an exec-backed one. It returns
+// unit-level rows
+```
+
+(keep the rest of the comment unchanged).
+
+Also update the stale parenthetical in the `Hibernate` field comment (line
+~630): change "(a bespoke unit like keymail-relay.service, or an exec child
+of the agent)" to "(a bespoke unit like keymail-relay.service, or an exec
+child on a box whose agent has no delegated subtree yet)".
+
+- [ ] **Step 4: Run tests to verify they pass**
+
+Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -count=1`
+Expected: PASS (the whole console suite — the existing allowlist and
+freshness tests must not regress).
+
+- [ ] **Step 5: gofmt + full repo gates, commit**
+
+```bash
+gofmt -l internal/console/
+GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./...
+GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go vet ./...
+# full suite, sandbox disabled:
+GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./... -count=1
+git add internal/console/metricsview.go internal/console/metricsview_test.go
+git commit -m "console: owner metrics match exec-backed instances by their cgroup key
+
+Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>"
+```
+
+---
+
+## After the plan: rollout (operator steps, NOT part of task execution)
+
+Per spec §6 — binary first, unit file second, verify on the dev box before
+flagship (`annie-welding`) / tito (`betty-fossicking`) / the Pi fleet:
+
+1. Ship the new binary fleet-wide; it tolerates the old unit file (nil
+ manager) indefinitely.
+2. Per box, one-time operator step (platform unit files are not
+ auto-converged — the follow-up gap is filed in the spec, not built
+ here): install the updated `carlos-edge.service`, `systemctl
+ daemon-reload`, `systemctl restart carlos-edge`.
+3. Verify with the binary actually built (stale-binary rule): journal line
+ "exec-child accounting armed", then exec-backed readings on the
+ Instances page (keymail's three flagship routes are the acceptance
+ case). Pi fleet note: a Pi kernel booted without `cgroup_enable=memory`
+ will fail the controller-enable write and degrade to nil — check the
+ journal line before concluding anything about a Pi.
diff --git a/docs/superpowers/specs/2026-08-02-host-process-monitoring-design.md b/docs/superpowers/specs/2026-08-02-host-process-monitoring-design.md
index c1ce58c14..8acc6e523 100644
--- a/docs/superpowers/specs/2026-08-02-host-process-monitoring-design.md
+++ b/docs/superpowers/specs/2026-08-02-host-process-monitoring-design.md
@@ -46,6 +46,14 @@ cgroup v2 files, never `ps` scraping:
Discovery therefore scans the top level of `system.slice` **and** one
level down inside each `system-*.slice` child; a top-level-only scan
finds every platform unit and no tenant at all.
+ **Amended 2026-08-08 (exec-child accounting,
+ `2026-08-07-exec-child-accounting-design.md`):** a third scan root was
+ added — the subtree systemd delegates to `carlos-edge.service`, where
+ exec-backed instances (children of the agent, no unit of their own) are
+ accounted in per-host `carlos-exec@<host>` cgroups. Those keys are
+ deliberately not `.service` names: no such units exist. Member-less
+ dirs are skipped (absence is the data), and a box without delegation
+ simply has no third root.
- Per unit: CPU fraction (`cpu.stat` usage_usec delta over wall clock),
`memory.current`, `memory.peak`, and cumulative read/write bytes from
`io.stat`.
@@ -179,10 +187,13 @@ before deriving each instance's `carlos-app@<host>.service` unit name — a
route stamped for a different account is invisible here even if the app
name matches.
-Implementation note: a box whose newest sample is older than 5 minutes
-(`metricsFreshWindow`) is treated as not-reporting in this view — its
+Implementation note: a box whose newest sample is older than
+`metricsFreshWindow` is treated as not-reporting in this view — its
instances render "asleep — no recent readings" rather than presenting
-stale numbers as current. The operator dashboard, by contrast, shows the
+stale numbers as current. (Originally 5 minutes; re-derived 2026-08-07 as
+8 minutes — flush-cadence floor + the stale-while-revalidate serve
+ceiling + slack; the constant's doc comment in
+`internal/console/metricsview.go` carries the full derivation.) The operator dashboard, by contrast, shows the
sample's own timestamp and applies no such cutoff.
House rendering rules for both views: vendored assets only (Lucide-style
diff --git a/docs/superpowers/specs/2026-08-07-exec-child-accounting-design.md b/docs/superpowers/specs/2026-08-07-exec-child-accounting-design.md
new file mode 100644
index 000000000..8397325a1
--- /dev/null
+++ b/docs/superpowers/specs/2026-08-07-exec-child-accounting-design.md
@@ -0,0 +1,170 @@
+# Exec-child accounting — design
+
+Per-instance resource readings for exec-backed routes. Today an instance
+route with `backing=''` runs as an exec child of the agent, inside
+`carlos-edge.service`'s own cgroup, and the metrics sampler
+(`2026-08-02-host-process-monitoring-design.md`) — which reads only
+`carlos-*.service` cgroups under `system.slice` — can never see it. The
+owner Instances page renders "no per-instance readings" for every such
+route, permanently. This is the default: a plain `carlos add` produces an
+exec-backed route, so the blind spot covers most tenants (diagnosed
+2026-08-06 on keymail's three flagship routes; `home.kass.training`,
+unit-backed, reports fine).
+
+Approach, chosen 2026-08-07: give each exec child its **own cgroup** under a
+subtree systemd delegates to the agent, and extend the existing sampler to
+read it. The monitoring spec's convention — a process means a cgroup, never
+process-table scraping — is kept, the numbers are the same
+`cpu.stat`/`memory.current`/`io.stat` figures unit-backed tenants get, and
+grandchildren are covered for free. `/proc` sampling of tracked PIDs was
+considered and rejected: it breaks that convention, RSS is not comparable to
+`memory.current`, and CPU spent by grandchildren reaped between ticks is
+simply lost. Migrating routes to `backing=unit` was rejected as the fix
+because it patches one app while leaving the default path blind.
+
+The governing constraint is that the agent is **not root**: `User=carlos`,
+`ProtectControlGroups=yes`, seccomp-filtered. That rules out asking PID 1
+for transient scopes over D-Bus (unprivileged callers can't), and it means
+cgroup writability must come from systemd delegation, not privilege.
+
+## 1. Unit file — delegation
+
+`internal/host/units/carlos-edge.service` changes:
+
+- Add `Delegate=cpu memory io pids`. systemd chowns the unit's own cgroup
+ subtree to `User=carlos` and stops managing below it — the agent owns
+ that directory tree and nothing else.
+- Remove `ProtectControlGroups=yes`, with a comment recording the why:
+ delegation requires the subtree writable, and ordinary DAC keeps the rest
+ of `/sys/fs/cgroup` read-only to an unprivileged user — the hardening
+ this line bought is retained by ownership rather than by mount flags.
+
+Everything else — seccomp filter, `NoNewPrivileges`, capability set,
+`TasksMax` — stays exactly as it is. A child still runs inside the service's
+subtree, so `systemctl restart carlos-edge` still kills every instance:
+today's lifecycle, unchanged.
+
+## 2. Agent startup — the delegation dance
+
+A new small package, `internal/cgroup`, wired up by the agent
+(`internal/host/agent.go`). On start:
+
+1. Read the agent's own cgroup path from `/proc/self/cgroup` and resolve it
+ under `/sys/fs/cgroup`.
+2. Probe for live delegation: the subtree's `cgroup.procs` and
+ `cgroup.subtree_control` must be writable. If not — old unit file, a
+ test, darwin — the manager is **nil**, one line is logged, and every
+ spawn behaves exactly as today. Fail toward absent readings, never
+ toward a broken spawn path.
+3. Create a `supervise/` leaf and move the agent's own process into it.
+ cgroup v2 forbids enabling controllers on a cgroup that has member
+ processes ("no internal processes"); the move is what makes step 4
+ legal.
+4. Enable `+cpu +memory +io +pids` in the subtree root's
+ `cgroup.subtree_control`, so per-child cgroups get `memory.current`,
+ `io.stat`, and `pids` accounting (`cpu.stat` exists regardless).
+5. Sweep stale `carlos-exec@*` directories left by a previous agent
+ (post-restart they are empty; rmdir, failures tolerated).
+
+Any failure mid-dance degrades to the nil manager — never a fatal error.
+
+## 3. Spawning into per-host cgroups
+
+`activator.ExecBackend` gains the manager (nil-tolerant). Instance,
+sidecar, and **seed** processes for a host all spawn into one cgroup,
+`<subtree>/carlos-exec@<host>`, created on the host's first spawn of a wake
+— a seed's first-boot cost and a sidecar's load are that instance's work,
+and attributing them elsewhere would be a lie in the other direction.
+
+- Primary mechanism: Go's `SysProcAttr{UseCgroupFD, CgroupFD}` — clone3
+ with `CLONE_INTO_CGROUP`, so the child never spends a cycle in the wrong
+ cgroup.
+- Fallback: if clone3 is refused (a seccomp filter older than the syscall;
+ `SystemCallErrorNumber=EPERM` makes this an EPERM at spawn), start the
+ child normally and write its PID into the cgroup's `cgroup.procs`. The
+ window between start and move charges the child's first moments to the
+ agent — mis-accounting in the safe direction. The first clone3 refusal
+ latches the fallback for the life of the process: a seccomp filter is
+ static, so retrying the syscall on every spawn buys nothing.
+- Cleanup: after the instance exits, rmdir the (empty) cgroup dir; a
+ failure is tolerated and the startup sweep (§2.5) catches leftovers.
+
+## 4. Sampler — a third scan root
+
+`listCarlosUnits` (`internal/host/metrics_probes.go`) currently scans
+`system.slice` flat plus one level inside each `system-*.slice`. It gains a
+third root, passed in by the agent: the delegated subtree, matching
+directories named `carlos-exec@<host>` (the `supervise` leaf is skipped —
+it is the agent itself, already reported as `carlos-edge.service`).
+
+Sample keys are the directory name, `carlos-exec@<host>`, verbatim. The
+name is deliberately **not** a fake `.service`: an operator who takes a
+dashboard row to `systemctl status` must not be told a unit exists that
+doesn't. `readUnitRaw` is reused unchanged.
+
+`carlos-edge.service`'s own row keeps its current meaning: the hierarchical
+total, children included — which is what it already reports today, since
+the children have always lived inside it.
+
+## 5. Console — two allowlist keys per host
+
+`OwnerAppMetrics` (`internal/console/metricsview.go`) builds its allowlist
+as `want[activator.UnitName(h)] = h`. It gains a second entry per host:
+`want[activator.ExecName(h)] = h`, where `ExecName(host)` returns
+`"carlos-exec@" + host` (defined next to `UnitName`, the same one-place
+naming discipline). Nothing else changes: exec-backed instances flow
+through the existing row assembly, `Reporting()` turns true, sparklines
+render, the freshest-box tie-break already handles a host moving between
+backings. The operator dashboard needs zero changes — rows appear by key.
+
+A host can never legally report under both names at once (one route, one
+backing), but if both ever appear in a sweep the existing freshest-box
+logic picks deterministically; no special case is added.
+
+## 6. Compatibility and rollout
+
+Wire shape is additive: `Sample.Units` gains new keys, and every chunk and
+manifest already in a production bucket still parses (wire-shape rule).
+An old console rendering a new box's chunk shows the new keys on the
+operator dashboard and ignores them in owner views — harmless both ways.
+
+Rollout order, per box:
+
+1. Ship the new binary fleet-wide. It tolerates the old unit file (nil
+ manager) indefinitely.
+2. Update `carlos-edge.service`, `systemctl daemon-reload`, restart the
+ unit. This is a **one-time operator step**: platform unit files are not
+ auto-converged today (the root adopt pass converges tenant drop-ins
+ only). Teaching the root adopt pass to converge platform units is a
+ real gap this work exposes but does not build; it is filed as a
+ follow-up, not smuggled in.
+
+Boxes: flagship (`annie-welding`), tito (`betty-fossicking`), the Pi fleet.
+Verify on the dev box first, against the binary actually built (stale-binary
+rule).
+
+## 7. Testing
+
+- The delegation dance and directory lifecycle run against temp-dir
+ fixtures — the same seam `listCarlosUnits` already uses; tests never
+ touch the live cgroup tree.
+- The clone3 fallback is behind an injectable spawn error, so both paths
+ are exercised without a seccomp filter in the loop.
+- Sampler discovery: fixture gains the third root; asserts `supervise` is
+ skipped and `carlos-exec@<host>` keys come out verbatim.
+- Console: allowlist test covering both key forms for one host, and the
+ no-double-report property of §5.
+- End-to-end on the dev box before flagship/tito: spawn keymail-shaped
+ exec routes, watch readings appear on the Instances page.
+
+## Out of scope
+
+- Bespoke units (`keymail-relay.service`): invisible for a different
+ reason — the `carlos-` name filter — and wanting a different,
+ naming-convention fix.
+- `days/` rollups (still a later task's view).
+- Per-instance resource **limits**: out of scope here, but this design
+ unlocks it — with each child in its own cgroup the agent could set
+ `memory.max` per instance, which the unit file's "No MemoryMax" comment
+ today rules out because a cap would catch every child at once.
+- Auto-convergence of platform unit files (filed as follow-up, §6).
diff --git a/docs/superpowers/specs/2026-08-08-exec-instance-metrics-design.md b/docs/superpowers/specs/2026-08-08-exec-instance-metrics-design.md
index d52cbbc25..10a3d71f4 100644
--- a/docs/superpowers/specs/2026-08-08-exec-instance-metrics-design.md
+++ b/docs/superpowers/specs/2026-08-08-exec-instance-metrics-design.md
@@ -1,5 +1,16 @@
# Exec-backed instance metrics (issue #118)
+> **SUPERSEDED (2026-08-08, same day):** the pid-sampling this spec chose
+> shipped in PR #121 and ran live briefly, then was retired by the
+> delegated-cgroup design in
+> `2026-08-07-exec-child-accounting-design.md` (PR #91), which restores
+> this document's own option 2 — per-child cgroups — once `Delegate=`
+> landed on carlos-edge.service and made it cheap. Exec children now
+> report under honest `carlos-exec@<host>` keys with hierarchical cgroup
+> numbers; the `carlos-app@<host>.service` keys this design published
+> survive only in raw chunks for their 48h retention. Kept for the
+> problem statement and the options analysis.
+
**Date:** 2026-08-08
**Issue:** carlosframework/platform#118 — exec-backed instances report no
CPU/memory: metrics sample unit cgroups only.
diff --git a/internal/activator/activator.go b/internal/activator/activator.go
index 5500cee25..9014bfe37 100644
--- a/internal/activator/activator.go
+++ b/internal/activator/activator.go
@@ -710,35 +710,6 @@ func (a *Service) beginStopLocked(e *actEntry) {
go a.stop(e)
}
-// AwakeExecPids reports the pid of every awake exec-backed child, keyed by
-// route host — the metrics sampler's seam (issue #118): an exec child runs
-// inside the agent's own cgroup, invisible to the per-unit cgroup walk, so
-// its per-instance attribution has to read /proc/<pid> instead, and only
-// the activator knows which pids those are. Included entries are awake,
-// not yet exited, and expose a pid: waking has no stable process yet,
-// stopping is on its way out, and a unit-backed tenant's proc (systemd-
-// driven, no Pid method) already reports through its own unit cgroup. Nil
-// when nothing qualifies — no readings, not an empty set of them.
-func (a *Service) AwakeExecPids() map[string]int {
- a.mu.Lock()
- defer a.mu.Unlock()
- var out map[string]int
- for _, e := range a.entries {
- if e.state != actAwake || e.proc.Exited() {
- continue
- }
- p, ok := e.proc.(interface{ Pid() int })
- if !ok {
- continue
- }
- if out == nil {
- out = make(map[string]int)
- }
- out[e.host] = p.Pid()
- }
- return out
-}
-
// Owns reports whether host has a live entry in ANY state — waking,
// awake, or stopping. This is the external-socket advisory's predicate
// (issue #17): while an entry exists, whatever is serving the host's
diff --git a/internal/activator/activator_test.go b/internal/activator/activator_test.go
index 6afbbfa45..b0f0b1d9e 100644
--- a/internal/activator/activator_test.go
+++ b/internal/activator/activator_test.go
@@ -160,11 +160,6 @@ type fakeBackend struct {
// target).
execVerFn func() string
- // pidByHost, when a host is present, makes Start() return a pid-BEARING
- // proc (pidProc) for it — modeling ExecBackend's childProc. Hosts absent
- // from the map get the plain pid-less fakeProc, modeling unitProc.
- pidByHost map[string]int
-
// Sidecar co-spawn. sidecarStaged is HasSidecar's answer (false by
// default, so every pre-co-spawn test doubles as the "host without a
// sidecar spawns none" case); sidecarErr injects spawn failures.
@@ -229,7 +224,6 @@ func (b *fakeBackend) Start(_ context.Context, rt registry.Route) (Proc, string,
startErr := b.startErr
proc := &fakeProc{label: "instance", rec: b}
b.procs = append(b.procs, proc)
- pid, hasPid := b.pidByHost[rt.Host]
b.mu.Unlock()
if startErr != nil {
return nil, "", startErr
@@ -241,22 +235,9 @@ func (b *fakeBackend) Start(_ context.Context, rt registry.Route) (Proc, string,
if fn != nil {
ver = fn()
}
- if hasPid {
- return &pidProc{fakeProc: proc, pid: pid}, ver, nil
- }
return proc, ver, nil
}
-// pidProc is fakeProc plus a pid — the shape ExecBackend's childProc has,
-// versus unitProc's pid-less one. AwakeExecPids distinguishes the two by
-// the optional Pid() interface, so the fakes must too.
-type pidProc struct {
- *fakeProc
- pid int
-}
-
-func (p *pidProc) Pid() int { return p.pid }
-
func (b *fakeBackend) NextDue(context.Context, registry.Route) (int64, error) {
b.mu.Lock()
defer b.mu.Unlock()
@@ -3267,57 +3248,3 @@ func TestExecBackendDoesNotNormalizeSquatterSocket(t *testing.T) {
t.Fatalf("squatter socket mode after Start = %o, want its own 600 — a socket our child did not bind is never ours to widen", fi.Mode().Perm())
}
}
-
-// AwakeExecPids is the metrics sampler's seam (issue #118): the host→pid map
-// of awake exec children. Pid-less procs (unit-backed tenants — they report
-// via their own unit cgroup), dead children, and stopping entries must all
-// be excluded — a pid the sampler reads must belong to a process the
-// activator still vouches for.
-func TestAwakeExecPids(t *testing.T) {
- a, b, _, _ := testActivator(t, time.Hour)
- b.pidByHost = map[string]int{
- "one.example.com": 4242,
- "three.example.com": 4343,
- }
-
- if got := a.AwakeExecPids(); got != nil {
- t.Fatalf("nothing awake: got %v, want nil", got)
- }
-
- for _, host := range []string{"one.example.com", "two.example.com", "three.example.com"} {
- release, err := a.beginRequest(context.Background(), testRoute(host))
- if err != nil {
- t.Fatalf("beginRequest %s: %v", host, err)
- }
- release()
- }
-
- // two (pid-less, modeling a unit tenant) is excluded; one and three report.
- got := a.AwakeExecPids()
- if len(got) != 2 || got["one.example.com"] != 4242 || got["three.example.com"] != 4343 {
- t.Fatalf("got %v, want one=4242 three=4343 only", got)
- }
-
- // A stopping entry is excluded the moment it flips, not once the stop
- // finishes: hold three's stop open on its gate and look while it drains.
- pThree := b.procs[2]
- pThree.mu.Lock()
- pThree.stopGate = make(chan struct{})
- pThree.mu.Unlock()
- if !a.Bounce("three.example.com") {
- t.Fatal("Bounce three: not awake?")
- }
- if got := a.AwakeExecPids(); len(got) != 1 || got["one.example.com"] != 4242 {
- t.Fatalf("with three stopping: got %v, want one=4242 only", got)
- }
- close(pThree.stopGate)
-
- // A dead-but-unreaped child is excluded even before a sweep notices it.
- pOne := b.procs[0]
- pOne.mu.Lock()
- pOne.dead = true
- pOne.mu.Unlock()
- if got := a.AwakeExecPids(); got != nil {
- t.Fatalf("with one dead: got %v, want nil", got)
- }
-}
diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go
index e8c2a7e66..57775d560 100644
--- a/internal/activator/backend_exec.go
+++ b/internal/activator/backend_exec.go
@@ -20,6 +20,7 @@ import (
"syscall"
"time"
+ "github.com/carlosframework/platform/internal/cgroup"
"github.com/carlosframework/platform/internal/edge"
"github.com/carlosframework/platform/internal/lease"
"github.com/carlosframework/platform/internal/registry"
@@ -56,6 +57,11 @@ type ExecBackend struct {
// instance so it can invalidate its own cached responses right after a
// write. Nil disables this — no purge listener, no error.
Cache *edge.Cache
+ // Cgroups, when non-nil, places every spawned child — instance, sidecar,
+ // seed — in a per-host cgroup under the agent's delegated subtree so the
+ // metrics sampler can account it (exec-child accounting spec). Nil — no
+ // delegation: old unit file, tests, darwin — spawns exactly as before.
+ Cgroups *cgroup.Manager
}
var _ Backend = (*ExecBackend)(nil)
@@ -213,39 +219,49 @@ func (b *ExecBackend) Seed(ctx context.Context, rt registry.Route) error {
ctx, cancel := context.WithTimeout(ctx, seedTimeout)
defer cancel()
argv := append(seedArgs, "--db", dbPath)
- cmd := exec.CommandContext(ctx, b.liveLink(host), argv...)
- // The purge-channel vars are agent-owned in BOTH directions here too
- // (Start's discipline, see withoutVars): fileEnv is tenant-supplied, so
- // without this a tenant's own env file could hand its seed a
- // CARLOS_CONTROL_SOCKET the agent never opened and a CARLOS_PURGE for a
- // channel that does not exist — and the seed has no serving cache to
- // purge in any case. This is what makes the contract above true rather
- // than merely intended.
- cmd.Env = childEnv(withoutVars(fileEnv, "CARLOS_CONTROL_SOCKET", "CARLOS_PURGE"))
- // Like the instance child, the seed logs through the agent's own streams
- // (journald under systemd) — and its stderr is ALSO teed into a bounded
- // buffer, because the wake error this returns is the whole diagnosis an
- // operator gets for a host that refuses to come up.
var stderr capBuffer
- cmd.Stdout = os.Stdout
- cmd.Stderr = io.MultiWriter(os.Stderr, &stderr)
- // WaitDelay is what makes seedTimeout real. Because Stderr is a plain
- // io.Writer (the tee), os/exec pipes it and Run waits for that pipe to
- // close — and a seed that spawned its own grandchild holds the write end
- // open long after the timeout killed the seed itself. Without this, a
- // wedged seed would hold the lease until the wake's own 5m budget expired.
- cmd.WaitDelay = seedDrainGrace
log.Printf("activator: seeding %s: %s %s", host, b.liveLink(host), strings.Join(argv, " "))
- if err := cmd.Run(); err != nil {
+ // The seed shares the instance's carlos-exec@<host> cgroup: first-boot
+ // cost is that instance's work. No CleanupHost here — the instance spawn
+ // reuses the dir moments later, and its exit path (or the next agent
+ // start's sweep) removes it.
+ cmd, err := b.Cgroups.Start(host, func() *exec.Cmd {
+ cmd := exec.CommandContext(ctx, b.liveLink(host), argv...)
+ // The purge-channel vars are agent-owned in BOTH directions here too
+ // (Start's discipline, see withoutVars): fileEnv is tenant-supplied, so
+ // without this a tenant's own env file could hand its seed a
+ // CARLOS_CONTROL_SOCKET the agent never opened and a CARLOS_PURGE for a
+ // channel that does not exist — and the seed has no serving cache to
+ // purge in any case. This is what makes the contract above true rather
+ // than merely intended.
+ cmd.Env = childEnv(withoutVars(fileEnv, "CARLOS_CONTROL_SOCKET", "CARLOS_PURGE"))
+ // Like the instance child, the seed logs through the agent's own streams
+ // (journald under systemd) — and its stderr is ALSO teed into a bounded
+ // buffer, because the wake error this returns is the whole diagnosis an
+ // operator gets for a host that refuses to come up.
+ cmd.Stdout = os.Stdout
+ cmd.Stderr = io.MultiWriter(os.Stderr, &stderr)
+ // WaitDelay is what makes seedTimeout real. Because Stderr is a plain
+ // io.Writer (the tee), os/exec pipes it and Wait waits for that pipe to
+ // close — and a seed that spawned its own grandchild holds the write end
+ // open long after the timeout killed the seed itself. Without this, a
+ // wedged seed would hold the lease until the wake's own 5m budget expired.
+ cmd.WaitDelay = seedDrainGrace
+ return cmd
+ })
+ if err == nil {
+ err = cmd.Wait()
+ }
+ if err != nil {
// WaitDelay reports on the SUCCESS path too: a seed that exits 0 after
// backgrounding something (a helper, a stray `&`) leaves the inherited
- // stderr pipe open, and Run then returns ErrWaitDelay INSTEAD of nil
+ // stderr pipe open, and Wait then returns ErrWaitDelay INSTEAD of nil
// even though the process exited 0. Treating that as a failure would
// fail the wake AND delete the database the seed had just correctly
// built — leaving the host fresh again next wake, i.e. permanently
// un-wakeable. The exit status is the verdict; the leaked pipe is a
// log line.
- if errors.Is(err, exec.ErrWaitDelay) && cmd.ProcessState != nil && cmd.ProcessState.Success() {
+ if cmd != nil && errors.Is(err, exec.ErrWaitDelay) && cmd.ProcessState != nil && cmd.ProcessState.Success() {
log.Printf("activator: seeded %s (it exited 0 but left a child holding its output pipe open for more than %s)", host, seedDrainGrace)
return nil
}
@@ -285,7 +301,7 @@ func seedCreatedTheDB(statErr error) bool { return os.IsNotExist(statErr) }
// seedTimeout bounds a first-boot seed run — a wedged seed must fail the wake
// (which releases the lease) rather than sit on it. seedDrainGrace is how long
-// Run then waits for the killed seed's output pipes to close before giving up
+// Wait then waits for the killed seed's output pipes to close before giving up
// on them (cmd.WaitDelay). Both vars so tests can shrink them.
var (
seedTimeout = 2 * time.Minute
@@ -295,9 +311,9 @@ var (
// capBuffer keeps at most capBufferMax bytes of what's written to it and
// discards the rest, so quoting a child's stderr in an error can never turn a
// chatty seed into an unbounded string. Locked because it is read after
-// cmd.Run returns while os/exec's own copy goroutine may still be writing: on
-// a WaitDelay expiry (the wedged-seed path) Run returns without waiting for
-// that copy to finish.
+// the seed's Wait returns while os/exec's own copy goroutine may still be
+// writing: on a WaitDelay expiry (the wedged-seed path) Wait returns without
+// waiting for that copy to finish.
type capBuffer struct {
mu sync.Mutex
b []byte
@@ -437,23 +453,33 @@ func (b *ExecBackend) Start(ctx context.Context, rt registry.Route) (Proc, strin
} else {
childVars = withoutVars(childVars, "CARLOS_CONTROL_SOCKET", "CARLOS_PURGE")
}
- cmd := exec.Command(bin, "--socket", rt.Socket, "--db", dbPath)
- cmd.Env = childEnv(childVars)
- // The child logs through the agent's own stderr/stdout (journald under
- // systemd); there's no per-instance unit to own a log stream anymore.
- cmd.Stdout = os.Stdout
- cmd.Stderr = os.Stderr
// Record the build we're about to exec — read the symlink immediately before
- // Start() so it names the version the kernel will resolve. Reading it here,
+ // the start so it names the version the kernel will resolve. Reading it here,
// rather than back in wake() after the health wait, closes a real window: a
// concurrent ship's repoint during the up-to-30s wait would otherwise make
// us record the NEW target while this child runs the OLD binary, and the
// sweep would then think it's already converged and never cycle it. The
- // residual sub-exec race (a repoint between this read and Start) is in the
- // safe direction — we'd record the older version, so the sweep cycles once
- // too often rather than never. Empty on a read error: the sweep won't cycle.
+ // residual sub-exec race (a repoint between this read and the start) is in
+ // the safe direction — we'd record the older version, so the sweep cycles
+ // once too often rather than never. Empty on a read error: the sweep won't
+ // cycle.
spawnVer, _ := b.LiveVersion(host)
- if err := cmd.Start(); err != nil {
+ // Spawned through the cgroup manager (nil-tolerant: no delegation means
+ // a plain start, exactly as before) so the child lands in
+ // carlos-exec@<host> and the sampler can account it. build may run
+ // twice — the manager's clone3-refusal fallback — so everything about
+ // the Cmd is configured inside it.
+ cmd, err := b.Cgroups.Start(host, func() *exec.Cmd {
+ cmd := exec.Command(bin, "--socket", rt.Socket, "--db", dbPath)
+ cmd.Env = childEnv(childVars)
+ // The child logs through the agent's own stderr/stdout (journald
+ // under systemd); there's no per-instance unit to own a log stream
+ // anymore.
+ cmd.Stdout = os.Stdout
+ cmd.Stderr = os.Stderr
+ return cmd
+ })
+ if err != nil {
return nil, "", err
}
@@ -479,6 +505,9 @@ func (b *ExecBackend) Start(ctx context.Context, rt registry.Route) (Proc, strin
}
go func() {
p.err = cmd.Wait()
+ // Last exiter wins: rmdir refuses while the sidecar (same host
+ // cgroup) is still alive, and that failure is tolerated.
+ b.Cgroups.CleanupHost(host)
close(p.done)
}()
@@ -632,18 +661,26 @@ func (b *ExecBackend) StartSidecar(_ context.Context, rt registry.Route) (Proc,
if err != nil {
return nil, fmt.Errorf("read sidecar env: %w", err)
}
- cmd := exec.Command(b.liveLink(rt.Host), "sidecar", "run")
- cmd.Env = childEnv(fileEnv)
- // Like the instance child: logs through the agent's own streams (journald
- // under systemd) — no per-host unit owns a log stream anymore.
- cmd.Stdout = os.Stdout
- cmd.Stderr = os.Stderr
- if err := cmd.Start(); err != nil {
+ // Same host cgroup as the instance: a sidecar's load is that instance's
+ // work, and attributing it elsewhere would be a lie in the other
+ // direction.
+ cmd, err := b.Cgroups.Start(rt.Host, func() *exec.Cmd {
+ cmd := exec.Command(b.liveLink(rt.Host), "sidecar", "run")
+ cmd.Env = childEnv(fileEnv)
+ // Like the instance child: logs through the agent's own streams
+ // (journald under systemd) — no per-host unit owns a log stream
+ // anymore.
+ cmd.Stdout = os.Stdout
+ cmd.Stderr = os.Stderr
+ return cmd
+ })
+ if err != nil {
return nil, err
}
p := &childProc{cmd: cmd, done: make(chan struct{})}
go func() {
p.err = cmd.Wait()
+ b.Cgroups.CleanupHost(rt.Host)
close(p.done)
}()
return p, nil
@@ -730,14 +767,6 @@ type childProc struct {
purge *edge.PurgeListener // nil if b.Cache was nil or ListenPurge failed
}
-// Pid reports the spawned child's OS pid — the seam AwakeExecPids reads so
-// the metrics sampler can find this child under /proc (issue #118: an exec
-// child lives in the agent's own cgroup, so per-pid reads are its only
-// per-instance attribution). cmd.Process is set once Start succeeded, which
-// is the only way a childProc is ever constructed, and stays valid for the
-// struct's life.
-func (p *childProc) Pid() int { return p.cmd.Process.Pid }
-
func (p *childProc) Exited() bool {
select {
case <-p.done:
diff --git a/internal/activator/backend_unit.go b/internal/activator/backend_unit.go
index 55caeda87..992e5423f 100644
--- a/internal/activator/backend_unit.go
+++ b/internal/activator/backend_unit.go
@@ -56,6 +56,7 @@ import (
"sync"
"time"
+ "github.com/carlosframework/platform/internal/cgroup"
"github.com/carlosframework/platform/internal/registry"
"github.com/carlosframework/platform/internal/release"
)
@@ -65,6 +66,13 @@ import (
// try-restarts for non-hibernating tenants.
func UnitName(host string) string { return "carlos-app@" + host + ".service" }
+// ExecName is the metrics sample key for an exec-backed tenant's children —
+// the delegated cgroup directory the activator spawns them into. Defined by
+// internal/cgroup (which creates the directories); surfaced here next to
+// UnitName so the console learns both of a host's legal metric keys from
+// the same place.
+func ExecName(host string) string { return cgroup.ExecName(host) }
+
// UnitBackend is the Backend for routes with Backing == registry.BackingUnit.
type UnitBackend struct {
// Systemctl runs one systemctl verb against a unit — the Proc-style seam
diff --git a/internal/cgroup/cgroup.go b/internal/cgroup/cgroup.go
new file mode 100644
index 000000000..9752a0cf0
--- /dev/null
+++ b/internal/cgroup/cgroup.go
@@ -0,0 +1,274 @@
+// Package cgroup gives each exec-backed child its own cgroup under the
+// subtree systemd delegates to carlos-edge.service (Delegate= in the unit
+// file), so the metrics sampler can report per-instance figures for routes
+// that run as exec children of the agent — today those live inside the
+// agent's own cgroup and can never show readings. Design:
+// docs/superpowers/specs/2026-08-07-exec-child-accounting-design.md.
+//
+// The agent is NOT root: everything here works through ownership of the
+// delegated subtree, never privilege. When delegation is absent (old unit
+// file, a test, darwin) Setup returns a nil *Manager and every method on it
+// degrades to today's behavior — fail toward absent readings, never toward
+// a broken spawn path.
+package cgroup
+
+import (
+ "errors"
+ "fmt"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "strconv"
+ "strings"
+ "sync/atomic"
+)
+
+// execPrefix is the ONE definition of the per-host exec cgroup naming —
+// activator.ExecName and the sampler's directory match both come through
+// this package.
+const execPrefix = "carlos-exec@"
+
+// ExecName is the cgroup directory name — and therefore the metrics sample
+// key — for an exec-backed host's children. Deliberately NOT dressed up as
+// a fake ".service": an operator who takes a dashboard row to `systemctl
+// status` must not be told a unit exists that doesn't.
+func ExecName(host string) string { return execPrefix + host }
+
+// IsExecName reports whether a cgroup directory name is one of our per-host
+// exec cgroups: the carlos-exec@ prefix with a non-empty host after it.
+func IsExecName(name string) bool {
+ return strings.HasPrefix(name, execPrefix) && len(name) > len(execPrefix)
+}
+
+// ParseSelfCgroup extracts the cgroup v2 path from /proc/self/cgroup
+// content — the "0::" unified-hierarchy line — relative (no leading slash),
+// ready to join under /sys/fs/cgroup. Not-ok when there is no v2 line (a
+// v1-only box) or the process sits in the root cgroup: neither is a subtree
+// systemd could have delegated to us.
+func ParseSelfCgroup(data string) (string, bool) {
+ for _, line := range strings.Split(data, "\n") {
+ rest, found := strings.CutPrefix(line, "0::")
+ if !found {
+ continue
+ }
+ p := strings.TrimPrefix(rest, "/")
+ if p == "" {
+ return "", false
+ }
+ return p, true
+ }
+ return "", false
+}
+
+// Manager owns the delegated subtree. A nil *Manager is the documented
+// "no delegation" state: every method is nil-safe and degrades to today's
+// behavior.
+type Manager struct {
+ root string
+ logf func(string, ...any)
+ // fallback, once latched, means clone3-into-cgroup is refused here (a
+ // seccomp filter older than the syscall, an old kernel) and children are
+ // started normally then moved by PID. A filter is static for the life of
+ // the process, so retrying the syscall on every spawn buys nothing.
+ fallback atomic.Bool
+}
+
+// Root is the delegated subtree's path — the sampler's third scan root.
+// "" for a nil manager (no delegation): the sampler skips an empty root.
+func (m *Manager) Root() string {
+ if m == nil {
+ return ""
+ }
+ return m.root
+}
+
+// Setup arms exec-child accounting for the running agent: resolve our own
+// cgroup from /proc/self/cgroup and run the delegation dance there. Any
+// failure — v1-only box, old unit file without Delegate=, a test, darwin —
+// returns nil with one logged line, and every spawn behaves exactly as
+// today. Fail toward absent readings, never toward a broken spawn path.
+func Setup(logf func(string, ...any)) *Manager {
+ b, err := os.ReadFile("/proc/self/cgroup")
+ if err != nil {
+ logf("agent: exec-child accounting disabled: %v", err)
+ return nil
+ }
+ rel, ok := ParseSelfCgroup(string(b))
+ if !ok {
+ logf("agent: exec-child accounting disabled: no cgroup v2 path in /proc/self/cgroup")
+ return nil
+ }
+ m, err := SetupAt(filepath.Join("/sys/fs/cgroup", rel), logf)
+ if err != nil {
+ logf("agent: exec-child accounting disabled (no delegated subtree — unit file without Delegate=?): %v", err)
+ return nil
+ }
+ logf("agent: exec-child accounting armed (delegated subtree %s)", m.Root())
+ return m
+}
+
+// SetupAt is the delegation dance against an explicit subtree root — the
+// testable core Setup wraps. Order matters: probe, move self to a leaf, THEN
+// enable controllers — cgroup v2 refuses subtree_control writes while the
+// cgroup itself still has member processes ("no internal processes").
+func SetupAt(root string, logf func(string, ...any)) (*Manager, error) {
+ // 1. Probe live delegation: both control files must be writable by us.
+ // systemd chowns exactly these to User= when Delegate= is set; without
+ // it this open is the cheap, side-effect-free "no" that keeps an
+ // undelegated box on today's path.
+ for _, name := range []string{"cgroup.procs", "cgroup.subtree_control"} {
+ f, err := os.OpenFile(filepath.Join(root, name), os.O_WRONLY, 0)
+ if err != nil {
+ return nil, fmt.Errorf("probe delegation: %w", err)
+ }
+ f.Close()
+ }
+ // 2. Move ourselves into a supervise/ leaf, making step 3 legal.
+ sup := filepath.Join(root, "supervise")
+ if err := os.MkdirAll(sup, 0o755); err != nil {
+ return nil, fmt.Errorf("create supervise leaf: %w", err)
+ }
+ if err := os.WriteFile(filepath.Join(sup, "cgroup.procs"), []byte(strconv.Itoa(os.Getpid())), 0o644); err != nil {
+ return nil, fmt.Errorf("move self into supervise leaf: %w", err)
+ }
+ // 3. Enable the controllers per-child cgroups need for memory.current,
+ // io.stat and pids accounting (cpu.stat exists regardless, but +cpu
+ // keeps utilization attributable). One write, all four: the kernel
+ // applies subtree_control writes atomically.
+ if err := os.WriteFile(filepath.Join(root, "cgroup.subtree_control"), []byte("+cpu +memory +io +pids"), 0o644); err != nil {
+ return nil, fmt.Errorf("enable controllers: %w", err)
+ }
+ m := &Manager{root: root, logf: logf}
+ // 4. Sweep per-host dirs a previous agent left behind — empty after a
+ // restart killed their members. rmdir refuses a non-empty dir, which is
+ // exactly the tolerance we want.
+ m.sweepStale()
+ return m, nil
+}
+
+// sweepStale removes leftover carlos-exec@* dirs under the root. Failures
+// are logged and tolerated: a dir that won't rmdir has something alive in
+// it, and killing is the service manager's job, not ours.
+func (m *Manager) sweepStale() {
+ entries, err := os.ReadDir(m.root)
+ if err != nil {
+ return
+ }
+ for _, e := range entries {
+ if !e.IsDir() || !IsExecName(e.Name()) {
+ continue
+ }
+ if err := os.Remove(filepath.Join(m.root, e.Name())); err != nil {
+ m.logf("cgroup: stale %s not removed: %v", e.Name(), err)
+ }
+ }
+}
+
+// startCmd is Start's spawn seam — a package var so the clone3 refusal
+// path is testable without a seccomp filter in the loop.
+var startCmd = func(cmd *exec.Cmd) error { return cmd.Start() }
+
+// hostDir is the per-host cgroup: instance, sidecar and seed all share it —
+// a seed's first-boot cost and a sidecar's load ARE that instance's work.
+func (m *Manager) hostDir(host string) string { return filepath.Join(m.root, ExecName(host)) }
+
+// Start runs build() and starts the result inside host's cgroup. Nil-safe:
+// with no manager (or any accounting failure) the child starts exactly as
+// today — a spawn must never fail because accounting failed. build must
+// return a fully configured, unstarted Cmd; it may be called twice, because
+// a Cmd cannot be reused after a failed Start. On a non-nil error the
+// returned Cmd may be nil.
+//
+// Primary mechanism: clone3 with CLONE_INTO_CGROUP (SysProcAttr.UseCgroupFD)
+// — the child never spends a cycle in the wrong cgroup. If that start
+// fails, the cgroup fd may or may not be why (a seccomp filter older than
+// clone3 answers EPERM; so does a missing binary): retry WITHOUT the fd,
+// and only if the retry succeeds — proving the cgroup was the problem — and
+// only if the failure actually means the mechanism is gone (shouldLatch),
+// latch the fallback and move the child by PID. A transient, spawn-specific
+// error (e.g. the cgroup dir vanishing between os.Open and start) falls
+// back for this spawn only; the next Start still tries clone3. The window
+// between start and move charges the child's first moments to the agent:
+// mis-accounting in the safe direction. Note the PID write migrates only
+// the child process itself — any grandchildren it has already forked by
+// then stay charged to the agent's cgroup, not the child's.
+func (m *Manager) Start(host string, build func() *exec.Cmd) (*exec.Cmd, error) {
+ if m == nil {
+ cmd := build()
+ return cmd, startCmd(cmd)
+ }
+ dir := m.hostDir(host)
+ if err := os.MkdirAll(dir, 0o755); err != nil {
+ m.logf("cgroup: %s: create cgroup: %v — spawning unaccounted", host, err)
+ cmd := build()
+ return cmd, startCmd(cmd)
+ }
+ if intoCgroupSupported && !m.fallback.Load() {
+ f, err := os.Open(dir)
+ if err != nil {
+ m.logf("cgroup: %s: open cgroup dir: %v — using PID-move fallback for this spawn", host, err)
+ return m.startAndMove(host, build)
+ }
+ cmd := build()
+ setInto(cmd, int(f.Fd()))
+ err = startCmd(cmd)
+ f.Close()
+ if err == nil {
+ return cmd, nil
+ }
+ firstErr := err
+ retried, retryErr := m.startAndMove(host, build)
+ if retryErr != nil {
+ // startAndMove already dropped the ghost host dir on its own
+ // failure; join both errors so the operator can see a clone3
+ // attempt happened at all, not just that the retry failed.
+ return nil, errors.Join(firstErr, retryErr)
+ }
+ if shouldLatch(firstErr) {
+ m.fallback.Store(true)
+ m.logf("cgroup: clone3-into-cgroup refused (%v) — latching PID-move fallback for the life of this process", firstErr)
+ } else {
+ m.logf("cgroup: clone3-into-cgroup failed for this spawn (%v) — not latching", firstErr)
+ }
+ return retried, nil
+ }
+ return m.startAndMove(host, build)
+}
+
+// startAndMove is the fallback spawn: plain start, then write the child's
+// PID into the host cgroup. A failed move is logged and tolerated — the
+// child runs (unaccounted), which beats not running.
+func (m *Manager) startAndMove(host string, build func() *exec.Cmd) (*exec.Cmd, error) {
+ cmd := build()
+ if err := startCmd(cmd); err != nil {
+ // The spawn never happened: drop the now-ownerless host dir so a
+ // failed wake doesn't leave the sampler reporting permanent zeros
+ // for a host with nothing running. rmdir-safe — os.Remove refuses a
+ // non-empty dir, so this can never take a live sibling's dir.
+ _ = os.Remove(m.hostDir(host))
+ return nil, err
+ }
+ if cmd.Process == nil {
+ // A broken startCmd seam reporting success with no process: this
+ // package must never panic the agent over it. Log and skip the
+ // move — the child, if any, simply stays unaccounted.
+ m.logf("cgroup: %s: start reported success with no process — skipping pid move", host)
+ return cmd, nil
+ }
+ procs := filepath.Join(m.hostDir(host), "cgroup.procs")
+ if err := os.WriteFile(procs, []byte(strconv.Itoa(cmd.Process.Pid)), 0o644); err != nil {
+ m.logf("cgroup: %s: move pid %d: %v — child stays unaccounted", host, cmd.Process.Pid, err)
+ }
+ return cmd, nil
+}
+
+// CleanupHost removes host's per-host cgroup dir after its children exit.
+// Nil-safe and rmdir-tolerant: while a sibling (instance vs sidecar) is
+// still alive the dir is non-empty and the remove fails — the LAST exiter's
+// call succeeds, and the next agent start's sweep catches anything missed.
+func (m *Manager) CleanupHost(host string) {
+ if m == nil {
+ return
+ }
+ _ = os.Remove(m.hostDir(host))
+}
diff --git a/internal/cgroup/cgroup_linux.go b/internal/cgroup/cgroup_linux.go
new file mode 100644
index 000000000..5b362aab9
--- /dev/null
+++ b/internal/cgroup/cgroup_linux.go
@@ -0,0 +1,38 @@
+//go:build linux
+
+package cgroup
+
+import (
+ "errors"
+ "os/exec"
+ "syscall"
+)
+
+// intoCgroupSupported gates the clone3 CLONE_INTO_CGROUP attempt to
+// platforms whose SysProcAttr has the fields — everywhere else Start goes
+// straight to the PID-move fallback.
+const intoCgroupSupported = true
+
+// setInto arms cmd for clone3-into-cgroup without clobbering any
+// SysProcAttr build() already configured (Setpgid, Pdeathsig, ...) — build()
+// owns all Cmd configuration; Start only adds to it, never replaces it.
+func setInto(cmd *exec.Cmd, fd int) {
+ if cmd.SysProcAttr == nil {
+ cmd.SysProcAttr = &syscall.SysProcAttr{}
+ }
+ cmd.SysProcAttr.UseCgroupFD = true
+ cmd.SysProcAttr.CgroupFD = fd
+}
+
+// shouldLatch reports whether err proves clone3-into-cgroup is unavailable
+// for the life of this process — EPERM (a seccomp filter predating clone3,
+// or one that forbids it outright) and ENOSYS (a kernel predating clone3)
+// mean the mechanism itself is gone; EINVAL means the kernel understood
+// clone3 but rejected CLONE_INTO_CGROUP. Anything else (e.g. ENODEV from the
+// cgroup dir disappearing between os.Open and start, raced by a concurrent
+// CleanupHost) is spawn-specific and says nothing about the mechanism —
+// latching on it would permanently downgrade the process over a fluke and
+// log a lie ("refused").
+func shouldLatch(err error) bool {
+ return errors.Is(err, syscall.EPERM) || errors.Is(err, syscall.ENOSYS) || errors.Is(err, syscall.EINVAL)
+}
diff --git a/internal/cgroup/cgroup_linux_test.go b/internal/cgroup/cgroup_linux_test.go
new file mode 100644
index 000000000..05d902b58
--- /dev/null
+++ b/internal/cgroup/cgroup_linux_test.go
@@ -0,0 +1,185 @@
+//go:build linux
+
+package cgroup
+
+import (
+ "errors"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "syscall"
+ "testing"
+)
+
+// TestStartUsesCgroupFDFirst: with delegation live and no latch, the child
+// starts via clone3-into-cgroup — SysProcAttr set, host dir created, and NO
+// cgroup.procs write (the kernel placed it, no PID move needed).
+func TestStartUsesCgroupFDFirst(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ calls := fakeStart(t, func(*exec.Cmd) error { return nil })
+ if _, err := m.Start("h.example.com", buildTrue); err != nil {
+ t.Fatal(err)
+ }
+ if len(*calls) != 1 {
+ t.Fatalf("started %d times, want 1", len(*calls))
+ }
+ attr := (*calls)[0].SysProcAttr
+ if attr == nil || !attr.UseCgroupFD {
+ t.Fatalf("SysProcAttr = %+v, want UseCgroupFD", attr)
+ }
+ dir := filepath.Join(root, ExecName("h.example.com"))
+ if _, err := os.Stat(dir); err != nil {
+ t.Fatalf("host cgroup dir not created: %v", err)
+ }
+ if _, err := os.Stat(filepath.Join(dir, "cgroup.procs")); !os.IsNotExist(err) {
+ t.Errorf("clone3 path must not write cgroup.procs (the kernel places the child): %v", err)
+ }
+}
+
+// TestStartFallsBackAndLatches pins the refusal path: clone3 refused
+// (seccomp EPERM), the retry WITHOUT the cgroup fd succeeds — proving the
+// cgroup was the problem — the child's PID is moved by file write, and the
+// latch makes every later spawn skip clone3 entirely.
+func TestStartFallsBackAndLatches(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ calls := fakeStart(t, func(cmd *exec.Cmd) error {
+ if cmd.SysProcAttr != nil {
+ return syscall.EPERM // the filter refusing clone3
+ }
+ return nil
+ })
+ if _, err := m.Start("h.example.com", buildTrue); err != nil {
+ t.Fatal(err)
+ }
+ if len(*calls) != 2 {
+ t.Fatalf("first spawn: %d start calls, want 2 (clone3 attempt + plain retry)", len(*calls))
+ }
+ procs := filepath.Join(root, ExecName("h.example.com"), "cgroup.procs")
+ if _, err := os.Stat(procs); err != nil {
+ t.Fatalf("fallback did not move the PID: %v", err)
+ }
+ if !m.fallback.Load() {
+ t.Fatal("refusal did not latch the fallback")
+ }
+
+ if _, err := m.Start("h2.example.com", buildTrue); err != nil {
+ t.Fatal(err)
+ }
+ if len(*calls) != 3 {
+ t.Fatalf("latched spawn: %d total start calls, want 3 (no clone3 re-attempt)", len(*calls))
+ }
+ if (*calls)[2].SysProcAttr != nil {
+ t.Fatal("latched spawn still set SysProcAttr")
+ }
+}
+
+// TestStartMergesIntoBuilderSysProcAttr: build() owns all Cmd configuration
+// — Start must ADD UseCgroupFD/CgroupFD to whatever SysProcAttr build()
+// already set, never replace it. A builder setting Setpgid or Pdeathsig
+// must keep that field on the clone3 attempt.
+func TestStartMergesIntoBuilderSysProcAttr(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ calls := fakeStart(t, func(*exec.Cmd) error { return nil })
+ build := func() *exec.Cmd {
+ cmd := exec.Command("/bin/true")
+ cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+ return cmd
+ }
+ if _, err := m.Start("h.example.com", build); err != nil {
+ t.Fatal(err)
+ }
+ attr := (*calls)[0].SysProcAttr
+ if attr == nil || !attr.Setpgid {
+ t.Fatalf("SysProcAttr = %+v, want Setpgid preserved from build()", attr)
+ }
+ if !attr.UseCgroupFD {
+ t.Fatalf("SysProcAttr = %+v, want UseCgroupFD added by Start", attr)
+ }
+}
+
+// TestStartTransientClone3FailureDoesNotLatch: ENODEV (e.g. the cgroup dir
+// disappearing between os.Open and start, raced by a concurrent
+// CleanupHost) says nothing about whether clone3-into-cgroup works at all —
+// the spawn falls back for THIS call only, and the NEXT Start still
+// attempts clone3.
+func TestStartTransientClone3FailureDoesNotLatch(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ calls := fakeStart(t, func(cmd *exec.Cmd) error {
+ if cmd.SysProcAttr != nil {
+ return syscall.ENODEV
+ }
+ return nil
+ })
+ if _, err := m.Start("h.example.com", buildTrue); err != nil {
+ t.Fatal(err)
+ }
+ if len(*calls) != 2 {
+ t.Fatalf("first spawn: %d start calls, want 2 (clone3 attempt + plain retry)", len(*calls))
+ }
+ if m.fallback.Load() {
+ t.Fatal("a transient clone3 failure latched the fallback")
+ }
+
+ if _, err := m.Start("h2.example.com", buildTrue); err != nil {
+ t.Fatal(err)
+ }
+ // The fake keeps refusing clone3 with ENODEV for h2 too, so the second
+ // spawn again pays for a clone3 attempt (fails) plus a plain retry
+ // (succeeds): two more calls. What matters is call index 2 — the second
+ // spawn's FIRST attempt — still carries SysProcAttr: proof clone3 was
+ // retried rather than skipped by a latched fallback.
+ if len(*calls) != 4 {
+ t.Fatalf("second spawn: %d total start calls, want 4 (clone3 attempted again + its own retry)", len(*calls))
+ }
+ if (*calls)[2].SysProcAttr == nil || !(*calls)[2].SysProcAttr.UseCgroupFD {
+ t.Fatal("second spawn did not attempt clone3 again — fallback must not have latched")
+ }
+}
+
+// TestStartJoinsBothErrorsOnTotalFailure: when the clone3 attempt AND the
+// PID-move retry both fail, the operator needs to see that a clone3 attempt
+// happened at all — the returned error must reference both, and the ghost
+// host dir must not survive.
+func TestStartJoinsBothErrorsOnTotalFailure(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ errClone3 := errors.New("clone3 boom")
+ errRetry := errors.New("retry boom")
+ first := true
+ fakeStart(t, func(*exec.Cmd) error {
+ if first {
+ first = false
+ return errClone3
+ }
+ return errRetry
+ })
+ _, err = m.Start("h.example.com", buildTrue)
+ if err == nil {
+ t.Fatal("Start with two failing spawns succeeded, want error")
+ }
+ if !errors.Is(err, errClone3) || !errors.Is(err, errRetry) {
+ t.Fatalf("Start error = %v, want it to wrap both %v and %v", err, errClone3, errRetry)
+ }
+ if _, statErr := os.Stat(filepath.Join(root, ExecName("h.example.com"))); !os.IsNotExist(statErr) {
+ t.Errorf("ghost host dir survived total spawn failure: %v", statErr)
+ }
+}
diff --git a/internal/cgroup/cgroup_other.go b/internal/cgroup/cgroup_other.go
new file mode 100644
index 000000000..04e8dbcbb
--- /dev/null
+++ b/internal/cgroup/cgroup_other.go
@@ -0,0 +1,14 @@
+//go:build !linux
+
+package cgroup
+
+import "os/exec"
+
+// intoCgroupSupported gates the clone3 CLONE_INTO_CGROUP attempt to
+// platforms whose SysProcAttr has the fields — everywhere else Start goes
+// straight to the PID-move fallback.
+const intoCgroupSupported = false
+
+func setInto(cmd *exec.Cmd, fd int) {}
+
+func shouldLatch(error) bool { return false }
diff --git a/internal/cgroup/cgroup_test.go b/internal/cgroup/cgroup_test.go
new file mode 100644
index 000000000..ee4ffba26
--- /dev/null
+++ b/internal/cgroup/cgroup_test.go
@@ -0,0 +1,269 @@
+package cgroup
+
+import (
+ "errors"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "strconv"
+ "testing"
+)
+
+func TestExecNameAndIsExecName(t *testing.T) {
+ if got := ExecName("keymail.dev"); got != "carlos-exec@keymail.dev" {
+ t.Fatalf("ExecName = %q, want %q", got, "carlos-exec@keymail.dev")
+ }
+ for name, want := range map[string]bool{
+ "carlos-exec@keymail.dev": true,
+ "carlos-exec@": false, // no host — not one of ours
+ "supervise": false, // the agent's own leaf
+ "carlos-app@x.service": false,
+ "cgroup.procs": false,
+ } {
+ if got := IsExecName(name); got != want {
+ t.Errorf("IsExecName(%q) = %v, want %v", name, got, want)
+ }
+ }
+}
+
+// TestParseSelfCgroup pins the /proc/self/cgroup v2 line format: the "0::"
+// line carries the unified-hierarchy path. v1-only content (no 0:: line)
+// and the root cgroup ("0::/") both report not-ok — neither is a subtree
+// systemd could have delegated to us.
+func TestParseSelfCgroup(t *testing.T) {
+ cases := []struct {
+ name, data, want string
+ ok bool
+ }{
+ {"systemd service", "0::/system.slice/carlos-edge.service\n", "system.slice/carlos-edge.service", true},
+ {"hybrid box, v2 line among v1", "12:pids:/system.slice\n0::/system.slice/carlos-edge.service\n", "system.slice/carlos-edge.service", true},
+ {"v1 only", "12:pids:/system.slice\n11:cpu:/system.slice\n", "", false},
+ {"root cgroup", "0::/\n", "", false},
+ {"empty", "", "", false},
+ }
+ for _, c := range cases {
+ got, ok := ParseSelfCgroup(c.data)
+ if got != c.want || ok != c.ok {
+ t.Errorf("%s: ParseSelfCgroup = (%q, %v), want (%q, %v)", c.name, got, ok, c.want, c.ok)
+ }
+ }
+}
+
+// newDelegatedRoot builds the temp-dir shape of a delegated cgroup: the two
+// control files systemd chowns to the unit's user when Delegate= is set.
+// Everything the dance does is plain file I/O, so a temp dir is a faithful
+// fixture — the same seam listCarlosUnits uses in internal/host.
+func newDelegatedRoot(t *testing.T) string {
+ t.Helper()
+ root := t.TempDir()
+ for _, f := range []string{"cgroup.procs", "cgroup.subtree_control"} {
+ if err := os.WriteFile(filepath.Join(root, f), nil, 0o644); err != nil {
+ t.Fatal(err)
+ }
+ }
+ return root
+}
+
+// TestSetupAtRefusesUndelegatedRoot: without Delegate= the control files are
+// root-owned (here: absent) — SetupAt must error so Setup degrades to the
+// nil manager instead of half-arming.
+func TestSetupAtRefusesUndelegatedRoot(t *testing.T) {
+ if _, err := SetupAt(t.TempDir(), t.Logf); err == nil {
+ t.Fatal("SetupAt on an undelegated root succeeded, want error")
+ }
+}
+
+// TestSetupAtDance pins the dance's observable results: the agent's own PID
+// moved into the supervise/ leaf (cgroup v2 forbids enabling controllers on
+// a cgroup with member processes), then the four controllers enabled in
+// subtree_control, and Root reporting the subtree.
+func TestSetupAtDance(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if m.Root() != root {
+ t.Errorf("Root = %q, want %q", m.Root(), root)
+ }
+ pid, err := os.ReadFile(filepath.Join(root, "supervise", "cgroup.procs"))
+ if err != nil {
+ t.Fatalf("supervise leaf not populated: %v", err)
+ }
+ if string(pid) != strconv.Itoa(os.Getpid()) {
+ t.Errorf("supervise/cgroup.procs = %q, want our own pid %d", pid, os.Getpid())
+ }
+ ctrl, err := os.ReadFile(filepath.Join(root, "cgroup.subtree_control"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if string(ctrl) != "+cpu +memory +io +pids" {
+ t.Errorf("subtree_control = %q, want %q", ctrl, "+cpu +memory +io +pids")
+ }
+}
+
+// TestSetupAtSweepsStaleExecDirs: a previous agent's per-host dirs are empty
+// after its children died with the service; the dance removes them. A
+// non-empty one (something still alive in it — shouldn't happen, but rmdir
+// would fail anyway) and a foreign name are left alone.
+func TestSetupAtSweepsStaleExecDirs(t *testing.T) {
+ root := newDelegatedRoot(t)
+ stale := filepath.Join(root, "carlos-exec@old.example.com")
+ if err := os.Mkdir(stale, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ occupied := filepath.Join(root, "carlos-exec@busy.example.com")
+ if err := os.MkdirAll(filepath.Join(occupied, "child"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ foreign := filepath.Join(root, "not-ours")
+ if err := os.Mkdir(foreign, 0o755); err != nil {
+ t.Fatal(err)
+ }
+
+ if _, err := SetupAt(root, t.Logf); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := os.Stat(stale); !os.IsNotExist(err) {
+ t.Errorf("stale exec dir survived the sweep: %v", err)
+ }
+ if _, err := os.Stat(occupied); err != nil {
+ t.Errorf("non-empty exec dir must be tolerated, not force-removed: %v", err)
+ }
+ if _, err := os.Stat(foreign); err != nil {
+ t.Errorf("foreign dir must be untouched: %v", err)
+ }
+}
+
+func TestNilManagerRoot(t *testing.T) {
+ var m *Manager
+ if got := m.Root(); got != "" {
+ t.Fatalf("nil Root = %q, want \"\"", got)
+ }
+}
+
+// fakeStart swaps the package's spawn seam for a recorder, restoring it on
+// cleanup. The fake never spawns anything: "success" fabricates cmd.Process
+// from our own PID so the PID-move path has a real number to write.
+func fakeStart(t *testing.T, fail func(cmd *exec.Cmd) error) *[]*exec.Cmd {
+ t.Helper()
+ var calls []*exec.Cmd
+ orig := startCmd
+ startCmd = func(cmd *exec.Cmd) error {
+ calls = append(calls, cmd)
+ if err := fail(cmd); err != nil {
+ return err
+ }
+ cmd.Process, _ = os.FindProcess(os.Getpid())
+ return nil
+ }
+ t.Cleanup(func() { startCmd = orig })
+ return &calls
+}
+
+func buildTrue() *exec.Cmd { return exec.Command("/bin/true") }
+
+// TestStartNilManagerSpawnsPlainly: the nil manager is the no-delegation
+// state — one plain start, no SysProcAttr, no filesystem writes.
+func TestStartNilManagerSpawnsPlainly(t *testing.T) {
+ calls := fakeStart(t, func(*exec.Cmd) error { return nil })
+ var m *Manager
+ cmd, err := m.Start("h.example.com", buildTrue)
+ if err != nil || cmd == nil {
+ t.Fatalf("Start = (%v, %v), want started cmd", cmd, err)
+ }
+ if len(*calls) != 1 || (*calls)[0].SysProcAttr != nil {
+ t.Fatalf("nil manager must start exactly once with no SysProcAttr: %d calls", len(*calls))
+ }
+}
+
+// TestStartGenuineSpawnFailureDoesNotLatch: when the retry-without-cgroup
+// ALSO fails, the cgroup was not the problem — the error propagates and the
+// fallback must not latch (the next spawn should still try clone3).
+func TestStartGenuineSpawnFailureDoesNotLatch(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ fakeStart(t, func(*exec.Cmd) error { return errors.New("no such binary") })
+ if _, err := m.Start("h.example.com", buildTrue); err == nil {
+ t.Fatal("Start with a failing spawn succeeded, want error")
+ }
+ if m.fallback.Load() {
+ t.Fatal("a genuine spawn failure latched the clone3 fallback")
+ }
+}
+
+// TestCleanupHost: removes an empty per-host dir, tolerates a non-empty
+// one, and is nil-safe.
+func TestCleanupHost(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ empty := filepath.Join(root, ExecName("gone.example.com"))
+ if err := os.Mkdir(empty, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ m.CleanupHost("gone.example.com")
+ if _, err := os.Stat(empty); !os.IsNotExist(err) {
+ t.Errorf("empty host dir not removed: %v", err)
+ }
+
+ busy := filepath.Join(root, ExecName("busy.example.com"))
+ if err := os.MkdirAll(filepath.Join(busy, "child"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ m.CleanupHost("busy.example.com") // must not panic or force-remove
+ if _, err := os.Stat(busy); err != nil {
+ t.Errorf("non-empty host dir must survive: %v", err)
+ }
+
+ var nilM *Manager
+ nilM.CleanupHost("h.example.com") // must not panic
+}
+
+// TestStartAndMoveRemovesGhostDirOnSpawnFailure: when the spawn itself
+// never happens, the host cgroup dir must not survive it — a ghost dir
+// would make the sampler report permanent zeros for a host with nothing
+// running.
+func TestStartAndMoveRemovesGhostDirOnSpawnFailure(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ dir := m.hostDir("h.example.com")
+ if err := os.MkdirAll(dir, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ fakeStart(t, func(*exec.Cmd) error { return errors.New("no such binary") })
+ if _, err := m.startAndMove("h.example.com", buildTrue); err == nil {
+ t.Fatal("startAndMove with a failing spawn succeeded, want error")
+ }
+ if _, err := os.Stat(dir); !os.IsNotExist(err) {
+ t.Errorf("ghost host dir survived a failed spawn: %v", err)
+ }
+}
+
+// TestStartAndMoveToleratesBrokenSeam: a startCmd seam that reports success
+// without ever setting cmd.Process must not panic while writing the PID —
+// this package must never panic the agent over a broken seam.
+func TestStartAndMoveToleratesBrokenSeam(t *testing.T) {
+ root := newDelegatedRoot(t)
+ m, err := SetupAt(root, t.Logf)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := os.MkdirAll(m.hostDir("h.example.com"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ orig := startCmd
+ startCmd = func(cmd *exec.Cmd) error { return nil } // never sets cmd.Process
+ t.Cleanup(func() { startCmd = orig })
+ if _, err := m.startAndMove("h.example.com", buildTrue); err != nil {
+ t.Fatal(err)
+ }
+}
diff --git a/internal/console/metricsview.go b/internal/console/metricsview.go
index 4dd763e2a..2aae2b16f 100644
--- a/internal/console/metricsview.go
+++ b/internal/console/metricsview.go
@@ -13,6 +13,7 @@ import (
"encoding/json"
"errors"
"fmt"
+ "log"
"os"
"sort"
"strconv"
@@ -26,13 +27,15 @@ import (
// UnitRow is one per-instance reading, ready to render, plus its own 48h
// sparklines (the owner app-metrics view renders a unit on its own, with no
-// host row to borrow a shape from). Keyed by unit name, but the name is the
-// reading's IDENTITY, not a promise that systemd knows it: an exec-backed
-// instance has no unit at all — it is a child of `carlos agent`, sampled by
-// pid — and publishes under the carlos-app@<host>.service name it WOULD
-// have (issue #118), so `systemctl status` on a box can come up empty for a
-// row that is live here. The route host inside the name is still the thing
-// to grep the agent's journal for.
+// host row to borrow a shape from). Keyed by sample name, which is the
+// reading's IDENTITY, not always a systemd unit: an exec-backed instance has
+// no unit at all — it is a child of `carlos agent`, accounted in a delegated
+// cgroup — and reports under carlos-exec@<host> (no .service; exec-child
+// accounting spec). Boxes running the superseded pid sampler (issue #118,
+// retired by that spec) published exec children under the
+// carlos-app@<host>.service name they WOULD have had, and those keys survive
+// in raw chunks for the 48h retention window. Either way the route host
+// inside the name is the thing to grep the agent's journal for.
type UnitRow struct {
Unit string // full unit name
CPUFrac *float64
@@ -59,27 +62,77 @@ type AlertRow struct {
FiredAt time.Time
}
-// metricsCacheTTL bounds how long LoadMetrics reuses its last successful
-// sweep before touching the bucket again. All callers within the window
-// share the one sweep — the operator dashboard and every owner's app-metrics
-// page render off the same read.
+// metricsCacheTTL is how soon after a successful sweep LoadMetrics considers
+// the cache FRESH and skips even trying to refresh it. It does NOT bound how
+// long a result may be served overall — see metricsStaleServeCeiling for
+// that outer bound. Past metricsCacheTTL the cache becomes STALE (still
+// served immediately, but a background refresh is triggered) rather than
+// unusable. All callers within the FRESH window share the one sweep — the
+// operator dashboard and every owner's app-metrics page render off the same
+// read.
const metricsCacheTTL = 60 * time.Second
+// metricsStaleServeCeiling bounds how old the cache may be and still be
+// handed out immediately while a background refresh runs (LoadMetrics's
+// STALE-WITHIN-CEILING state). It exists because nothing keeps the cache
+// warm between requests — its age tracks the gap since the last REQUEST,
+// not a timer — so without a ceiling, the first request after a quiet
+// stretch would serve data that is arbitrarily old (a healthy box's actual
+// reporting gap, PLUS however long nobody happened to ask), and a
+// persistent bucket outage would let LoadMetrics return stale data with a
+// nil error forever, since a background sweep's failure is only logged (see
+// startBackgroundSweep) and nothing else ever revisits it. Past this
+// ceiling, LoadMetrics falls through to the same blocking single-flight
+// path the COLD case uses — costing on the order of a second now that
+// sweepBox parallelises its GETs, rather than the several seconds a
+// sequential sweep used to cost — in exchange for two things a stale answer
+// can't give: data no older than one fresh sweep, and a sweep error
+// actually reaching a caller instead of vanishing into a log line. No
+// separate failure backoff is needed on top of this: a sweep that keeps
+// failing never advances c.at, so the cache's age crosses the ceiling on its
+// own within a couple of minutes regardless of how long the outage lasts.
+const metricsStaleServeCeiling = 2 * time.Minute
+
// metricsSweepHours is how far back LoadMetrics reads raw chunks: the last
// 48 UTC hours, days/ rollups untouched (a later task's job).
const metricsSweepHours = 48
// metricsFreshWindow is how old a box's newest sample may be before its
-// readings stop being anyone's CURRENT figure. The agent samples once a
-// minute, so five minutes is five consecutively missed samples — past any
-// normal hiccup, and the point at which a box has plainly stopped
-// publishing. It matters because the 48h sweep keeps serving a dead box's
-// final sample for two days: without this bound an instance that vanished
-// on Monday still reads "11% / 1.0 KB, current" on Wednesday. Owners get the
-// no-recent-readings state instead (they have no timestamp on the page to
-// judge staleness for themselves; the operator dashboard prints "Latest
-// <time>" and so is deliberately left unbounded).
-const metricsFreshWindow = 5 * time.Minute
+// readings stop being anyone's CURRENT figure. The value is derived, not
+// chosen freehand, from three terms:
+//
+// - Flush cadence, 5 minutes — a FLOOR, not an exact figure: metricsLoop
+// sleeps 60s per tick and does per-tick work on top of that (alertTick
+// every tick, metricsFlush's own bucket writes every 5th), so the actual
+// gap between flushes only ever drifts upward from 5:00, never down.
+// - metricsStaleServeCeiling, 2 minutes — the oldest a cache
+// LoadMetrics hands back under stale-while-revalidate may be, i.e. how
+// far behind the true "latest flush" a box.At a caller sees can sit even
+// with a store that is never slow.
+// - One more minute of slack, covering both the flush-cadence drift above
+// and the ≤metricsSweepTimeout (15s) a background refresh can still be
+// mid-flight for when its result gets read.
+//
+// 5 + 2 + 1 = 8 minutes. This window must exceed that sum or a perfectly
+// healthy, perfectly reachable box flaps between reporting and
+// no-recent-readings for the tail of every cycle.
+//
+// It still has to do the job it always did on top of that: the 48h sweep
+// keeps serving a dead box's final sample for two days, so without SOME
+// bound an instance that vanished on Monday still reads "11% / 1.0 KB,
+// current" on Wednesday. Owners get the no-recent-readings state instead
+// (they have no timestamp on the page to judge staleness for themselves; the
+// operator dashboard prints "Latest <time>" and so is deliberately left
+// unbounded).
+//
+// What this window does NOT promise: it does not survive even ONE missed
+// flush — miss a single 5-minute flush and the gap to the NEXT successful
+// one is already ~10 minutes, past this window, and the box drops out
+// exactly as it would have before this fix. That was true of the old 5-minute window too; widening
+// far enough to also cover a missed flush would mean a truly dead box's
+// stale reading survives even longer before it stops being anyone's
+// "current" figure — not a trade worth making for one skipped beat.
+const metricsFreshWindow = 8 * time.Minute
// metricsBoxesPrefix is where a box publishes its raw chunks and day
// rollups (internal/metrics.RawKey / DayKey); box discovery lists this
@@ -108,15 +161,29 @@ const (
const metricsSweepTimeout = 15 * time.Second
// metricsCache holds the result of the last successful bucket sweep. A
-// failed sweep never reaches here — LoadMetrics returns the error to its
-// caller and leaves the previous (possibly stale-but-valid, possibly zero)
-// cache exactly as it was.
+// failed sweep never reaches here — a failed BLOCKING sweep (c.at still
+// zero, OR the cache is stale past metricsStaleServeCeiling) returns the
+// error to its caller and leaves the cache exactly as it was; a failed
+// BACKGROUND sweep (c.at set and still within the ceiling) is only logged,
+// since nobody is waiting on it and the stale cache it leaves behind is
+// still valid data, just not fresher.
+//
+// No separate failure backoff sits on top of the ceiling: a sweep that keeps
+// failing never advances c.at, so the cache's age crosses
+// metricsStaleServeCeiling on its own within a couple of minutes regardless
+// of how long an outage lasts, and every caller after that lands back on the
+// blocking path where the failure is visible again — see
+// metricsStaleServeCeiling's own comment.
//
// mu guards the fields and is deliberately NOT held across the sweep's bucket
// IO; sweeping/done are what serialise the IO instead, so that a cold sweep
// delays only the callers that actually need its result rather than every
-// caller that touches the cache. sweeping is true while one goroutine is out
-// at the bucket, and done is the channel it closes when it comes back.
+// caller that touches the cache. sweeping is true while one goroutine (either
+// a blocking caller's own, or the detached refresh goroutine LoadMetrics
+// launches for stale-while-revalidate) is out at the bucket, and done is the
+// channel it closes when it comes back — the SAME channel and flag serve
+// both shapes of sweep, so a caller arriving mid-refresh, whichever kind
+// started it, coalesces onto it rather than starting a second one.
type metricsCache struct {
mu sync.Mutex
at time.Time
@@ -128,36 +195,95 @@ type metricsCache struct {
// LoadMetrics reads every box under metrics/boxes/, the raw chunks of the
// last 48 hours (missing hours skipped silently), and alerts/boxes/*.json,
-// caching the result in-process for 60s so concurrent callers — the
-// dashboard page and every owner's app-metrics page — share one sweep.
+// caching the result in-process so concurrent callers — the dashboard page
+// and every owner's app-metrics page — usually share one sweep.
+//
+// Three cache states:
//
-// Two properties matter beyond the cache itself, both learned from the shape
-// this replaced (mutex held across the whole sweep, on the request's own
-// context):
+// - FRESH (c.at within metricsCacheTTL): the cached answer, immediately,
+// no bucket touched at all.
+// - STALE-WITHIN-CEILING (c.at is set, older than metricsCacheTTL, but no
+// older than metricsStaleServeCeiling): the cached answer is STILL
+// returned immediately — stale-while-revalidate — and, if no sweep is
+// already in flight, a fresh one is kicked off on a detached goroutine
+// (startBackgroundSweep) for the NEXT caller to benefit from. This is
+// the fix for the sweep's own cost: sweepBox's 48 hourly GETs for a
+// single box were measured at ~2.5s run sequentially, and that cost
+// used to land inline on whichever request's bad luck it was to arrive
+// right after a cache expiry — the dashboard page and every tenant's
+// app page alike.
+// - COLD (c.at is zero — nothing has ever swept successfully) or STALE
+// PAST THE CEILING (c.at is set but older than
+// metricsStaleServeCeiling — nothing has kept the cache warm for a
+// while, e.g. a quiet console between requests, or every sweep since
+// has been failing): the ORIGINAL blocking behaviour. There is either no
+// servable stale answer (COLD) or the one that exists is too old to
+// hand out one more time (STALE-PAST-CEILING) — either way this caller
+// (or whichever caller's sweep is already in flight) waits for a real
+// sweep and gets ITS error if the sweep fails. The ceiling is what keeps
+// a persistent bucket outage visible: without it, stale-while-revalidate
+// alone would let LoadMetrics return stale data with a nil error
+// forever regardless of how long the store stays down, because a
+// background sweep's failure is only logged (see startBackgroundSweep)
+// — crossing the ceiling forces the next caller back onto the path that
+// actually surfaces the error. Blocking here now costs on the order of
+// a second for a fleet this size rather than the several seconds it
+// cost before sweepBox parallelised its GETs, which is what makes
+// paying that cost here — instead of serving indefinitely-stale data —
+// an acceptable trade.
+//
+// Two properties matter beyond the cache states themselves, both learned
+// from the shape this replaced (mutex held across the whole sweep, on the
+// request's own context):
//
// - The cache mutex is never held across bucket IO. Only the goroutine that
// actually sweeps is slow; everyone else either takes the cached answer
-// immediately or waits on the in-flight sweep's channel, and neither
-// blocks a caller whose cache is fresh. handleApp calls this on every
-// tenant's app page, so a cold sweep must not become a queue every page
-// on the console stands in.
-// - The sweep runs on its own context — context.WithoutCancel of the
-// caller's, plus metricsSweepTimeout — never the request's. A request
-// cancelled mid-sweep (a reload, a navigate-away) would otherwise abort
-// the shared sweep AND, since a failed sweep is never cached, leave the
-// next request to start from scratch: reload impatiently enough and the
-// cache never populates at all. The caller's own context still governs
-// WAITING (a cancelled waiter gives up rather than hanging), it just no
-// longer governs the sweep.
+// immediately or waits on the in-flight sweep's channel (only the
+// blocking case waits at all), and neither blocks a caller whose cache
+// is fresh or stale-but-servable. handleApp calls this on every tenant's
+// app page, so a cold sweep must not become a queue every page on the
+// console stands in.
+// - Every sweep — blocking or background — runs on a context detached from
+// any particular caller's: context.WithoutCancel of whatever was handed
+// in (context.Background() for the background case, since there is no
+// caller to derive from), plus metricsSweepTimeout. A request cancelled
+// mid-sweep (a reload, a navigate-away) would otherwise abort a sweep
+// other callers are waiting on AND, since a failed blocking sweep is
+// never cached, leave the next request to start from scratch: reload
+// impatiently enough and the cache never populates at all. The caller's
+// own context still governs WAITING in the blocking case (a cancelled
+// waiter gives up rather than hanging), it just never governs the sweep
+// itself.
func (s *Server) LoadMetrics(ctx context.Context) ([]BoxMetricsView, []AlertRow, error) {
c := &s.metricsCache
for {
c.mu.Lock()
if !c.at.IsZero() && time.Since(c.at) < metricsCacheTTL {
+ // FRESH.
boxes, alerts := c.boxes, c.alerts
c.mu.Unlock()
return boxes, alerts, nil
}
+ if !c.at.IsZero() && time.Since(c.at) <= metricsStaleServeCeiling {
+ // STALE-WITHIN-CEILING: hand back what we have right now — it
+ // is still a real sweep's result, just older than
+ // metricsCacheTTL — and let a refresh happen off to the side
+ // rather than making this caller pay for it. If one is already
+ // running (background OR, in principle, another blocking
+ // caller's), don't start a second; this caller just doesn't
+ // wait for it either way.
+ boxes, alerts := c.boxes, c.alerts
+ if !c.sweeping {
+ s.startBackgroundSweep(c)
+ }
+ c.mu.Unlock()
+ return boxes, alerts, nil
+ }
+ // COLD, or STALE-PAST-THE-CEILING: no servable stale answer exists
+ // (COLD) or the one that does is too old to hand out again
+ // (past metricsStaleServeCeiling) — this caller must wait for real
+ // data and see its error if the sweep fails, exactly as the
+ // original blocking behaviour did.
if c.sweeping {
// Someone is already at the bucket: wait for their result
// rather than duplicating the sweep, then re-check (their
@@ -211,6 +337,59 @@ func (s *Server) LoadMetrics(ctx context.Context) ([]BoxMetricsView, []AlertRow,
}
}
+// startBackgroundSweep launches sweepAll on its own goroutine to refresh a
+// cache that has swept successfully at least once and is still within
+// metricsStaleServeCeiling, without making any caller wait on it — see
+// LoadMetrics's STALE-WITHIN-CEILING case. "Swept successfully at least
+// once" is exactly c.at being set: an empty-bucket console (c.at set,
+// c.boxes nil) takes this path just like a populated one, since nil boxes
+// is itself a complete, valid swept result, not an absent one. Must be
+// called with c.mu held: it sets c.sweeping and installs c.done itself, the
+// very same fields the blocking path uses, so a caller arriving while this
+// is in flight (whether via the STALE-WITHIN-CEILING branch or the blocking
+// one) coalesces onto it instead of starting a second sweep. sweepAll is
+// handed context.Background() rather than any request's
+// context because there IS no request behind this goroutine — it already
+// detaches internally via context.WithoutCancel and bounds itself with
+// metricsSweepTimeout, so Background() costs nothing extra.
+//
+// Unlike the blocking path, this goroutine has no net/http per-connection
+// recover sitting above it — an unrecovered panic here would take the whole
+// process down, not just one request — so it recovers explicitly. A failed
+// or panicked background sweep is only logged: no caller is waiting on its
+// error, and the stale cache it leaves untouched is still valid data.
+func (s *Server) startBackgroundSweep(c *metricsCache) {
+ c.sweeping = true
+ c.done = make(chan struct{})
+ done := c.done
+ go func() {
+ var (
+ boxes []BoxMetricsView
+ alerts []AlertRow
+ err = errors.New("console: background metrics sweep did not complete")
+ )
+ defer func() {
+ rec := recover()
+ c.mu.Lock()
+ if err == nil {
+ c.at = time.Now()
+ c.boxes = boxes
+ c.alerts = alerts
+ }
+ c.sweeping = false
+ close(done)
+ c.mu.Unlock()
+ switch {
+ case rec != nil:
+ log.Printf("console: background metrics sweep panic, cache left at its last good sweep: %v", rec)
+ case err != nil:
+ log.Printf("console: background metrics sweep: %v", err)
+ }
+ }()
+ boxes, alerts, err = s.sweepAll(context.Background())
+ }()
+}
+
// sweepAll is the bucket half of LoadMetrics: discovery, every box's 48h raw
// sweep (metricsSweepBoxes of them at a time), then the alerts pass, on a
// context detached from the caller's (WithoutCancel) and bounded by
@@ -501,8 +680,9 @@ type InstanceMetrics struct {
// is false, where it decides what the absence of readings may claim: a
// hibernating instance is presumed asleep, anything else is NOT — it may
// be serving right now from outside per-instance accounting (a bespoke
- // unit like keymail-relay.service, or an exec child of the agent), and
- // the template must not assert a sleep state nobody observed.
+ // unit like keymail-relay.service, or an exec child on a box whose agent
+ // has no delegated subtree yet), and the template must not assert a
+ // sleep state nobody observed.
Hibernate bool
}
@@ -515,26 +695,33 @@ type InstanceMetrics struct {
// was unreadable, which renders as the usual em-dash.)
func (m InstanceMetrics) Reporting() bool { return m.Box != "" }
-// OwnerAppMetrics filters the cached 48h sweep down to EXACTLY the units
-// carlos-app@<host>.service for the given hosts. It returns unit-level rows
-// only — no host totals, no other units, no alert data. The allowlist is the
-// hosts slice the caller derived from the app's OWN routes; nothing else can
-// enter the view.
+// OwnerAppMetrics filters the cached 48h sweep down to EXACTLY the given
+// hosts' own per-instance readings — carlos-app@<host>.service for a
+// unit-backed tenant, carlos-exec@<host> for an exec-backed one. It returns
+// unit-level rows only — no host totals, no other units, no alert data. The
+// allowlist is the hosts slice the caller derived from the app's OWN routes;
+// nothing else can enter the view.
//
// Every allowlisted host gets exactly one row, in host order, whether or not
// the sweep saw it: a host with no unit data is an UNKNOWN row (nil
// readings, Reporting false), never zeros and never silently dropped — an
// owner whose instance is asleep must still see it listed.
func (s *Server) OwnerAppMetrics(ctx context.Context, hosts []string) ([]InstanceMetrics, error) {
- // The allowlist, keyed by the one unit name each host can legally match.
+ // The allowlist, keyed by the unit names each host can legally match.
// Matching is exact-string on this map: a unit the caller did not ask
// for cannot be reached by any path below.
- want := make(map[string]string, len(hosts))
+ want := make(map[string]string, 2*len(hosts))
for _, h := range hosts {
if h == "" {
continue
}
+ // Two legal keys per host: the systemd unit a unit-backed tenant
+ // reports under, and the delegated exec cgroup an exec-backed one
+ // does. One route, one backing — a host can't legally report under
+ // both at once; if both ever appear in a sweep, the freshest-box
+ // rule below picks deterministically, no special case.
want[activator.UnitName(h)] = h
+ want[activator.ExecName(h)] = h
}
if len(want) == 0 {
return nil, nil
@@ -578,12 +765,21 @@ func (s *Server) OwnerAppMetrics(ctx context.Context, hosts []string) ([]Instanc
}
}
- rows := make([]InstanceMetrics, 0, len(want))
- for _, host := range want {
- if row, ok := found[host]; ok {
+ // Iterate the callers' hosts, not want's values: want now carries TWO
+ // keys per host, and "every allowlisted host gets exactly one row" is a
+ // promise about hosts, not keys. seen collapses duplicate input hosts,
+ // which the old unique-keyed map did implicitly.
+ rows := make([]InstanceMetrics, 0, len(hosts))
+ seen := make(map[string]bool, len(hosts))
+ for _, h := range hosts {
+ if h == "" || seen[h] {
+ continue
+ }
+ seen[h] = true
+ if row, ok := found[h]; ok {
rows = append(rows, row)
} else {
- rows = append(rows, InstanceMetrics{Host: host})
+ rows = append(rows, InstanceMetrics{Host: h})
}
}
sort.Slice(rows, func(i, j int) bool { return rows[i].Host < rows[j].Host })
diff --git a/internal/console/metricsview_test.go b/internal/console/metricsview_test.go
index 380b74eba..b2e260795 100644
--- a/internal/console/metricsview_test.go
+++ b/internal/console/metricsview_test.go
@@ -8,6 +8,7 @@ import (
"strconv"
"strings"
"sync"
+ "sync/atomic"
"testing"
"time"
@@ -303,6 +304,551 @@ func TestLoadMetricsConcurrentColdCallers(t *testing.T) {
}
}
+// writeBoxSample (re)publishes box's current-hour raw chunk with a single
+// host CPU reading — metrics.WriteRaw's own "replace whatever is there"
+// semantics (see chunk.go) let a later call simulate the agent's next flush
+// landing with a different reading, at the same key.
+func writeBoxSample(t *testing.T, rel release.Store, box string, cpu float64) {
+ t.Helper()
+ at := time.Now().UTC().Add(-time.Minute)
+ if err := metrics.WriteRaw(context.Background(), rel, metrics.RawChunk{
+ V: 1, Box: box, Hour: metrics.HourOf(at),
+ Samples: []metrics.Sample{{At: at, Host: metrics.HostSample{CPUFrac: p(cpu)}}},
+ }); err != nil {
+ t.Fatal(err)
+ }
+}
+
+// slowCountingStore wraps a release.Store, counting List/Get calls and
+// optionally sleeping getDelay before each Get. It is the fixture the
+// stale-while-revalidate tests use to (a) prove a background sweep never
+// makes its trigger's caller wait for it and (b) count how many sweeps
+// actually ran.
+type slowCountingStore struct {
+ release.Store
+ getDelay time.Duration
+ lists atomic.Int64
+ gets atomic.Int64
+ // listGate, when non-nil, is received from (blocking) before every List
+ // call proceeds to the underlying store. discoverBoxes's List is the
+ // very FIRST store operation any sweep makes (before any Get), so
+ // gating it deterministically freezes a background sweep before it
+ // touches the store at all — letting a test assert "the synchronous
+ // caller performed zero store reads" without racing against how fast
+ // the background goroutine happens to get scheduled. Set only from the
+ // test goroutine strictly before the call that triggers the sweep, so
+ // there is a happens-before edge (via the goroutine-creation rule) to
+ // whichever goroutine reads it — never written concurrently with a read.
+ // A CLOSED gate never blocks again (a receive on a closed channel
+ // returns immediately), so once released it stays released for any
+ // later List call in the same test, gated or not.
+ listGate chan struct{}
+ // listErr, when set, is what every List call returns (after clearing
+ // listGate, if any) instead of touching the underlying store — the
+ // fixture for simulating an outage that starts exactly when a gated
+ // sweep is released, so both that sweep AND whatever sweep a coalesced
+ // waiter runs on its own afterward see the same failure.
+ listErr error
+}
+
+func (s *slowCountingStore) List(ctx context.Context, prefix string) ([]string, error) {
+ s.lists.Add(1)
+ if s.listGate != nil {
+ <-s.listGate
+ }
+ if s.listErr != nil {
+ return nil, s.listErr
+ }
+ return s.Store.List(ctx, prefix)
+}
+
+func (s *slowCountingStore) Get(ctx context.Context, key string) ([]byte, string, error) {
+ s.gets.Add(1)
+ if s.getDelay > 0 {
+ time.Sleep(s.getDelay)
+ }
+ return s.Store.Get(ctx, key)
+}
+
+// newSlowMetricsFixture is newMetricsFixture with the server reading through
+// a slowCountingStore, for tests that need to observe how many sweeps ran or
+// need a sweep slow enough to prove a caller didn't wait on it.
+func newSlowMetricsFixture(t *testing.T, getDelay time.Duration) (*Server, *slowCountingStore) {
+ t.Helper()
+ st, err := Open(filepath.Join(t.TempDir(), "console.db"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { st.Close() })
+ dir := release.NewDirStore(filepath.Join(t.TempDir(), "bucket"))
+ slow := &slowCountingStore{Store: dir, getDelay: getDelay}
+ srv, err := NewServer(Config{Releases: slow}, st)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return srv, slow
+}
+
+// waitUntilNotSweeping polls the cache's sweeping flag directly (same
+// package, no exported seam needed) until a background sweep finishes or
+// the deadline passes.
+func waitUntilNotSweeping(t *testing.T, srv *Server, timeout time.Duration) {
+ t.Helper()
+ deadline := time.Now().Add(timeout)
+ for {
+ srv.metricsCache.mu.Lock()
+ sweeping := srv.metricsCache.sweeping
+ srv.metricsCache.mu.Unlock()
+ if !sweeping {
+ return
+ }
+ if time.Now().After(deadline) {
+ t.Fatalf("background sweep did not finish within %v", timeout)
+ }
+ time.Sleep(10 * time.Millisecond)
+ }
+}
+
+// metricsStaleTestBackdate is how far the tests below wind c.at back to
+// simulate an expired-but-servable cache: comfortably past metricsCacheTTL
+// (60s) so the cache is definitely STALE, and comfortably short of
+// metricsStaleServeCeiling (2min) so it's just as definitely still within
+// serving range — 30s of margin on each side, generous enough that neither
+// boundary is at risk from scheduling jitter.
+const metricsStaleTestBackdate = metricsCacheTTL + 30*time.Second
+
+// TestLoadMetricsStaleWhileRevalidate is fix B's core behaviour change: an
+// expired-but-still-within-ceiling cache is no longer a reason to make the
+// caller wait. The first call after expiry gets the OLD data back
+// immediately — proven by a zero Get-count delta on the store, not a wall-
+// clock guess — while a sweep runs in the background; once that sweep
+// lands, a LATER call sees the NEW data, still without ever blocking.
+func TestLoadMetricsStaleWhileRevalidate(t *testing.T) {
+ const getDelay = 80 * time.Millisecond
+ srv, store := newSlowMetricsFixture(t, getDelay)
+ ctx := context.Background()
+
+ writeBoxSample(t, store, "box-a", 0.5)
+ boxes, _, err := srv.LoadMetrics(ctx)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(boxes) != 1 || boxes[0].CPU == nil || *boxes[0].CPU != 0.5 {
+ t.Fatalf("initial cold sweep = %+v, want box-a at 0.5", boxes)
+ }
+
+ // Expire the cache (within the ceiling) and change what the store
+ // holds. From here on a FRESH sweep would see 0.9 — the immediate call
+ // below must still answer with the cache's last-known 0.5, proving it
+ // didn't wait for a fresh read.
+ srv.metricsCache.mu.Lock()
+ srv.metricsCache.at = time.Now().Add(-metricsStaleTestBackdate)
+ srv.metricsCache.mu.Unlock()
+ writeBoxSample(t, store, "box-a", 0.9)
+
+ // Freeze the background sweep this call is about to launch at its very
+ // first store operation (discoverBoxes's List, before any Get) — this
+ // makes the zero-Gets assertion below deterministic rather than a race
+ // against the background goroutine's scheduling.
+ store.listGate = make(chan struct{})
+ getsBefore := store.gets.Load()
+ boxes, _, err = srv.LoadMetrics(ctx)
+ getsAfter := store.gets.Load()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(boxes) != 1 || boxes[0].CPU == nil || *boxes[0].CPU != 0.5 {
+ t.Fatalf("stale call = %+v, want the OLD 0.5 (a background refresh must not be awaited)", boxes)
+ }
+ if getsAfter != getsBefore {
+ t.Fatalf("stale call performed %d store Gets, want 0 — it must return the cache without touching the store at all", getsAfter-getsBefore)
+ }
+ close(store.listGate) // release the frozen background sweep to finish
+
+ waitUntilNotSweeping(t, srv, 5*time.Second)
+
+ boxes, _, err = srv.LoadMetrics(ctx)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(boxes) != 1 || boxes[0].CPU == nil || *boxes[0].CPU != 0.9 {
+ t.Fatalf("after the background sweep landed = %+v, want the NEW 0.9", boxes)
+ }
+}
+
+// TestLoadMetricsStaleCacheSingleFlight: several callers arriving on an
+// EXPIRED-BUT-WITHIN-CEILING (not cold) cache at once must trigger exactly
+// one background sweep between them, never one each and never zero. The
+// baseline — however many List calls one full sweepAll makes (discoverBoxes
+// plus the alerts pass) — is measured from the initial cold sweep rather
+// than hard-coded, so this test doesn't have to know sweepAll's internals to
+// prove "exactly one more sweep happened."
+func TestLoadMetricsStaleCacheSingleFlight(t *testing.T) {
+ srv, store := newSlowMetricsFixture(t, 30*time.Millisecond)
+ ctx := context.Background()
+ writeBoxSample(t, store, "box-a", 0.5)
+
+ // Measured as a delta across just this call (not the cumulative total),
+ // so any List traffic from fixture/server construction before this
+ // point doesn't get folded into the "one sweep" baseline below.
+ beforeCold := store.lists.Load()
+ if _, _, err := srv.LoadMetrics(ctx); err != nil {
+ t.Fatal(err)
+ }
+ oneSweepLists := store.lists.Load() - beforeCold // however many List calls one sweepAll makes
+
+ srv.metricsCache.mu.Lock()
+ srv.metricsCache.at = time.Now().Add(-metricsStaleTestBackdate)
+ srv.metricsCache.mu.Unlock()
+ before := store.lists.Load()
+
+ const callers = 8
+ var wg sync.WaitGroup
+ for i := 0; i < callers; i++ {
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+ if _, _, err := srv.LoadMetrics(ctx); err != nil {
+ t.Error(err)
+ }
+ }()
+ }
+ wg.Wait()
+
+ waitUntilNotSweeping(t, srv, 5*time.Second)
+
+ if got := store.lists.Load() - before; got != oneSweepLists {
+ t.Fatalf("List calls triggered by %d concurrent callers on a stale cache = %d, want exactly %d (one sweep, shared)", callers, got, oneSweepLists)
+ }
+}
+
+// panicListStore panics on every List call — the fixture for proving a
+// panicking BACKGROUND sweep is recovered (startBackgroundSweep's explicit
+// recover — a detached goroutine has no net/http per-connection recover
+// sitting above it the way a request's own goroutine does) rather than
+// taking the whole process down, and that the cache is left usable
+// afterward rather than wedged.
+type panicListStore struct {
+ release.Store
+}
+
+func (panicListStore) List(context.Context, string) ([]string, error) {
+ panic("simulated bucket panic")
+}
+
+// TestLoadMetricsBackgroundSweepPanicRecovers: a background sweep
+// (triggered by a stale-but-within-ceiling cache) that panics must not crash
+// the test process, must leave the stale cache intact for its trigger's
+// caller (no error), must clear sweeping so nothing is wedged, and must not
+// prevent a later, healthy sweep from succeeding.
+func TestLoadMetricsBackgroundSweepPanicRecovers(t *testing.T) {
+ srv, rel := newMetricsFixture(t)
+ ctx := context.Background()
+ writeBoxSample(t, rel, "box-a", 0.5)
+
+ if _, _, err := srv.LoadMetrics(ctx); err != nil {
+ t.Fatal(err)
+ }
+
+ srv.metricsCache.mu.Lock()
+ srv.metricsCache.at = time.Now().Add(-metricsStaleTestBackdate)
+ srv.metricsCache.mu.Unlock()
+ srv.cfg.Releases = panicListStore{Store: rel}
+
+ boxes, _, err := srv.LoadMetrics(ctx)
+ if err != nil {
+ t.Fatalf("stale call with a panicking background sweep returned an error: %v", err)
+ }
+ if len(boxes) != 1 || boxes[0].CPU == nil || *boxes[0].CPU != 0.5 {
+ t.Fatalf("stale call = %+v, want the OLD cached data despite the panic", boxes)
+ }
+
+ waitUntilNotSweeping(t, srv, 5*time.Second)
+
+ srv.metricsCache.mu.Lock()
+ sweeping := srv.metricsCache.sweeping
+ srv.metricsCache.mu.Unlock()
+ if sweeping {
+ t.Fatal("cache left wedged (sweeping still true) after a panicking background sweep")
+ }
+
+ // Nothing is permanently wedged: restore a healthy store and force the
+ // COLD path (c.at zeroed) for a clean, unambiguous assertion that a
+ // later sweep still works.
+ srv.cfg.Releases = rel
+ srv.metricsCache.mu.Lock()
+ srv.metricsCache.at = time.Time{}
+ srv.metricsCache.mu.Unlock()
+
+ boxes, _, err = srv.LoadMetrics(ctx)
+ if err != nil {
+ t.Fatalf("sweep after a prior background panic failed: %v", err)
+ }
+ if len(boxes) != 1 || boxes[0].CPU == nil || *boxes[0].CPU != 0.5 {
+ t.Fatalf("recovered sweep = %+v, want box-a at 0.5", boxes)
+ }
+}
+
+// errListStore fails every List call with a fixed error — the fixture for
+// proving a background sweep's failure is only logged while its trigger's
+// caller sees no error and the stale cache survives (within the ceiling),
+// and that the SAME failure surfaces to a blocking caller once the cache
+// ages past metricsStaleServeCeiling — restoring outage visibility rather
+// than serving indefinitely-stale data with no error forever.
+type errListStore struct {
+ release.Store
+ err error
+}
+
+func (e errListStore) List(context.Context, string) ([]string, error) {
+ return nil, e.err
+}
+
+// TestLoadMetricsBackgroundSweepFailureThenCeilingSurfacesError covers both
+// halves of that contract against the one failing store.
+func TestLoadMetricsBackgroundSweepFailureThenCeilingSurfacesError(t *testing.T) {
+ srv, rel := newMetricsFixture(t)
+ ctx := context.Background()
+ writeBoxSample(t, rel, "box-a", 0.5)
+
+ if _, _, err := srv.LoadMetrics(ctx); err != nil {
+ t.Fatal(err)
+ }
+
+ wantErr := errors.New("bucket had a bad day")
+ srv.cfg.Releases = errListStore{Store: rel, err: wantErr}
+
+ // Within the ceiling: the stale cache survives, the failing background
+ // sweep is only logged, and this caller sees no error at all.
+ srv.metricsCache.mu.Lock()
+ srv.metricsCache.at = time.Now().Add(-metricsStaleTestBackdate)
+ srv.metricsCache.mu.Unlock()
+
+ boxes, _, err := srv.LoadMetrics(ctx)
+ if err != nil {
+ t.Fatalf("stale call with a failing background sweep returned an error: %v", err)
+ }
+ if len(boxes) != 1 || boxes[0].CPU == nil || *boxes[0].CPU != 0.5 {
+ t.Fatalf("stale call = %+v, want the OLD cached data", boxes)
+ }
+
+ waitUntilNotSweeping(t, srv, 5*time.Second)
+ srv.metricsCache.mu.Lock()
+ sweeping := srv.metricsCache.sweeping
+ srv.metricsCache.mu.Unlock()
+ if sweeping {
+ t.Fatal("cache left wedged after a failing background sweep")
+ }
+
+ // Past the ceiling: the SAME failing store must now surface its error
+ // to a blocking caller instead of handing back stale data forever.
+ srv.metricsCache.mu.Lock()
+ srv.metricsCache.at = time.Now().Add(-(metricsStaleServeCeiling + time.Second))
+ srv.metricsCache.mu.Unlock()
+
+ if _, _, err := srv.LoadMetrics(ctx); err == nil {
+ t.Fatal("call past the ceiling with a failing store returned no error")
+ } else if !errors.Is(err, wantErr) {
+ t.Fatalf("error = %v, want it to wrap %v", err, wantErr)
+ }
+}
+
+// TestLoadMetricsPastCeilingBlocksForFreshData: a cache older than
+// metricsStaleServeCeiling is no longer eligible for stale-while-revalidate
+// even with a perfectly healthy store — the caller blocks on a synchronous
+// sweep and gets genuinely fresh data, proven by a non-zero Get-count delta
+// (the sweep ran INSIDE this call, not off to the side) rather than a
+// timing guess.
+func TestLoadMetricsPastCeilingBlocksForFreshData(t *testing.T) {
+ srv, store := newSlowMetricsFixture(t, 0)
+ ctx := context.Background()
+ writeBoxSample(t, store, "box-a", 0.5)
+
+ if _, _, err := srv.LoadMetrics(ctx); err != nil {
+ t.Fatal(err)
+ }
+
+ writeBoxSample(t, store, "box-a", 0.9)
+ srv.metricsCache.mu.Lock()
+ srv.metricsCache.at = time.Now().Add(-(metricsStaleServeCeiling + time.Second))
+ srv.metricsCache.mu.Unlock()
+
+ getsBefore := store.gets.Load()
+ boxes, _, err := srv.LoadMetrics(ctx)
+ getsAfter := store.gets.Load()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if getsAfter == getsBefore {
+ t.Fatal("call past the ceiling performed no store Gets — it must sweep synchronously, not serve stale data")
+ }
+ if len(boxes) != 1 || boxes[0].CPU == nil || *boxes[0].CPU != 0.9 {
+ t.Fatalf("boxes = %+v, want the FRESH 0.9 — this call must have swept, not served the stale 0.5", boxes)
+ }
+}
+
+// TestLoadMetricsPastCeilingWaiterCoalescesOntoBackgroundSweep pins the one
+// state-machine transition the other tests only exercise as a side effect:
+// a caller landing in the blocking COLD-or-STALE-PAST-CEILING branch while a
+// BACKGROUND sweep (started earlier by a within-ceiling stale caller) is
+// already in flight. LoadMetrics's blocking branch waits on c.sweeping/
+// c.done regardless of which kind of sweep set them — this proves that
+// holds for a background sweep specifically, not just another blocking
+// caller's, by gating the background sweep at its very first store call so
+// the two callers' timing is deterministic rather than a race.
+func TestLoadMetricsPastCeilingWaiterCoalescesOntoBackgroundSweep(t *testing.T) {
+ srv, store := newSlowMetricsFixture(t, 0)
+ ctx := context.Background()
+ writeBoxSample(t, store, "box-a", 0.5)
+
+ // Baseline: however many List calls one full sweepAll makes, measured
+ // the same way the single-flight test does.
+ beforeCold := store.lists.Load()
+ if _, _, err := srv.LoadMetrics(ctx); err != nil {
+ t.Fatal(err)
+ }
+ oneSweepLists := store.lists.Load() - beforeCold
+
+ listsBeforeTrigger := store.lists.Load()
+
+ // Gate the NEXT sweep at its first store call, then trigger it via a
+ // within-ceiling stale call — this is startBackgroundSweep's path, not
+ // the blocking one.
+ store.listGate = make(chan struct{})
+ srv.metricsCache.mu.Lock()
+ srv.metricsCache.at = time.Now().Add(-metricsStaleTestBackdate)
+ srv.metricsCache.mu.Unlock()
+ writeBoxSample(t, store, "box-a", 0.9) // what the gated sweep will see once released
+
+ if boxes, _, err := srv.LoadMetrics(ctx); err != nil {
+ t.Fatal(err)
+ } else if len(boxes) != 1 || boxes[0].CPU == nil || *boxes[0].CPU != 0.5 {
+ t.Fatalf("triggering call = %+v, want the OLD 0.5 (background sweep just launched, now gated)", boxes)
+ }
+
+ // Age the SAME cache past the ceiling while the background sweep is
+ // still stuck at the gate — exactly the state the waiter below lands in.
+ srv.metricsCache.mu.Lock()
+ srv.metricsCache.at = time.Now().Add(-(metricsStaleServeCeiling + time.Second))
+ srv.metricsCache.mu.Unlock()
+
+ waiterDone := make(chan struct{})
+ var waiterBoxes []BoxMetricsView
+ var waiterErr error
+ go func() {
+ defer close(waiterDone)
+ waiterBoxes, _, waiterErr = srv.LoadMetrics(ctx)
+ }()
+
+ select {
+ case <-waiterDone:
+ t.Fatal("past-ceiling waiter returned before the gated background sweep was released")
+ case <-time.After(200 * time.Millisecond):
+ // still blocked, as it must be — coalesced onto the background
+ // sweep's c.done rather than racing off to start its own.
+ }
+
+ close(store.listGate)
+
+ select {
+ case <-waiterDone:
+ case <-time.After(5 * time.Second):
+ t.Fatal("past-ceiling waiter never returned after the gate was released")
+ }
+ if waiterErr != nil {
+ t.Fatalf("waiter error = %v, want nil", waiterErr)
+ }
+ if len(waiterBoxes) != 1 || waiterBoxes[0].CPU == nil || *waiterBoxes[0].CPU != 0.9 {
+ t.Fatalf("waiter boxes = %+v, want the FRESH 0.9 the background sweep it coalesced onto produced", waiterBoxes)
+ }
+ if got := store.lists.Load() - listsBeforeTrigger; got != oneSweepLists {
+ t.Fatalf("List calls across the whole gated-sweep-plus-waiter span = %d, want exactly %d (one sweep, not duplicated by the waiter)", got, oneSweepLists)
+ }
+ srv.metricsCache.mu.Lock()
+ sweeping := srv.metricsCache.sweeping
+ srv.metricsCache.mu.Unlock()
+ if sweeping {
+ t.Fatal("cache left wedged (sweeping still true) after the waiter returned")
+ }
+}
+
+// TestLoadMetricsPastCeilingWaiterTakesOverFailedBackgroundSweep is the
+// failure half of the same coalescing transition: the background sweep the
+// waiter coalesced onto fails (rather than lands), so the cache stays past
+// the ceiling — the waiter must then take the sweep over ITSELF (the
+// c.sweeping check it started blocked on now reads false) rather than
+// staying blocked or silently giving up, and surface that second attempt's
+// error like any ordinary blocking caller would. Nothing may wedge: the
+// flag clears either way, and a later call against a healthy store must
+// still succeed.
+func TestLoadMetricsPastCeilingWaiterTakesOverFailedBackgroundSweep(t *testing.T) {
+ srv, store := newSlowMetricsFixture(t, 0)
+ ctx := context.Background()
+ writeBoxSample(t, store, "box-a", 0.5)
+
+ if _, _, err := srv.LoadMetrics(ctx); err != nil {
+ t.Fatal(err)
+ }
+
+ store.listGate = make(chan struct{})
+ srv.metricsCache.mu.Lock()
+ srv.metricsCache.at = time.Now().Add(-metricsStaleTestBackdate)
+ srv.metricsCache.mu.Unlock()
+
+ if _, _, err := srv.LoadMetrics(ctx); err != nil {
+ t.Fatal(err)
+ } // stale 0.5 returned, background sweep launched and now gated
+
+ srv.metricsCache.mu.Lock()
+ srv.metricsCache.at = time.Now().Add(-(metricsStaleServeCeiling + time.Second))
+ srv.metricsCache.mu.Unlock()
+
+ waiterDone := make(chan struct{})
+ var waiterErr error
+ go func() {
+ defer close(waiterDone)
+ _, _, waiterErr = srv.LoadMetrics(ctx)
+ }()
+
+ select {
+ case <-waiterDone:
+ t.Fatal("past-ceiling waiter returned before the gated background sweep was released")
+ case <-time.After(200 * time.Millisecond):
+ }
+
+ // The outage starts exactly when the gate opens: the background
+ // sweep's blocked List call, and any later List call (including the
+ // waiter's own follow-up sweep), all see the same error from here on.
+ wantErr := errors.New("bucket had a bad day")
+ store.listErr = wantErr
+ close(store.listGate)
+
+ select {
+ case <-waiterDone:
+ case <-time.After(5 * time.Second):
+ t.Fatal("past-ceiling waiter never returned after the gate was released")
+ }
+ if waiterErr == nil {
+ t.Fatal("waiter that took over a failed background sweep returned no error")
+ }
+ if !errors.Is(waiterErr, wantErr) {
+ t.Fatalf("waiter error = %v, want it to wrap %v", waiterErr, wantErr)
+ }
+
+ waitUntilNotSweeping(t, srv, 5*time.Second)
+
+ // Nothing is permanently wedged: clear the outage and confirm a later
+ // call against the same (now healthy) store still succeeds.
+ store.listErr = nil
+ boxes, _, err := srv.LoadMetrics(ctx)
+ if err != nil {
+ t.Fatalf("sweep after the prior failures still failed: %v", err)
+ }
+ if len(boxes) != 1 || boxes[0].CPU == nil || *boxes[0].CPU != 0.5 {
+ t.Fatalf("recovered sweep = %+v, want box-a at 0.5", boxes)
+ }
+}
+
// errOnKeyStore fails one specific Get with a non-NotExist error and passes
// everything else through — the "bucket is answering, but this object will
// not read" case, which must fail the whole sweep rather than being skipped
@@ -556,6 +1102,63 @@ func TestOwnerAppMetricsFiltersToOwnUnits(t *testing.T) {
}
}
+// TestOwnerAppMetricsSurvivesHealthyFlushCycle pins metricsFreshWindow's
+// derivation (flush cadence floor 5min + stale-serve ceiling 2min + 1min
+// slack = 8 minutes): a healthy agent's newest VISIBLE sample can be as old
+// as flush-cadence-plus-ceiling and still be current, but a sample old
+// enough that NOTHING short of a dead agent explains it must still flap to
+// no-recent-readings. Three ages, three different hosts/boxes (so the
+// "freshest box wins" rule in OwnerAppMetrics can't let one case's fresh
+// reading paper over another's staleness):
+//
+// - 5m30s: the original healthy-worst-case under the OLD (5min-flush-only)
+// math — comfortably inside both the old and new window.
+// - 7m0s: the NEW healthy worst case this fix widens the window to cover
+// — flush cadence's floor (5min) plus the full stale-serve ceiling
+// (2min) a caller's cache may be behind by.
+// - 8m30s: comfortably past the 8-minute window regardless of derivation
+// — must NOT read as current.
+func TestOwnerAppMetricsSurvivesHealthyFlushCycle(t *testing.T) {
+ srv, rel := newMetricsFixture(t)
+ ctx := context.Background()
+
+ write := func(box, host string, age time.Duration) {
+ at := time.Now().UTC().Add(-age)
+ if err := metrics.WriteRaw(ctx, rel, metrics.RawChunk{
+ V: 1, Box: box, Hour: metrics.HourOf(at),
+ Samples: []metrics.Sample{{
+ At: at,
+ Units: map[string]metrics.UnitSample{
+ "carlos-app@" + host + ".service": {CPUFrac: p(0.42), MemBytes: pi64(2048)},
+ },
+ }},
+ }); err != nil {
+ t.Fatal(err)
+ }
+ }
+ write("box-healthy-1", "flush1.example.com", 5*time.Minute+30*time.Second)
+ write("box-healthy-2", "flush2.example.com", 7*time.Minute)
+ write("box-dead", "dead.example.com", 8*time.Minute+30*time.Second)
+
+ rows, err := srv.OwnerAppMetrics(ctx, []string{"flush1.example.com", "flush2.example.com", "dead.example.com"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ byHost := map[string]InstanceMetrics{}
+ for _, r := range rows {
+ byHost[r.Host] = r
+ }
+ if r := byHost["flush1.example.com"]; !r.Reporting() {
+ t.Fatalf("a 5m30s-old sample flapped to no-recent-readings: %+v", r)
+ }
+ if r := byHost["flush2.example.com"]; !r.Reporting() {
+ t.Fatalf("a 7m0s-old sample (flush floor + stale-serve ceiling) flapped to no-recent-readings: %+v", r)
+ }
+ if r := byHost["dead.example.com"]; r.Reporting() {
+ t.Fatalf("an 8m30s-old sample — past the fresh window — still read as current: %+v", r)
+ }
+}
+
// TestSparkPointsInvertsYAndSkipsNil pins the coordinate math the brief
// calls out explicitly: a nil sample contributes no pair at all (not a
// zero), and a higher fraction plots at a SMALLER y (SVG's y axis points
@@ -666,3 +1269,98 @@ func TestSparkPointsDownsampleKeepsNilUnknown(t *testing.T) {
}
}
}
+
+// TestOwnerAppMetricsSeesExecBackedInstances pins the second allowlist key:
+// an exec-backed host reports under carlos-exec@<host> (the delegated
+// cgroup's verbatim name, no .service), and its readings reach the owner
+// row exactly like a unit-backed tenant's. Another tenant's exec cgroup
+// stays invisible — the allowlist boundary holds for the new key form too.
+func TestOwnerAppMetricsSeesExecBackedInstances(t *testing.T) {
+ srv, rel := newMetricsFixture(t)
+ ctx := context.Background()
+ at := time.Now().UTC().Add(-time.Minute)
+ if err := metrics.WriteRaw(ctx, rel, metrics.RawChunk{
+ V: 1, Box: "box-a", Hour: metrics.HourOf(at),
+ Samples: []metrics.Sample{{
+ At: at,
+ Units: map[string]metrics.UnitSample{
+ "carlos-exec@keymail.example.com": {CPUFrac: p(0.22), MemBytes: pi64(2048)},
+ "carlos-exec@other.example.com": {CPUFrac: p(0.44), MemBytes: pi64(4096)},
+ },
+ }},
+ }); err != nil {
+ t.Fatal(err)
+ }
+
+ rows, err := srv.OwnerAppMetrics(ctx, []string{"keymail.example.com"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(rows) != 1 {
+ t.Fatalf("rows = %+v, want exactly the one allowlisted host", rows)
+ }
+ got := rows[0]
+ if got.Host != "keymail.example.com" || !got.Reporting() {
+ t.Fatalf("exec-backed host not reporting: %+v", got)
+ }
+ if got.CPUFrac == nil || *got.CPUFrac != 0.22 {
+ t.Fatalf("CPU = %v, want 0.22", got.CPUFrac)
+ }
+ if got.MemBytes == nil || *got.MemBytes != 2048 {
+ t.Fatalf("mem = %v, want 2048", got.MemBytes)
+ }
+ if got.SparkCPU == "" || got.SparkMem == "" {
+ t.Fatalf("sparklines empty: %+v", got)
+ }
+}
+
+// TestOwnerAppMetricsExecAndUnitKeysNeverDoubleReport pins the spec's
+// no-double-report property: a host can't legally report under both key
+// forms at once (one route, one backing), but if a sweep ever carries both
+// — a backing flip mid-window, two boxes — the existing freshest-box rule
+// picks ONE deterministically. No special case, still exactly one row.
+func TestOwnerAppMetricsExecAndUnitKeysNeverDoubleReport(t *testing.T) {
+ srv, rel := newMetricsFixture(t)
+ ctx := context.Background()
+ fresher := time.Now().UTC().Add(-time.Minute)
+ older := fresher.Add(-2 * time.Minute)
+ // The older box reports the host as a systemd unit, the fresher box as
+ // an exec cgroup — the flip-mid-window shape.
+ if err := metrics.WriteRaw(ctx, rel, metrics.RawChunk{
+ V: 1, Box: "box-old", Hour: metrics.HourOf(older),
+ Samples: []metrics.Sample{{
+ At: older,
+ Units: map[string]metrics.UnitSample{
+ "carlos-app@flip.example.com.service": {CPUFrac: p(0.99), MemBytes: pi64(9999)},
+ },
+ }},
+ }); err != nil {
+ t.Fatal(err)
+ }
+ if err := metrics.WriteRaw(ctx, rel, metrics.RawChunk{
+ V: 1, Box: "box-new", Hour: metrics.HourOf(fresher),
+ Samples: []metrics.Sample{{
+ At: fresher,
+ Units: map[string]metrics.UnitSample{
+ "carlos-exec@flip.example.com": {CPUFrac: p(0.11), MemBytes: pi64(1111)},
+ },
+ }},
+ }); err != nil {
+ t.Fatal(err)
+ }
+
+ rows, err := srv.OwnerAppMetrics(ctx, []string{"flip.example.com"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(rows) != 1 {
+ t.Fatalf("rows = %+v, want exactly one row for the host, never two", rows)
+ }
+ got := rows[0]
+ if got.Box != "box-new" {
+ t.Fatalf("Box = %q, want the freshest box-new: %+v", got.Box, got)
+ }
+ if got.CPUFrac == nil || *got.CPUFrac != 0.11 {
+ t.Fatalf("CPU = %v, want the freshest box's 0.11", got.CPUFrac)
+ }
+}
diff --git a/internal/console/server.go b/internal/console/server.go
index 814be44a1..121523cbb 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -209,9 +209,11 @@ type Server struct {
// direct call so an alertPass assertion needs no keymail server.
sendApp func(ctx context.Context, token, to, subject, body string) (string, error)
// metricsCache holds the result of LoadMetrics's last successful bucket
- // sweep (metricsview.go), shared across callers for metricsCacheTTL so
- // the operator dashboard and every owner's app-metrics page don't each
- // re-sweep 48h of chunks per request.
+ // sweep (metricsview.go). Concurrent callers within metricsCacheTTL share
+ // it outright; callers arriving later still get it immediately (stale-
+ // while-revalidate, up to metricsStaleServeCeiling) so the operator
+ // dashboard and every owner's app-metrics page don't each re-sweep 48h of
+ // chunks per request.
metricsCache metricsCache
// shipSem bounds how many `carlos ship` uploads (apicli.go,
// handleAPIShip) may run AT ONCE, as opposed to s.cli.ship, which
diff --git a/internal/host/agent.go b/internal/host/agent.go
index 41cecde9d..94a784712 100644
--- a/internal/host/agent.go
+++ b/internal/host/agent.go
@@ -92,6 +92,7 @@ import (
"github.com/carlosframework/platform/internal/activator"
"github.com/carlosframework/platform/internal/appconfig"
"github.com/carlosframework/platform/internal/caps"
+ "github.com/carlosframework/platform/internal/cgroup"
"github.com/carlosframework/platform/internal/domains"
"github.com/carlosframework/platform/internal/edge"
"github.com/carlosframework/platform/internal/lease"
@@ -131,6 +132,13 @@ func RunAgent(args []string) {
// hibernation branch.
cache := edge.NewCache(edge.DefaultCacheMaxEntries, edge.DefaultCacheMaxBodyBytes)
+ // Exec-child accounting: claim the cgroup subtree the unit file
+ // delegates to us (Delegate= in carlos-edge.service). Nil when there is
+ // no delegation — an old unit file, a dev run outside systemd, darwin —
+ // in which case children spawn exactly as before and simply have no
+ // per-instance readings.
+ cgm := cgroup.Setup(log.Printf)
+
cfg, err := activator.ConfigFromEnv()
if err != nil {
log.Fatalf("agent: %v", err)
@@ -149,6 +157,7 @@ func RunAgent(args []string) {
DataDir: dd,
Cache: cache,
Materialize: materializeHook(reg, dd),
+ Cgroups: cgm,
},
Unit: &activator.UnitBackend{OptDir: release.OptDir()},
}, reg) // the registry satisfies activator.DueStore directly
@@ -162,7 +171,7 @@ func RunAgent(args []string) {
go statusLoop(reg)
go domainsLoop(reg, peekAcmeCache(args))
go instancesLoop(reg)
- go metricsLoop(svc)
+ go metricsLoop(cgm.Root())
go configSyncLoop(reg, svc, dd)
go configGenLoop(reg, svc, dd)
go socketAdvisoryLoop(reg, svc)
diff --git a/internal/host/host_test.go b/internal/host/host_test.go
index 52c7d1a4e..f7b844a10 100644
--- a/internal/host/host_test.go
+++ b/internal/host/host_test.go
@@ -116,11 +116,24 @@ func TestBootstrapWritesUnitsToStagingDir(t *testing.T) {
"SystemCallFilter=@system-service",
"User=carlos",
"Restart=always",
+ // Delegation is the SOLE mechanism behind exec-instance readings
+ // fleet-wide (exec-child accounting spec): without it the agent's
+ // cgroup manager degrades to nil and every exec-backed tenant goes
+ // blind on the Instances page.
+ "Delegate=cpu memory io pids",
} {
if !strings.Contains(string(edge), want) {
t.Errorf("carlos-edge.service: missing %q", want)
}
}
+ // The counterpart of Delegate=: ProtectControlGroups would mount the
+ // cgroup tree read-only and silently un-arm delegation. Its hardening is
+ // retained by ownership (DAC) instead — see the unit file's comment. A
+ // future "hardening cleanup" that re-adds it must fail here, not on the
+ // fleet.
+ if strings.Contains(string(edge), "ProtectControlGroups=") {
+ t.Error("carlos-edge.service: the ProtectControlGroups= directive is deliberately absent (Delegate= needs the subtree writable); do not re-add it")
+ }
// The tenant socket unit: systemd owns the listener as carlos so the
// edge can connect and tenants can't reach each other (the whole point).
diff --git a/internal/host/metrics_loop.go b/internal/host/metrics_loop.go
index 4d0b3753d..4c7045cec 100644
--- a/internal/host/metrics_loop.go
+++ b/internal/host/metrics_loop.go
@@ -20,7 +20,6 @@ import (
"path/filepath"
"time"
- "github.com/carlosframework/platform/internal/activator"
"github.com/carlosframework/platform/internal/metrics"
"github.com/carlosframework/platform/internal/release"
"github.com/carlosframework/platform/internal/status"
@@ -618,24 +617,14 @@ func httpPostWebhook(url string, body []byte) error {
// realSampleProbes wires sampleProbes to the actual OS/cgroup readers in
// metrics_probes.go and probes.go — the production seam metricsLoop uses,
// never called from a test (host_test.go's convention: tests inject
-// fixtures, never touch /proc or a syscall). svc feeds the units probe the
-// activator's awake exec children (AwakeExecPids); nil — no activator
-// armed, e.g. plain `carlos edge` — keeps the units probe cgroup-only,
-// exactly the pre-#118 behavior, the same nilable-svc convention every
-// other activator-fed loop in this package uses.
-func realSampleProbes(svc *activator.Service) sampleProbes {
- units := realUnitReadings
- if svc != nil {
- units = func() map[string]unitRaw {
- return mergeUnitReadings(realUnitReadings(), execInstanceReadings(svc.AwakeExecPids(), readExecInstanceRaw))
- }
- }
+// fixtures, never touch /proc or a syscall).
+func realSampleProbes(execRoot string) sampleProbes {
return sampleProbes{
procStat: readProcStat,
loadavg: readLoadAvg,
memFrac: readMemFrac,
diskFrac: readDiskFrac,
- units: units,
+ units: func() map[string]unitRaw { return realUnitReadings(execRoot) },
now: time.Now,
}
}
@@ -644,10 +633,13 @@ func realSampleProbes(svc *activator.Service) sampleProbes {
// each one's cgroup usage — the production units func sampleProbes.units
// wraps. Keyed by UNIT NAME (what the console matches against), read from the
// unit's own cgroup DIRECTORY (which for a tenant instance is a level down;
-// see listCarlosUnits).
-func realUnitReadings() map[string]unitRaw {
+// see listCarlosUnits) — plus, when delegation is armed, the per-host
+// exec-child cgroups under execRoot (keyed carlos-exec@<host>; see
+// listExecCgroups).
+func realUnitReadings(execRoot string) map[string]unitRaw {
units := listCarlosUnits(cgroupSliceDir)
- if units == nil {
+ units = append(units, listExecCgroups(execRoot)...)
+ if len(units) == 0 {
return nil
}
out := make(map[string]unitRaw, len(units))
@@ -709,10 +701,8 @@ func loadAlertState(path, box string) metrics.AlertState {
// still a valid deployment" shape every other optional loop in this package
// uses. Flush runs every 5th tick (5 minutes at the 60s tick cadence);
// alertTick runs every tick, since a threshold breach deserves faster
-// detection than a metrics flush does. svc feeds the units probe the
-// activator's awake exec children (nil when no activator is armed) — see
-// realSampleProbes.
-func metricsLoop(svc *activator.Service) {
+// detection than a metrics flush does.
+func metricsLoop(execRoot string) {
// This loop is always on and shares a process with the agent that serves
// this box's traffic, so a panic in it — an unreadable cgroup shape, a
// nil no one expected — must not take the box down with it. Monitoring
@@ -745,7 +735,7 @@ func metricsLoop(svc *activator.Service) {
log.Printf("agent: metrics loop armed (60s ticks, 5min flush), publishing to bucket %s", bucket)
sm := &sampler{}
- probes := realSampleProbes(svc)
+ probes := realSampleProbes(execRoot)
tick := 0
for {
time.Sleep(60 * time.Second)
diff --git a/internal/host/metrics_probes.go b/internal/host/metrics_probes.go
index ef0ffc744..30ec0a3bb 100644
--- a/internal/host/metrics_probes.go
+++ b/internal/host/metrics_probes.go
@@ -3,22 +3,20 @@
// a guessed zero. This file is thin OS glue — os.ReadFile and os.ReadDir
// wrapping the pure parsers in internal/metrics — and the readers are
// deliberately untested by the same convention as probes.go readers;
-// internal/metrics tests exercise the parsers with fixtures instead. The one
-// exceptions are the functions with decisions in them: listCarlosUnits is a
-// directory WALK, not a single read, and which directories it walks has
-// already been wrong once (see its doc comment), so it takes its root as an
-// argument and is pinned by metrics_probes_test.go against a temp-dir
-// fixture; execInstanceReadings/mergeUnitReadings are pure (keying and
-// precedence for exec-child readings, issue #118) and are pinned there too.
+// internal/metrics tests exercise the parsers with fixtures instead. The
+// exceptions are listCarlosUnits and listExecCgroups: they are directory
+// WALKs, not single reads, and which directories they walk is a decision that
+// has already been wrong once (see listCarlosUnits' doc comment), so they take
+// their roots as arguments and are pinned by metrics_probes_test.go against
+// temp-dir fixtures.
package host
import (
"os"
"path/filepath"
- "strconv"
"strings"
- "github.com/carlosframework/platform/internal/activator"
+ "github.com/carlosframework/platform/internal/cgroup"
"github.com/carlosframework/platform/internal/metrics"
)
@@ -137,73 +135,6 @@ type unitRaw struct {
writeBytes *int64
}
-// readExecInstanceRaw reads one awake exec child's resource usage from
-// /proc/<pid>/{stat,status,io} — the per-pid counterpart of readUnitRaw for
-// instances that have no unit cgroup to read (issue #118: an exec-backed
-// instance is a direct child of `carlos agent`, inside the agent's own
-// cgroup, so the unit walk above can never see it). Same contract as
-// readUnitRaw: each field reads and fails independently, toward nil — a pid
-// that vanished mid-read (raced its own exit) contributes nils, never zeros.
-// Known limit, documented in the design: a pid reading covers ONE process,
-// not a subtree — an app that forks workers under-reports here where a
-// cgroup would not.
-func readExecInstanceRaw(pid int) unitRaw {
- dir := filepath.Join("/proc", strconv.Itoa(pid))
- raw := unitRaw{}
- if b, err := os.ReadFile(filepath.Join(dir, "stat")); err == nil {
- raw.cpuUsec = metrics.ParsePidStatCPUUsec(string(b))
- }
- if b, err := os.ReadFile(filepath.Join(dir, "status")); err == nil {
- raw.memBytes, raw.memPeakBytes = metrics.ParsePidStatusMem(string(b))
- }
- if b, err := os.ReadFile(filepath.Join(dir, "io")); err == nil {
- raw.readBytes, raw.writeBytes = metrics.ParsePidIO(string(b))
- }
- return raw
-}
-
-// execInstanceReadings turns the activator's awake host→pid map into unit
-// readings keyed by the SAME carlos-app@<host>.service name the console's
-// owner view matches (activator.UnitName) — which is what makes the whole
-// fix additive: the wire shape (Sample.Units, a string-keyed map) and the
-// console's allowlist both take the new entries as-is. An exec-backed host
-// has no such unit by construction (units are for the edge; instances are
-// the agent's children), so the key cannot collide with a real cgroup
-// reading on this box; mergeUnitReadings still defines the winner in case
-// that ever changes. read is a parameter (readExecInstanceRaw in
-// production) so the keying is testable without /proc.
-func execInstanceReadings(pids map[string]int, read func(pid int) unitRaw) map[string]unitRaw {
- if len(pids) == 0 {
- return nil
- }
- out := make(map[string]unitRaw, len(pids))
- for host, pid := range pids {
- out[activator.UnitName(host)] = read(pid)
- }
- return out
-}
-
-// mergeUnitReadings composes the units probe's two sources: the cgroup walk
-// (every systemd-backed CARLOS unit) plus the per-pid readings for awake
-// exec children. On a key collision the cgroup reading wins — it accounts a
-// whole subtree where the pid reading covers one process, so it can only be
-// more complete. Both-empty stays nil (no readings, not an empty set), and
-// a nil cgroup map with live exec children still reports the children — a
-// box whose cgroup walk failed has not stopped running instances.
-func mergeUnitReadings(cgroup, exec map[string]unitRaw) map[string]unitRaw {
- if len(exec) == 0 {
- return cgroup
- }
- out := make(map[string]unitRaw, len(cgroup)+len(exec))
- for name, u := range exec {
- out[name] = u
- }
- for name, u := range cgroup {
- out[name] = u
- }
- return out
-}
-
// readUnitRaw reads a unit's resource usage from the cgroup v2 files in
// unitPath — the directory listCarlosUnits reported for that unit, which for
// an instantiated unit is NOT <cgroupSliceDir>/<name>. Each file read failure
@@ -235,3 +166,37 @@ func readUnitRaw(unitPath string) unitRaw {
return raw
}
+
+// listExecCgroups lists the per-host exec-child cgroups under the agent's
+// delegated subtree — the third scan root, alongside listCarlosUnits' two
+// levels. An empty root (no delegation armed) or a read failure -> nil. The
+// agent's own supervise leaf fails IsExecName and is skipped: that process
+// is already reported as carlos-edge.service. A dir whose cgroup.procs is
+// empty or unreadable is skipped too: a failed seed or an aborted wake
+// leaves the dir behind (until a later exit's rmdir or the next agent
+// start's sweep), and reporting it would render a host whose wake FAILED
+// as a live instance at zero — absence is the data, never a guessed zero.
+// readUnitRaw reads the included dirs unchanged — same cgroup v2 files,
+// same fail-toward-nil fields.
+func listExecCgroups(root string) []unitCgroup {
+ if root == "" {
+ return nil
+ }
+ entries, err := os.ReadDir(root)
+ if err != nil {
+ return nil
+ }
+ var units []unitCgroup
+ for _, entry := range entries {
+ if !entry.IsDir() || !cgroup.IsExecName(entry.Name()) {
+ continue
+ }
+ dir := filepath.Join(root, entry.Name())
+ procs, err := os.ReadFile(filepath.Join(dir, "cgroup.procs"))
+ if err != nil || len(strings.TrimSpace(string(procs))) == 0 {
+ continue
+ }
+ units = append(units, unitCgroup{name: entry.Name(), dir: dir})
+ }
+ return units
+}
diff --git a/internal/host/metrics_probes_test.go b/internal/host/metrics_probes_test.go
index bf9d7e443..0e691d0ca 100644
--- a/internal/host/metrics_probes_test.go
+++ b/internal/host/metrics_probes_test.go
@@ -89,60 +89,46 @@ func TestReadUnitRawReadsFromUnitDir(t *testing.T) {
}
}
-// Exec children publish under the SAME carlos-app@<host>.service key the
-// console's owner allowlist matches (activator.UnitName) — that keying is
-// the whole fix for issue #118, so it's pinned here: get it wrong and the
-// readings publish fine but match nothing, silently reopening the gap.
-func TestExecInstanceReadingsKeysByUnitName(t *testing.T) {
- cpu := int64(5_000_000)
- got := execInstanceReadings(map[string]int{"lpay.bde.oncarlos.com": 4242}, func(pid int) unitRaw {
- if pid != 4242 {
- t.Fatalf("read called with pid %d, want 4242", pid)
+// TestListExecCgroupsFindsPerHostDirs pins the delegated subtree's scan:
+// carlos-exec@<host> dirs with live members come out with their names
+// VERBATIM as sample keys (deliberately not dressed up as .service units —
+// an operator who takes a dashboard row to systemctl must not be told a
+// unit exists that doesn't). The agent's own supervise leaf and foreign
+// names are invisible; so is a MEMBER-LESS dir (empty or missing
+// cgroup.procs) — a failed seed's leftover must read as absent, never as a
+// live instance at zero. An empty root — the no-delegation state — scans
+// nothing.
+func TestListExecCgroupsFindsPerHostDirs(t *testing.T) {
+ root := t.TempDir()
+ mk := func(name, procs string) string {
+ t.Helper()
+ dir := filepath.Join(root, name)
+ if err := os.MkdirAll(dir, 0o755); err != nil {
+ t.Fatal(err)
}
- return unitRaw{cpuUsec: &cpu}
- })
- raw, ok := got["carlos-app@lpay.bde.oncarlos.com.service"]
- if !ok || len(got) != 1 {
- t.Fatalf("got keys %v, want exactly carlos-app@lpay.bde.oncarlos.com.service", got)
- }
- if raw.cpuUsec == nil || *raw.cpuUsec != cpu {
- t.Fatalf("reading not carried through: %v", raw.cpuUsec)
- }
-
- if execInstanceReadings(nil, nil) != nil {
- t.Fatal("no pids must be nil, not an empty map")
- }
-}
-
-func TestMergeUnitReadings(t *testing.T) {
- v := func(n int64) unitRaw { return unitRaw{cpuUsec: &n} }
- deref := func(u unitRaw) int64 { return *u.cpuUsec }
-
- // Both sources present: union, cgroup wins the collision (it accounts a
- // whole subtree; the pid reading is one process).
- got := mergeUnitReadings(
- map[string]unitRaw{"carlos-edge.service": v(1), "carlos-app@both.example.com.service": v(2)},
- map[string]unitRaw{"carlos-app@x.example.com.service": v(3), "carlos-app@both.example.com.service": v(4)},
- )
- if len(got) != 3 {
- t.Fatalf("got %d keys, want 3", len(got))
- }
- if deref(got["carlos-app@both.example.com.service"]) != 2 {
- t.Fatal("collision must keep the cgroup reading")
- }
- if deref(got["carlos-edge.service"]) != 1 || deref(got["carlos-app@x.example.com.service"]) != 3 {
- t.Fatal("non-colliding readings must survive from both sides")
+ if procs != "" {
+ if err := os.WriteFile(filepath.Join(dir, "cgroup.procs"), []byte(procs), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ }
+ return dir
}
+ execDir := mk("carlos-exec@keymail.dev", "1234\n")
+ mk("supervise", "1\n")
+ mk("carlos-exec@", "99\n") // no host — not one of ours
+ mk("random-dir", "77\n") // foreign name
+ mk("carlos-exec@failed.example.com", "\n") // member-less: whitespace-only procs
+ mk("carlos-exec@aborted.example.com", "") // member-less: no procs file at all
- // A failed cgroup walk (nil) with live exec children still reports them.
- got = mergeUnitReadings(nil, map[string]unitRaw{"carlos-app@x.example.com.service": v(3)})
+ got := listExecCgroups(root)
if len(got) != 1 {
- t.Fatalf("nil cgroup side: got %v, want the exec reading alone", got)
+ t.Fatalf("discovered %d cgroups, want 1: %+v", len(got), got)
+ }
+ if got[0].name != "carlos-exec@keymail.dev" || got[0].dir != execDir {
+ t.Fatalf("got %+v, want name carlos-exec@keymail.dev dir %s", got[0], execDir)
}
- // No exec children: the cgroup map passes through untouched — including
- // nil staying nil ("no readings", not an empty set of them).
- if mergeUnitReadings(nil, nil) != nil {
- t.Fatal("both-nil must stay nil")
+ if out := listExecCgroups(""); out != nil {
+ t.Fatalf("empty root must scan nothing, got %+v", out)
}
}
diff --git a/internal/host/units/carlos-edge.service b/internal/host/units/carlos-edge.service
index aa41e536a..ee57b7f40 100644
--- a/internal/host/units/carlos-edge.service
+++ b/internal/host/units/carlos-edge.service
@@ -38,7 +38,11 @@ ProtectHostname=yes
ProtectKernelTunables=yes
ProtectKernelModules=yes
ProtectKernelLogs=yes
-ProtectControlGroups=yes
+# ProtectControlGroups is deliberately ABSENT: it would mount /sys/fs/cgroup
+# read-only, and Delegate= (below) needs this unit's own subtree writable.
+# The hardening that line bought is retained by ownership instead — plain
+# DAC keeps everything OUTSIDE the chowned subtree read-only to this
+# unprivileged user.
ProtectProc=invisible
RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX
RestrictNamespaces=yes
@@ -53,6 +57,13 @@ SystemCallArchitectures=native
# No MemoryMax: hibernating instances are this unit's children, so a cap here
# would cap the whole set of awake instances at once.
TasksMax=1024
+# Exec-backed instances run as this unit's children. Delegation hands the
+# unit its own cgroup subtree — chowned to User=carlos, unmanaged by systemd
+# below this point — so the agent can give each child its own cgroup for
+# per-instance accounting (docs/superpowers/specs/
+# 2026-08-07-exec-child-accounting-design.md). The subtree still dies with
+# the service: restart semantics are unchanged.
+Delegate=cpu memory io pids
[Install]
WantedBy=multi-user.target
diff --git a/internal/metrics/pidparse.go b/internal/metrics/pidparse.go
deleted file mode 100644
index 1bb70e736..000000000
--- a/internal/metrics/pidparse.go
+++ /dev/null
@@ -1,101 +0,0 @@
-// /proc/<pid> parsers — the per-process counterpart of the cgroup parsers in
-// parse.go, for instances that have no unit cgroup to read: an exec-backed
-// instance is a direct child of `carlos agent` inside the agent's own cgroup
-// (issue #118), so its readings come from /proc/<pid>/{stat,status,io}
-// instead. Same package contract as everything else here: parse text the
-// caller read, fail toward nil, never touch the OS.
-package metrics
-
-import (
- "strconv"
- "strings"
-)
-
-// userHZ is the tick unit of /proc/<pid>/stat's utime/stime fields. The
-// kernel fixes the userspace-visible USER_HZ at 100 on every architecture
-// CARLOS runs on (it is independent of the kernel's internal CONFIG_HZ), so
-// a constant is the honest value — there is no portable syscall to ask for
-// it without cgo.
-const userHZ = 100
-
-// ParsePidStatCPUUsec parses /proc/<pid>/stat and returns utime+stime as CPU
-// microseconds — the same unit as cpu.stat's usage_usec, so the reading
-// feeds UnitCPUFrac's delta machinery unchanged. The comm field (2) is the
-// process name in parentheses and may itself contain spaces and parens, so
-// fields are located from the LAST ')' — counting from the front would
-// misparse any binary with a space in its name. After that anchor the line
-// reads: state ppid pgrp session tty tpgid flags minflt cminflt majflt
-// cmajflt utime stime — utime and stime at offsets 11 and 12. Too few
-// fields or a non-numeric tick count -> nil.
-func ParsePidStatCPUUsec(text string) *int64 {
- i := strings.LastIndexByte(text, ')')
- if i < 0 {
- return nil
- }
- fields := strings.Fields(text[i+1:])
- if len(fields) < 13 {
- return nil
- }
- utime, err1 := strconv.ParseInt(fields[11], 10, 64)
- stime, err2 := strconv.ParseInt(fields[12], 10, 64)
- if err1 != nil || err2 != nil {
- return nil
- }
- usec := (utime + stime) * (1e6 / userHZ)
- return &usec
-}
-
-// ParsePidStatusMem parses /proc/<pid>/status and returns VmRSS (resident
-// set) and VmHWM (its peak) in bytes. RSS rather than anything
-// cache-inclusive on purpose: a per-process reading cannot attribute page
-// cache the way memory.current does, and for "what your app uses" resident
-// memory is the honest number it CAN give. Each field fails toward nil
-// independently — the kernel prints these in kB always, so any other unit
-// suffix means the line is being misread and nils that field rather than
-// guessing a scale.
-func ParsePidStatusMem(text string) (rss, hwm *int64) {
- for _, line := range strings.Split(text, "\n") {
- fields := strings.Fields(line)
- if len(fields) != 3 || fields[2] != "kB" {
- continue
- }
- v, err := strconv.ParseInt(fields[1], 10, 64)
- if err != nil {
- continue
- }
- bytes := v * 1024
- switch fields[0] {
- case "VmRSS:":
- rss = &bytes
- case "VmHWM:":
- hwm = &bytes
- }
- }
- return rss, hwm
-}
-
-// ParsePidIO parses /proc/<pid>/io and returns read_bytes / write_bytes —
-// actual block-device I/O, cumulative since process start, the same
-// semantics as the cgroup io.stat counters. rchar/wchar are deliberately
-// ignored: they count every read/write syscall including cache hits, a
-// different (and much larger) number. Each field fails toward nil
-// independently.
-func ParsePidIO(text string) (rbytes, wbytes *int64) {
- for _, line := range strings.Split(text, "\n") {
- key, val, ok := strings.Cut(line, ":")
- if !ok {
- continue
- }
- v, err := strconv.ParseInt(strings.TrimSpace(val), 10, 64)
- if err != nil {
- continue
- }
- switch key {
- case "read_bytes":
- rbytes = &v
- case "write_bytes":
- wbytes = &v
- }
- }
- return rbytes, wbytes
-}
diff --git a/internal/metrics/pidparse_test.go b/internal/metrics/pidparse_test.go
deleted file mode 100644
index 85d8dfdd0..000000000
--- a/internal/metrics/pidparse_test.go
+++ /dev/null
@@ -1,83 +0,0 @@
-package metrics
-
-import "testing"
-
-func TestParsePidStatCPUUsec(t *testing.T) {
- // pid (comm) state ppid pgrp session tty tpgid flags minflt cminflt
- // majflt cmajflt utime stime ... — utime=250 stime=150 ticks at USER_HZ
- // 100 is 4.0 CPU-seconds = 4_000_000 usec.
- line := "1234 (lpay) S 1 1234 1234 0 -1 4194560 900 0 0 0 250 150 0 0 20 0 8 0 12345 1000000 500 18446744073709551615 1 1 0 0 0 0 0 0 0 0 0 0 17 3 0 0 0 0 0"
- v := ParsePidStatCPUUsec(line)
- if v == nil || *v != 4_000_000 {
- t.Fatalf("got %v, want 4000000", v)
- }
-
- // comm may contain spaces AND parens; everything before the LAST ')' is
- // name, so the field offsets must be taken from there.
- tricky := "1234 (my app) (v2)) R 1 1234 1234 0 -1 4194560 900 0 0 0 100 100 0 0 20 0 8 0 12345 1000000 500 0 1 1 0 0 0 0 0 0 0 0 0 0 17 3 0 0 0 0 0"
- v = ParsePidStatCPUUsec(tricky)
- if v == nil || *v != 2_000_000 {
- t.Fatalf("paren-comm: got %v, want 2000000", v)
- }
-
- for _, bad := range []string{
- "",
- "no parens at all",
- "1234 (lpay) S 1 2", // too few fields after comm
- "1234 (lpay) S 1 2 3 4 5 6 7 8 9 10 x 150 0", // non-numeric utime
- } {
- if ParsePidStatCPUUsec(bad) != nil {
- t.Fatalf("garbage %q must be nil", bad)
- }
- }
-}
-
-func TestParsePidStatusMem(t *testing.T) {
- text := "Name:\tlpay\nVmPeak:\t 20000 kB\nVmHWM:\t 9000 kB\nVmRSS:\t 8192 kB\nThreads:\t8\n"
- rss, hwm := ParsePidStatusMem(text)
- if rss == nil || *rss != 8192*1024 {
- t.Fatalf("rss: got %v, want %d", rss, 8192*1024)
- }
- if hwm == nil || *hwm != 9000*1024 {
- t.Fatalf("hwm: got %v, want %d", hwm, 9000*1024)
- }
-
- // Each field fails toward nil independently: a kernel thread has no VmRSS
- // line at all, and a garbled VmHWM must not take VmRSS down with it.
- rss, hwm = ParsePidStatusMem("Name:\tx\nVmRSS:\t 100 kB\nVmHWM:\tgarbage kB\n")
- if rss == nil || *rss != 100*1024 || hwm != nil {
- t.Fatalf("independent fields: got rss=%v hwm=%v", rss, hwm)
- }
-
- rss, hwm = ParsePidStatusMem("")
- if rss != nil || hwm != nil {
- t.Fatal("empty must be both-nil")
- }
-
- // A VmRSS value not in kB (the kernel never emits another unit, so an
- // unexpected one means we're misreading) must be nil, not a wrong scale.
- rss, _ = ParsePidStatusMem("VmRSS:\t 100 mB\n")
- if rss != nil {
- t.Fatal("non-kB unit must be nil")
- }
-}
-
-func TestParsePidIO(t *testing.T) {
- text := "rchar: 999\nwchar: 888\nsyscr: 10\nsyscw: 20\nread_bytes: 4096\nwrite_bytes: 8192\ncancelled_write_bytes: 0\n"
- r, w := ParsePidIO(text)
- if r == nil || *r != 4096 || w == nil || *w != 8192 {
- t.Fatalf("got r=%v w=%v, want 4096/8192", r, w)
- }
-
- // Independent nils: rchar/wchar (which count ALL io including cached)
- // must never stand in for the missing block-io counters.
- r, w = ParsePidIO("rchar: 999\nread_bytes: 4096\n")
- if r == nil || *r != 4096 || w != nil {
- t.Fatalf("independent fields: got r=%v w=%v", r, w)
- }
-
- r, w = ParsePidIO("")
- if r != nil || w != nil {
- t.Fatal("empty must be both-nil")
- }
-}
diff --git a/internal/metrics/sample.go b/internal/metrics/sample.go
index 99dcef827..da14784a0 100644
--- a/internal/metrics/sample.go
+++ b/internal/metrics/sample.go
@@ -27,8 +27,10 @@ type UnitSample struct {
}
// Sample is one point-in-time reading of a box: the host as a whole plus
-// every monitored unit, keyed by full unit name (e.g.
-// carlos-app@x.example.com.service).
+// every monitored unit, keyed by unit name: systemd unit names (e.g.
+// carlos-app@x.example.com.service) and delegated exec-child cgroup names
+// (e.g. carlos-exec@x.example.com — no .service suffix; written by the
+// sampler's exec-subtree scan since exec-child accounting).
type Sample struct {
At time.Time `json:"at"`
Host HostSample `json:"host"`