# Review brief: isolated exec design

You are GPT-6 reviewing a design spec independently of its author. Your
output is a findings report, not an edit. Do not modify any file.

## What to read

1. The spec under review:
   `docs/superpowers/specs/2026-09-11-isolated-exec-design.md`.
2. The problem it answers, as filed: amadan discussion #126 on
   carlos/platform, reproduced in spec §1; its companion #127 (spec §7).
3. The code it fits into, in this worktree:
   - `internal/activator/backend_exec.go` (Start 557-773, spawnInCgroup
     154-156, childProc 992-1033, instanceEnv 775-795, childEnv 802-814,
     teeStd 123-137, Seed 332-440, StartSidecar 879-912, Restore 220-290,
     PurgeLocalDBArtifacts 1041-1057);
   - `internal/activator/activator.go` (Proc/Rep 194-207, doWake 772-900,
     Bounce 946-956, stop 962-1047, sweepOnce 1082);
   - `internal/activator/backend_unit.go` (systemctl 94-103, package doc);
   - `internal/activator/admintoken.go` (6-14, 111-195);
   - `internal/edge/purge.go` (53-107), `internal/edge/edge.go` (449-451,
     536-538);
   - `internal/cgroup/cgroup.go` (37-62, 219, 242-285, 398-403) and the
     `_linux.go` files;
   - `internal/lease/lease.go` (257-299);
   - `internal/appconfig/syncexec.go` (1-31, 465, 566-628, 860-956,
     977-1024);
   - `internal/provision/provision.go` (36-135, 246-262, 995-1012),
     `internal/provision/reconcile.go` (280-301, 374-436, 461-464,
     862-892);
   - `internal/registry/registry.go` (58, 152-156, 413, 1057-1075);
   - `internal/ops/litestream.go` (273-428);
   - `internal/host/units/*` (all of them; especially
     `carlos-edge.service`, `carlos-app@.service`, `carlos-app@.socket`,
     `carlos-stage.*`, `carlos-restart.*`, `carlos-litestream.service`);
   - `internal/host/bootstrap.go` (33-34, 116-180),
     `internal/host/systemapply.go` (339-398, 517-589),
     `internal/host/systemunits.go` (34-83);
   - `internal/host/movespass.go` (100-132, 285), `internal/host/agent.go`
     (186-192, 285, 305-309, 1771), `internal/schedule/runner.go` (461),
     `internal/host/schedule_wiring.go` (163, 343-369);
   - `cli.go` (83-199, 1622-1625);
   - `AGENTS.md`.
4. The precedent the namespace recipe is taken from:
   `/home/paulca/github.com/tito/titogo/deploy/systemd/titogo@.service`
   and `titogo-instance-dir@.service` (read-only; another repo).
5. The pinfra scripts the rollout depends on:
   `/home/paulca/github.com/carlosframework/platform-infrastructure/scripts/update-platform-binary.sh`
   and `harden-host.sh` (read-only; another repo), and the two earlier
   designs the spec moves resources out of:
   `/home/paulca/github.com/carlosframework/platform-infrastructure/platform-design/superpowers/specs/2026-08-07-exec-child-accounting-design.md`
   and `2026-08-28-memory-pressure-containment-design.md`.
6. systemd's documented semantics (systemd.exec(5), systemd.service(5),
   systemd.resource-control(5), systemd.socket(5)) for every directive
   the spec relies on: `TemporaryFileSystem=`, `BindPaths=`,
   `PrivatePIDs=`, `ProtectProc=`, `RestrictNamespaces=`,
   `StandardOutput=file:` on an `AF_UNIX` path, `PartOf=`, `KillMode=mixed`,
   `OOMPolicy=kill`, `OOMScoreAdjust=`, `Slice=`,
   `systemctl set-property --runtime` on a not-yet-started template
   instance, and which `systemctl` verbs an unprivileged caller may run
   without polkit.

## Rounds after the first

From round 2 on, the prior rounds' findings files
(`CODEX-FINDINGS-ISOLATED-EXEC.md`, then `-R2.md`, ...) are committed
beside the spec, and the spec's §13 records how each finding was
taken. **Start your report with a disposition table**: one row per
prior finding, stating whether the revision resolves it, resolves it
partially (say what is missing), or does not resolve it. Then report
only findings that are new in this revision, numbered `R<n>-1` onward
for the current round. Do not repeat a resolved finding; do not
re-argue a ruling.

## Decisions already taken (report a mechanism that fails one; do not re-argue one)

- Paul approved approach 3 on 2026-09-11: the activator keeps ownership;
  the process becomes a systemd template unit; still uid `carlos`;
  namespace isolation; no console change; migration by rename.
- Approaches 1 and 2 (spec §12) are rejected.
- Ruled with codex R1-1 (2026-09-11): the root channel is the
  `carlos-instance-control` service with a bearer token; polkit is not
  installed and not relied on. Report a way the token or the channel
  fails; do not re-open the polkit comparison.

## Things to check hard in revision 2

- **`nosymfollow` (§3.2).** Does a bind mount of a directory onto itself
  with `nosymfollow` hold for lookups through the covering mount from
  the agent's namespace, and through systemd's `BindPaths=` bind inside
  the instance's namespace? Is there any path the agent, the edge, PID 1
  (`StandardOutput=file:`) or litestream resolves under the instance
  trees that is not on that mount? Is the `openat2` helper redundant,
  insufficient, or wrong in any flag?
- **The token (§5.4).** Where does it live, who writes it, who reads it,
  and can an instance obtain it or cause the agent to present it on the
  instance's behalf?
- **Exit observation (§6.5).** `cgroup.events` inotify semantics for
  `populated`; the invocation id match; the fallback poll; what happens
  to a proc whose unit is restarted by something other than the agent.
- **Slices (§4.3).** `Slice=carlos-instances-%i.slice` from a template
  with an escaped instance name; on-demand slice creation;
  `set-property --runtime` on a slice that does not exist yet; whether
  moving the edge into `carlos.slice` keeps the 2026-08-28 wall's
  semantics.
- **The fence (§8.1) and the reverse migration (§8.3).** Every consumer
  that must honour the marker; the recovery table; the "two databases"
  refusal; what `carlos system rollback` does today and whether a leg
  can be added before the binary swap.
- **The probe and the gate (§8.4).** Can the probe pass while an instance
  would not be isolated? Can it fail while instances would be?

## What matters most

- **The isolation claim in §4.1.** From inside the unit, can an instance
  reach a sibling's database, env file, config file, listening socket,
  control socket, log socket, pid, or the control service socket? Can it
  reach IMDS? Can it reach `/data/registry.db`, `/data/litestream.env`,
  the console DB? Check each directive does what the spec says, and
  check the order systemd applies them in (bind sources, tmpfs, ro).
- **The lifecycle in §6.** Exit observation with `Restart=no`; the
  stop ordering (stop → done → close listeners) against
  `backend_exec.go:992-1009`'s stated invariant; the log socket as a
  replacement for the pipe (`teeStd`), including a grandchild holding it
  open and `KillMode=mixed`; the race between binding `log.sock` and
  PID 1 connecting; a wake that fails health and must stop; `Bounce`;
  the agent-start sweep (§6.7) and `PartOf=`.
- **Memory (§4.3, §6.6).** The wall moves from one unit to a slice plus a
  smaller edge cap. Is 60/20/20 defensible against the 2026-08-28
  design's reasoning? Does `OOMPolicy=kill` on the instance unit
  reproduce `memory.oom.group=1`? Does `OOMScoreAdjust=500` reproduce
  `ExecOOMScoreAdj`? Does the sampler's new scan root give the console
  the same per-host numbers?
- **Migration (§8).** Idempotence between the file renames and the row
  CAS; the ineligible-row policy (refuse to wake, name the fix); the
  rollback story (old binary restores flat, nested copy retained); the
  reconciler's drift log; the box-wide litestream generator's flat glob
  (`internal/ops/litestream.go:354`) and what `FleetDBsWithSkipped`
  claims after the row changes; what an in-flight residence move
  (`movespass.go`) sees.
- **The trampoline (§6.2).** `instance-exec` reads an agent-written JSON
  from the instance directory and execs. Is there a way the tenant
  influences that file? Is refusing `argv[0] != /opt/carlos/live/<host>`
  the right check? Anything the current `exec.Command` path does that
  the trampoline drops (working dir, fds, `WaitDelay`, `Pdeathsig`)?
- **Every consumer (§9).** AGENTS.md's rule: everything that PARSES the
  shape. Find a consumer the list misses.
- **Claims about the code.** Every `file:line` and every "already
  exists" or "unchanged" claim: check it. Report each one that is wrong.
- **Operability.** Can a member, with no box access, tell that their
  instance is isolated, see why it refused to wake, and read its logs as
  before? Can an operator roll this back?
- **Scope.** Anything in the spec that is not needed for the isolation
  property and could be cut; anything needed that is missing.

## How to report

Number findings R1-1 onward. For each: severity (P1 breaks the isolation
property, breaks hibernation, loses data, or makes the change unbuildable
as written; P2 a contract, claim or migration defect to fix before the
plan; P3 advisory), the section, the problem, the evidence with
citations, a proposed correction. Put the §5.4 advisement first, as its
own finding, with a clear recommendation. End with a verdict: accept,
accept with corrections, or request changes. Do not soften, do not pad,
do not restate the spec.