amadan
plan: app capabilities — nine tasks, declaration surface included
The plan's code reading caught what the spec missed: there was no way to put a capability block in a manifest at all (ship gains -capabilities and the console API meta gains the block), the ship door's identical-content short-circuit would keep a stale capability block under an immutable version (now a 409), and restartRoute's unconditional nil made the restart-failure rollback dead code for unit routes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 file changed,
+894
−0
docs/superpowers/plans/2026-08-03-app-capabilities.md+894 −0
diff --git a/docs/superpowers/plans/2026-08-03-app-capabilities.md b/docs/superpowers/plans/2026-08-03-app-capabilities.md| new file mode 100644 |
| index 000000000..ceffc01b1 |
| --- /dev/null |
| +++ b/docs/superpowers/plans/2026-08-03-app-capabilities.md |
| @@ -0,0 +1,894 @@ |
| +# App Capabilities 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:** An app declares what it needs from the platform in its release manifest; the platform decides how — and whether — to provide it. A closed vocabulary of seven names materializes as per-instance systemd drop-ins over the one unchanged `carlos-app@` template, with the two operator-consented capabilities (`ports`, `host_role`) gated on a per-route, per-box registry grant. The end state is that console and the keymail relay stop being hand-provisioned units. |
| + |
| +**Architecture:** One new pure-plus-seams package `internal/caps` (vocabulary, strict decoding, validation, the deterministic drop-in renderer, and a converge function whose only OS contact is behind injectable seams) + an additive `capabilities` object on `release.Manifest` + a staged sibling manifest on disk + an additive `caps` column on the registry + `carlos route -grant/-revoke` + converge wired into `release.adoptHost` (version change) and `host.RunAdopt` (grant drift) + three advisories. |
| + |
| +**Tech Stack:** Go stdlib only. No new deps, no new bucket prefixes, no schema bumps. |
| + |
| +## Global Constraints |
| + |
| +- Spec: `docs/superpowers/specs/2026-08-03-app-capabilities-design.md` — **read it in full first**; it is the authority. Its §10 open questions are **already decided** (see "Decisions taken" below) and must not be reopened. |
| +- **Wire-shape changes are additive.** `Manifest.schema` stays `1` forever for this change. Every manifest already in a production bucket has no `capabilities` key, must keep parsing, and must mean "no capabilities" — today's behaviour byte for byte. The precedent is `appconfig.Manifest.ExecDelivery`. |
| +- **Production channel names are pinned** in `internal/release/laddercheck_test.go` (`canary/platform-adoption`, `canary/keymail`, `canary/console`, `canary/rehearsal`). Nothing in this change may consult, constrain, or be constrained by a channel name — Task 9 adds the assertion. |
| +- **Adoption swaps a symlink; it does not restart processes.** Order stays promote → adopt → restart. The one thing this change adds is that a `daemon` route's restart becomes the platform's, inside that same order. |
| +- **Console security gates are untouched.** Session → CSRF → owner (byte-identical 404) → `requireFreshAuth` LAST. The optional console work in Task 9 is a read-only render inside an already-gated page; it adds no route and reorders nothing. |
| +- **No systemd directives, paths, users, uids, or numbers from a manifest, ever.** The vocabulary is seven names; the directives are Go constants in `internal/caps`. There is no passthrough and no `extra` field. The escape hatch is `90-operator.conf`, which the platform never writes and never reads. |
| +- **Fail closed in the direction that changes nothing.** An unknown capability name, or a `"v"` this binary does not know, refuses the *version on this box* — not staged, not pointed at. The box keeps serving what it has. |
| +- **Deploy order is pinned:** platform binaries first, capability-bearing releases second (same rule and reason as scoped signing keys). |
| +- Build gate before every commit: `go build ./... && go vet ./... && go test ./... -count=1` all clean, `gofmt -l` clean for touched files. Sandbox: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off`; disable the sandbox if the toolchain reports "read-only file system". |
| +- Commit messages follow repo style: lowercase prefix (`caps:`, `release:`, `registry:`, `host:`, `cli:`), what + why. |
| +- Delegation (repo CLAUDE.md): every task here either grants authority or touches a production path, so **every task is reviewed by Opus**. Implementation tier is named per task. |
| + |
| +## Decisions taken (do not reopen) |
| + |
| +1. keymail takes the `LISTEN_FDS` change; **that change lives in the keymail repo and is out of scope here**. The platform ships socket-unit `ports` support regardless. |
| +2. `carlos-<app>` static users plus operator-run nft allowlist entries: accepted as designed. The platform **prints** the firewall command and never writes it. |
| +3. console keeps zero-bake `canary/console` — promote-is-deploy. |
| +4. Grants are **per route, per box**. |
| +5. `console.db` moves to `/var/lib/carlos-app/<host>/`. |
| + |
| +## Out of scope (say so, do not drift into it) |
| + |
| +- **The keymail `LISTEN_FDS` change** — a different repository. Nothing in this plan waits on it, and no task in this plan may edit it. |
| +- **The actual box migrations (spec §5)** — operator sessions on the flagship and Tito boxes. Documented, not coded. No task here runs `systemctl` against a real box, stops `console.service`, or copies a production database. |
| +- **Infra-repo script deletions (spec §5d)** — `platform-infrastructure` is a different checkout. Removing `rollout-status-flagship.sh`'s restart pair and `rehearse-keymail.sh`'s unit heredoc happens after the migrations land, not here. |
| + |
| +--- |
| + |
| +### Task 1: `internal/caps` — the vocabulary, strict decoding, validation |
| + |
| +**Model:** implement **Sonnet** (precise brief, one new package, pure code); review **Opus** (this is the whole security model's parser — it is the file that decides what a hostile manifest can say). |
| + |
| +**Files:** |
| +- Create: `internal/caps/caps.go`, `internal/caps/caps_test.go` |
| + |
| +**Interfaces (produces; consumed by every later task):** |
| + |
| +```go |
| +// Package caps is the closed capability vocabulary: what an app may ask the |
| +// platform for, and what each name maps to. A manifest never names a path, a |
| +// user, a uid, a resource number, or a systemd directive — the directives are |
| +// constants in this package. |
| +package caps |
| + |
| +// Version is the capability block version THIS binary understands. A block |
| +// declaring a higher "v" refuses the version on this box (spec §3b). |
| +const Version = 1 |
| + |
| +// Marker is the first line of every drop-in this package generates — the same |
| +// marker `carlos ops litestream` uses. A file at a generated path WITHOUT it is |
| +// never overwritten and never deleted. |
| +const Marker = "# GENERATED by carlos adopt — do not hand-edit" |
| + |
| +type Set struct { |
| + V int `json:"v"` |
| + Command string `json:"command,omitempty"` |
| + Daemon bool `json:"daemon,omitempty"` |
| + Children bool `json:"children,omitempty"` |
| + RuntimeDir string `json:"runtime_dir,omitempty"` |
| + Ports []Port `json:"ports,omitempty"` |
| + Data *Data `json:"data,omitempty"` |
| + HostRole bool `json:"host_role,omitempty"` |
| + |
| + notes []string // unknown-field-inside-a-known-capability lines; see Notes |
| +} |
| +type Port struct { |
| + Port int `json:"port"` |
| + Name string `json:"name,omitempty"` // an operator/log label; carries NO behaviour |
| +} |
| +type Data struct { |
| + Replicate string `json:"replicate"` // DataReplicatePlatform | DataReplicateApp |
| + DBs []string `json:"dbs,omitempty"` |
| +} |
| + |
| +const ( |
| + DataReplicatePlatform = "platform" |
| + DataReplicateApp = "app" |
| +) |
| + |
| +// UnmarshalJSON is the strict door (spec §3b), and the three outcomes are |
| +// deliberately different: |
| +// - an unknown CAPABILITY NAME, or a "v" greater than Version: error, naming |
| +// the capability and "update the platform binary". This propagates through |
| +// json.Unmarshal -> release.GetManifest -> ensureStaged, which is exactly |
| +// how the version comes to be refused on this box. |
| +// - an unknown FIELD INSIDE a known capability: accepted, recorded in notes. |
| +// - anything absent: the zero value, which means "not declared". |
| +func (s *Set) UnmarshalJSON(b []byte) error |
| + |
| +// Notes are the ignore-with-a-log lines the decoder collected, sorted. Callers |
| +// print them once per pass, never per host. |
| +func (s *Set) Notes() []string |
| + |
| +// ValidCapabilities is the semantic gate, shared by SHIP (a human is present to |
| +// read the refusal) and ADOPT (the box refuses the version), exactly as |
| +// appconfig.ValidExecDelivery is shared by set-time and sync-time. A nil Set is |
| +// valid and means no capabilities. app is needed for the host_role user-name |
| +// length check and nothing else. |
| +func ValidCapabilities(app string, s *Set) error |
| + |
| +// AppUser is the static system user a host_role app runs as, DERIVED by the |
| +// platform and never taken from a manifest. |
| +func AppUser(app string) string // "carlos-" + app |
| +``` |
| + |
| +Validation rules to encode exactly (each is a test case): |
| + |
| +| Rule | Refusal | |
| +|---|---| |
| +| `s.V != Version` (including `0`, i.e. the key omitted from a present block) | `capabilities: v %d is not supported by this binary (this binary speaks v1) — update the platform binary` | |
| +| `command` non-empty and not `^[a-z][a-z0-9-]{0,31}$` | names the charset; one token, no flags, no spaces | |
| +| `runtime_dir` non-empty and not `^[a-z][a-z0-9-]{0,31}$` | so `../etc`, `carlos/apps` and absolute paths do not parse | |
| +| `runtime_dir` in the reserved list: `carlos`, `systemd`, `lock`, `log`, `user`, `dbus`, `sshd`, `udev`, or `strings.HasPrefix(name, "systemd-")` | names the reserved word. `carlos` is the load-bearing one: a tenant owning `/run/carlos` would own every tenant socket the edge dials | |
| +| `ports[].port` outside `1..1023` | "only privileged ports are a capability; a high port needs no help from the platform" | |
| +| `ports[].port` in `{22, 80, 443}` | reserved absolutely; **not grantable at all** | |
| +| duplicate port in one `ports` list | refused | |
| +| `len(ports) > 4` | refused (a bound, not a feature) | |
| +| `ports[].name` non-empty and not `^[a-z][a-z0-9-]{0,31}$` | it lands in logs | |
| +| `data.replicate` not one of the two constants | names both | |
| +| `data.dbs` entry not `^[A-Za-z0-9._-]{1,64}$`, or equal to `.` / `..`, or containing any separator | used only as `filepath.Join(stateDir, name)` and must stay a bare filename | |
| +| `len(data.dbs) > 8` | bound | |
| +| `data.dbs` non-empty while `replicate == "app"` | refused: `dbs` is meaningless when the app streams its own WAL, and accepting it would invite the two-replicators hazard | |
| +| `host_role` and `len(AppUser(app)) > 32` | the account-name limit; refused at ship where a human can rename | |
| + |
| +- [ ] **Step 1: Write the failing tests.** Minimum set, all table-driven where sensible: |
| + |
| +```go |
| +func TestUnknownCapabilityNameIsRefused(t *testing.T) { |
| + var s Set |
| + err := json.Unmarshal([]byte(`{"v":1,"daemon":true,"gpu":true}`), &s) |
| + if err == nil || !strings.Contains(err.Error(), "gpu") { |
| + t.Fatalf("err = %v, want a refusal naming gpu", err) |
| + } |
| + if !strings.Contains(err.Error(), "update the platform binary") { |
| + t.Fatalf("refusal %q does not name the fix", err) |
| + } |
| +} |
| + |
| +func TestUnknownFieldInsideAKnownCapabilityIsIgnoredAndNoted(t *testing.T) { |
| + var s Set |
| + if err := json.Unmarshal([]byte(`{"v":1,"data":{"replicate":"platform","dbs":["console.db"],"future":7}}`), &s); err != nil { |
| + t.Fatalf("unknown field inside a known capability must NOT fail: %v", err) |
| + } |
| + if s.Data == nil || s.Data.Replicate != DataReplicatePlatform || len(s.Data.DBs) != 1 { |
| + t.Fatalf("known fields lost: %+v", s.Data) |
| + } |
| + if n := s.Notes(); len(n) != 1 || !strings.Contains(n[0], "future") { |
| + t.Fatalf("notes = %v, want one line naming future", n) |
| + } |
| +} |
| + |
| +func TestVersionBumpIsRefused(t *testing.T) { |
| + var s Set |
| + if err := json.Unmarshal([]byte(`{"v":2,"daemon":true}`), &s); err == nil { |
| + t.Fatal("v:2 must be refused — fail closed, the box keeps what it serves") |
| + } |
| +} |
| + |
| +func TestRuntimeDirCharsetAndReservedNames(t *testing.T) { |
| + for _, bad := range []string{"../etc", "carlos/apps", "/run/keymail", "Keymail", "", "carlos", "systemd", "systemd-foo", "lock", "log", "user", "dbus", "sshd", "udev", strings.Repeat("a", 33)} { |
| + s := &Set{V: 1, RuntimeDir: bad} |
| + if err := ValidCapabilities("keymail", s); err == nil { |
| + t.Errorf("runtime_dir %q was accepted", bad) |
| + } |
| + } |
| + if err := ValidCapabilities("keymail", &Set{V: 1, RuntimeDir: "keymail"}); err != nil { |
| + t.Fatalf("keymail refused: %v", err) |
| + } |
| +} |
| + |
| +func TestPortsReservedAndNonPrivilegedRefused(t *testing.T) { |
| + for _, p := range []int{0, 22, 80, 443, 1024, 8080, 65535, -1} { |
| + if err := ValidCapabilities("keymail", &Set{V: 1, Ports: []Port{{Port: p}}}); err == nil { |
| + t.Errorf("port %d was accepted", p) |
| + } |
| + } |
| + if err := ValidCapabilities("keymail", &Set{V: 1, Ports: []Port{{Port: 25, Name: "mx"}}}); err != nil { |
| + t.Fatalf("port 25 refused: %v", err) |
| + } |
| +} |
| + |
| +func TestHostRoleRefusesAnAppNameTooLongForAUser(t *testing.T) { |
| + long := strings.Repeat("a", 30) // carlos-<30> = 37 > 32 |
| + if err := ValidCapabilities(long, &Set{V: 1, HostRole: true}); err == nil { |
| + t.Fatal("host_role on an over-long app name must be refused at ship") |
| + } |
| +} |
| + |
| +func TestNilSetIsValidAndMeansNothing(t *testing.T) { |
| + if err := ValidCapabilities("console", nil); err != nil { |
| + t.Fatalf("nil (today's every manifest) must be valid: %v", err) |
| + } |
| +} |
| +``` |
| + |
| +Plus: `data.dbs` charset/`..`/separator refusals; `dbs` with `replicate:"app"` refused; `command` charset refusals (`"relay --flag"`, `"Relay"`, `"1relay"`); duplicate port refused. |
| + |
| +- [ ] **Step 2:** `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/caps/ -count=1` — FAIL (package does not exist). |
| +- [ ] **Step 3: Implement `caps.go`.** `UnmarshalJSON` decodes into `map[string]json.RawMessage` first, checks every key against the seven known names plus `v`, then decodes each known capability's object with a second `map[string]json.RawMessage` pass to collect notes before decoding into the typed value. Do **not** use `Decoder.DisallowUnknownFields` on the whole block — it cannot distinguish an unknown *name* (refuse) from an unknown *field* (ignore), and that distinction is the entire forward-compatibility contract. |
| +- [ ] **Step 4:** tests PASS; `go build ./... && go vet ./...` clean; `gofmt -l internal/caps/*.go` empty. |
| +- [ ] **Step 5: Commit** `caps: the closed capability vocabulary — strict names, tolerant fields, fail-closed versions` |
| + |
| +**What the reviewer should try to break:** a manifest with `"capabilities": {}` (no `v`) — must refuse, not silently mean nothing. `"v": 1` with `"ports": []` — valid, materializes nothing. A `runtime_dir` of `"con"`/`"aux"`-style oddities (harmless on Linux, but confirm the charset alone is the gate). `data.dbs: ["console.db/../../etc/passwd"]`. `data.dbs: ["."]`. A `Set` value copied by value after `Notes()` — confirm `notes` being unexported does not make a decoded `Set` compare unequal to a literal in a caller's test (`reflect.DeepEqual` on `Set` values is used in Task 2; either keep notes out of the compared value or document the helper). |
| + |
| +--- |
| + |
| +### Task 2: `release.Manifest` gains `capabilities` — additive, refused at ship and at adopt |
| + |
| +**Model:** implement **Opus** (wire shape on the trust chain, and the refusal direction is the whole of §3b); review **Opus**. |
| + |
| +**Files:** |
| +- Modify: `internal/release/release.go` (`Manifest` at :44-55, `validateManifest` at :205-233) |
| +- Modify: `cli.go` (`runShip` at :810-880, `runShipAPI` at :903-970) |
| +- Modify: `apimode.go` (`apiSession.ship` at :374-421 — the `meta` part) |
| +- Modify: `internal/console/apicli.go` (`handleAPIShip`'s meta struct at :516-533 and the `release.Manifest` literal at :603-611) |
| +- Test: `internal/release/release_test.go` (additions), `internal/release/golden_test.go` (one new test) |
| + |
| +**Interfaces:** |
| + |
| +```go |
| +// release.go |
| +type Manifest struct { |
| + // ... unchanged fields ... |
| + |
| + // Capabilities is what this BUILD needs from the platform (internal/caps). |
| + // ADDITIVE to schema 1, exactly as appconfig.Manifest.ExecDelivery was: a |
| + // manifest written before this field existed has no "capabilities" key, |
| + // parses with a nil pointer, and means no capabilities — which is today's |
| + // behaviour byte for byte, and is most of every production bucket. |
| + // |
| + // It lives in the release manifest and not the config manifest because it |
| + // describes the unit the platform OWNS and it changes with the code: a |
| + // rollback rolls the unit shape back with the binary, and the declaration |
| + // rides the same trust chain as the bytes (signed pointer -> immutable |
| + // manifest -> sha256). |
| + Capabilities *caps.Set `json:"capabilities,omitempty"` |
| +} |
| +``` |
| + |
| +`validateManifest` gains, as its LAST check: `if err := caps.ValidCapabilities(m.App, m.Capabilities); err != nil { return fmt.Errorf("capabilities: %w", err) }`. |
| + |
| +That one line is what makes ship and adopt share the gate: `Ship` calls `validateManifest` before writing a byte, and `GetManifest` calls it on every read, so a box meeting a capability it does not understand fails `ensureStaged` and prints one line per pass without staging or pointing. |
| + |
| +Ship-side declaration input (**spec gap resolved here — see "Risks" at the foot of this plan**): the spec says a manifest declares capabilities but never says how one gets into a manifest. Pin it as the smallest possible surface: |
| + |
| +```go |
| +// cli.go runShip |
| +capsFile := fs.String("capabilities", "", "path to a JSON capability block (internal/caps) to embed in the manifest") |
| +``` |
| +- Direct mode: read the file, `json.Unmarshal` into a `*caps.Set` (the strict decoder refuses unknown names here, at the operator's terminal), assign to `m.Capabilities`. `release.Ship`'s `validateManifest` then refuses anything semantically wrong. |
| +- API mode (`runShipAPI` → `apiSession.ship`): the same bytes ride in the existing `meta` part as a new field `"capabilities": <raw JSON>`; `handleAPIShip`'s meta struct gains `Capabilities json.RawMessage \`json:"capabilities,omitempty"\`` and decodes it into a `*caps.Set`, answering `400` with the decoder's own message on refusal. The meta part is already read under an `io.LimitReader(first, 8<<10)` — a capability block is a few hundred bytes, so the limit stands unchanged. |
| +- No flag given ⇒ `nil` ⇒ byte-identical manifests to today. **Assert that.** |
| + |
| +- [ ] **Step 1: Failing tests.** |
| + |
| +```go |
| +// internal/release/golden_test.go — joins the byte-identical guarantees. |
| +// A manifest written before capabilities existed round-trips and yields none. |
| +func TestYesterdaysManifestParsesWithNoCapabilities(t *testing.T) { |
| + const yesterday = `{ |
| + "schema": 1, |
| + "app": "kass", |
| + "version": "9f3c1ad", |
| + "kind": "binary", |
| + "artifacts": { "linux-arm64": { "file": "linux-arm64", "sha256": "` + strings.Repeat("ab", 32) + `", "size": 12 } } |
| +}` |
| + var m Manifest |
| + if err := json.Unmarshal([]byte(yesterday), &m); err != nil { |
| + t.Fatalf("a manifest already in every production bucket stopped parsing: %v", err) |
| + } |
| + if m.Capabilities != nil { |
| + t.Fatalf("Capabilities = %+v, want nil (absent means none)", m.Capabilities) |
| + } |
| + if err := validateManifest(&m); err != nil { |
| + t.Fatalf("validate: %v", err) |
| + } |
| + // And a capability-free manifest still marshals with NO capabilities key. |
| + raw, _ := json.Marshal(&m) |
| + if bytes.Contains(raw, []byte("capabilities")) { |
| + t.Fatalf("a capability-free manifest grew a key: %s", raw) |
| + } |
| +} |
| + |
| +func TestShipRefusesAnUnknownCapability(t *testing.T) // Ship() with a Set carrying an out-of-range port -> error naming the port |
| +func TestAdoptRefusesTheVersionWhenAManifestDeclaresAnUnknownName(t *testing.T) |
| +// ^ write the manifest bytes DIRECTLY to the store (bypassing Ship, as a newer |
| +// binary would), promote, run Pass, then assert: no live symlink, nothing |
| +// staged, and the printed line names the capability. This is the fail-closed |
| +// direction, and it is the test that matters most in this task. |
| +``` |
| + |
| +- [ ] **Step 2:** `go test ./internal/release/ -count=1` — FAIL. |
| +- [ ] **Step 3: Implement.** Field + `validateManifest` line + the four call sites for the `-capabilities` input. |
| +- [ ] **Step 4:** `go build ./... && go vet ./... && go test ./... -count=1` — all clean, all packages. `gofmt -l` on touched files empty. |
| +- [ ] **Step 5: Commit** `release: additive capabilities on the manifest — one gate shared by ship and adopt` |
| + |
| +**What the reviewer should try to break:** does an OLD binary (no `caps` package) still parse a NEW manifest? — it must, via Go's decoder ignoring unknown top-level keys; state that in the commit message and confirm no `DisallowUnknownFields` exists anywhere on the manifest read path. Does `Ship`'s idempotent re-ship still work — `putImmutable` compares bytes, and a `nil` `Capabilities` must marshal identically to before? Does `handleAPIShip`'s existing "identical content already shipped" short-circuit (which compares **artifacts**, not the manifest) now let a *changed capability block* re-ship under the same version silently? **It does, and that is a real hole:** pin it — when `GetManifest` succeeds and the artifacts match but the stored `Capabilities` differ from the incoming ones, answer `409` with "version %s was already shipped with different capabilities — versions are immutable". Add that test. |
| + |
| +--- |
| + |
| +### Task 3: the staged sibling manifest |
| + |
| +**Model:** implement **Sonnet** (precise brief, two files); review **Opus** (it is on the adopt path). |
| + |
| +**Files:** |
| +- Modify: `internal/release/versions.go` (new path helper + writer + reader, beside `StageArtifact` at :126-149) |
| +- Modify: `internal/release/adopt.go` (`ensureStaged` at :355-378) |
| +- Test: `internal/release/versions_test.go`, `internal/release/adopt_test.go` |
| + |
| +**Interfaces:** |
| + |
| +```go |
| +// versions.go |
| + |
| +// StagedManifestPath is the staged binary's SIBLING file: |
| +// <opt>/versions/<ver>/[<acct>/]<app>.manifest.json |
| +// A sibling, not a new directory, so an old box's tree stays valid and no |
| +// directory shape changes. This is what lets the converge pass read a RUNNING |
| +// version's capabilities without a bucket round-trip on every tick. |
| +func StagedManifestPath(optDir, acct, ver, app string) string // VersionPath(...) + ".manifest.json" |
| + |
| +// StageManifest writes the manifest bytes atomically beside the binary, 0644. |
| +func StageManifest(optDir, acct, ver, app string, raw []byte) error |
| + |
| +// ReadStagedManifest parses the sibling. It returns (nil, nil) — NOT an error — |
| +// when the file is absent: a box staged before this feature existed has a |
| +// binary and no sibling, and the converge pass must read that as "unknown", not |
| +// as "no capabilities". The distinction is load-bearing: unknown LEAVES THE |
| +// DROP-INS ALONE (spec §4d), none REMOVES them. |
| +func ReadStagedManifest(optDir, acct, ver, app string) (*Manifest, error) |
| +``` |
| + |
| +`ensureStaged` changes in exactly two ways: |
| + |
| +1. The early return at :356 (`os.Stat(VersionPath(...)) == nil → return nil`) becomes: if the binary is staged **and** the sibling manifest exists, return nil; if the binary is staged and the sibling is **missing**, fall through to fetch the manifest and write only the sibling (never re-download the artifact). This is the back-fill for every box that staged a version before this change — without it, the converge pass is permanently blind on exactly the versions already running. |
| +2. After a successful `GetManifest`, marshal it (`json.MarshalIndent(m, "", " ")`) and `StageManifest` it. A failure to write the sibling is **not** fatal to staging the binary — log and continue; a binary that runs with an unreadable capability sibling is the leave-alone state, which is safe. |
| + |
| +- [ ] **Step 1: Failing tests.** |
| + - `TestEnsureStagedWritesTheSiblingManifest`: after a `Pass`, `ReadStagedManifest` returns a manifest whose `App`/`Version` match and whose `Capabilities` round-trip. |
| + - `TestEnsureStagedBackfillsASiblingForAnAlreadyStagedBinary`: `StageArtifact` the binary by hand (no sibling), run `Pass`, assert the sibling now exists and the binary's mtime/bytes are unchanged (no re-download). |
| + - `TestReadStagedManifestIsNilNilWhenAbsent`. |
| + - `TestStagedManifestPathIsASiblingForBothAccountShapes`: ops (`<opt>/versions/v1/app.manifest.json`) and non-ops (`<opt>/versions/v1/<sqid>/app.manifest.json`). |
| +- [ ] **Step 2:** `go test ./internal/release/ -count=1` — FAIL. |
| +- [ ] **Step 3: Implement. Step 4:** `go build ./... && go vet ./... && go test ./... -count=1` clean. **Step 5: Commit** `release: stage the manifest beside the binary — the converge pass reads capabilities off disk` |
| + |
| +**What the reviewer should try to break:** the sibling path colliding with an app literally named `foo.manifest` (it cannot — `ValidApp` forbids `.`); a partially-written sibling after a crash (the write must be atomic — reuse `WriteFileAtomic`); the sibling surviving a re-stage of the same version (it must be overwritten, not appended); `StageArtifact`'s `0755` dir chmod still applying so the sandboxed unit can traverse. |
| + |
| +--- |
| + |
| +### Task 4: grants — the `caps` column, the narrow setters, and `carlos route -grant/-revoke` |
| + |
| +**Model:** implement **Opus** (this is the operator's door to a transfer of box authority); review **Opus**. |
| + |
| +**Files:** |
| +- Modify: `internal/registry/registry.go` (`Route` struct :29-103, `migrate` ALTER list :163-186, `routeCols` :344-347, `scanRoute` :349-362, `Upsert` :427-465, new setters beside `SetChannel` :482) |
| +- Modify: `internal/caps/caps.go` (grant token vocabulary — additive to Task 1's file, or a new `internal/caps/grants.go`) |
| +- Modify: `cli.go` (`runRoute` :259-651, `runAdd` :62-134) |
| +- Test: `internal/registry/registry_test.go`, `internal/caps/grants_test.go`, `cli_test.go` if one exists for route (else table-test the pure rail helpers) |
| + |
| +**Interfaces:** |
| + |
| +```go |
| +// caps/grants.go |
| +type Grants struct { |
| + Ports map[int]bool |
| + HostRole bool |
| +} |
| + |
| +const HostRoleToken = "host-role" |
| +func PortToken(p int) string // "port:25" |
| + |
| +// ParseGrants reads the registry column. Unknown or malformed tokens are an |
| +// ERROR, not a silent skip: this column is only ever written by the narrow |
| +// setter below, so anything else in it means the row was edited by hand and the |
| +// honest answer is to refuse to materialize rather than guess a subset. |
| +func ParseGrants(col string) (Grants, error) |
| + |
| +// String is the canonical column value: tokens sorted lexicographically |
| +// ("host-role,port:25"), so a re-grant of the same set writes the same bytes. |
| +func (g Grants) String() string |
| + |
| +// ValidGrantToken is the rail an operator meets. It refuses: an unknown token |
| +// shape; a port outside 1..1023 (a high port needs no grant); and 22/80/443, |
| +// which are reserved ABSOLUTELY and are not grantable — an operator typo must |
| +// not be able to hand a tenant the edge's ports or the box's SSH. |
| +func ValidGrantToken(tok string) error |
| + |
| +// Grant/Revoke return the new canonical column value. |
| +func Grant(col, tok string) (string, error) |
| +func Revoke(col, tok string) (string, error) |
| +``` |
| + |
| +```go |
| +// registry.go |
| +type Route struct { |
| + // ... |
| + // Caps is the OPERATOR-granted capability tokens for this route ON THIS BOX, |
| + // comma-separated and canonically sorted ("port:25,host-role" normalises to |
| + // "host-role,port:25"). Written ONLY by SetCaps, behind |
| + // `carlos route -grant/-revoke`. Default '' = nothing granted = today's |
| + // behaviour for every row already on every box. |
| + // |
| + // Deliberately NOT in Fingerprint(): grants change unit shape, not routing, |
| + // and the edge has no reason to reload for one. |
| + Caps string |
| +} |
| +// migrate ALTER list gains: |
| +// `ALTER TABLE routes ADD COLUMN caps TEXT NOT NULL DEFAULT ''` |
| +// routeCols gains ", caps" (append at the END, before created_at is fine as |
| +// long as scanRoute and Upsert's placeholder count move with it — count them). |
| + |
| +// SetCaps writes the grant column as a narrow UPDATE, erroring when no row |
| +// matched (a human types a hostname into `carlos route -grant` directly, and a |
| +// silent no-op on a typo would be worse than a refusal). Same contract and same |
| +// rationale as SetChannel. |
| +func (r *Registry) SetCaps(host, caps string) error |
| + |
| +// SetDBPath stamps a route's database path narrowly — the converge pass uses it |
| +// to teach ops.FleetDBs where a `data.replicate: platform` app's databases are |
| +// (Task 8), instead of depending on an operator having typed -db at `carlos add`. |
| +func (r *Registry) SetDBPath(host, path string) error |
| +``` |
| + |
| +`Upsert` must **preserve** a non-empty existing `caps` exactly the way it preserves `environment` (`caps=CASE WHEN excluded.caps = '' THEN routes.caps ELSE excluded.caps END`). This is the issue-#14/IMPORTANT-3 lesson applied to a new column: `carlos add` re-upserts and the domains reconciler re-mints, and neither knows anything about grants — a plain `excluded.caps` would silently revoke `:25` on a re-add. |
| + |
| +CLI surface on `runRoute`: |
| + |
| +``` |
| +carlos route -host keymail.dev -grant port:25 |
| +carlos route -host platform.carlosframework.com -grant host-role |
| +carlos route -host keymail.dev -revoke port:25 |
| +carlos route -host keymail.dev -hibernate=false |
| +``` |
| + |
| +Rails, all of which are refusals with no override: |
| +- `-grant`/`-revoke` require `-host`. **Never `-app`**: grants are per route per box by design (decision 4), and a bulk grant is precisely the operation nobody should be able to type by accident. |
| +- `-grant`/`-revoke` are mutually exclusive with each other and with `-channel`/`-environment`. A grant change is a different operation from a repoint; combining them would put two unrelated writes in one transaction with two unrelated rails. |
| +- The existing "at least one of -channel or -environment is required" check is relaxed to "at least one of -channel, -environment, -grant, -revoke, -hibernate". |
| +- `platformCanaryOwnedRow(rt)` refuses `-grant`/`-revoke` for the same structural reason it refuses the other two axes: the reconciler rebuilds that row. (`Upsert`'s preservation makes it survivable, but survivable is not owned.) |
| +- The route must be `Kind == "instance"` and `Backing == registry.BackingUnit`; anything else is refused naming the reason ("capabilities are only honoured for unit-backed instance routes — see the backing advisory"). |
| +- `ValidGrantToken` runs before the registry is opened for writing. |
| +- Granting a port already granted to a DIFFERENT route on this box is refused, naming the other host. This is the operator-facing half of Task 6's `CheckClaims`; both exist because a grant is typed once and converge runs hourly. |
| +- `-hibernate=true` on a route whose live release declares `daemon` is refused. Resolution follows `runRoute`'s existing doctrine exactly: a *proven* daemon refuses; an *unproven* comparison (no pubkey, unresolvable channel, bad signature) also refuses, because "I could not find out" is never a licence. `-hibernate=false` needs no proof and always proceeds. Use `registry.SetHibernate`, which already exists and has had no CLI caller until now. |
| +- `runAdd`: `-hibernate` plus an app whose channel resolves to a `daemon` manifest is refused with the same message. Unlike `carlos route`, an **unresolvable** comparison here proceeds silently — `carlos add` runs at provision time on boxes with no pubkey and no release yet, and refusing there would break provisioning. The authoritative refusal is the converge pass (Task 6), which sees the manifest every hour. |
| + |
| +- [ ] **Step 1: Failing tests.** |
| + - `caps`: `ParseGrants("host-role,port:25")` round-trips; `ParseGrants("port:99999")` errors; `ParseGrants("")` is the zero Grants with no error; `Grant`/`Revoke` are idempotent and canonically sorted; `ValidGrantToken` refuses `port:22`, `port:80`, `port:443`, `port:8080`, `port:0`, `hostrole`, `host-role:1`, `""`. |
| + - `registry`: a registry file created **before** this change (open, insert a row, close, reopen with the new binary) still opens and the row reads `Caps == ""`; `SetCaps` on an unknown host errors; `Upsert` of a full row with empty `Caps` preserves an existing grant; `Fingerprint()` is **unchanged** by a `SetCaps` call (pin it — this is what keeps the edge from reloading on a grant). |
| + - CLI rails: table-test the pure predicate helpers you extract (`grantRail(rt, tok) error`), covering every refusal above. |
| +- [ ] **Step 2:** `go test ./internal/registry/ ./internal/caps/ -count=1` — FAIL. |
| +- [ ] **Step 3: Implement. Step 4:** `go build ./... && go vet ./... && go test ./... -count=1` clean. |
| +- [ ] **Step 5: Commit** `registry: per-route capability grants; cli: carlos route -grant/-revoke with the rails` |
| + |
| +**What the reviewer should try to break:** the placeholder count in `Upsert`'s `VALUES (?,?,...)` versus `routeCols` — miscounting there is a silent column shift that would write a grant into `created_at`; **count them by hand and assert a full round-trip of every column in a test**. `Fingerprint()`'s `SUM(...)` expression must not gain `caps`. A grant token containing a comma or a newline. `carlos route -host x -grant port:25 -revoke port:25`. A revoke of a token that was never granted (must be a quiet success — the caller's goal is "not granted", not "I removed it"). Whether `-hibernate` is a `flag.Bool` (which cannot distinguish "unset" from "false") — it must be tri-state, e.g. a `*string` with `""|"true"|"false"`, or the flag must be checked with `fs.Visit`. |
| + |
| +--- |
| + |
| +### Task 5: the drop-in renderer — deterministic, golden-file tested |
| + |
| +**Model:** implement **Sonnet** (exact output is pinned in this task); review **Opus**. |
| + |
| +**Files:** |
| +- Create: `internal/caps/render.go`, `internal/caps/render_test.go` |
| +- Create: `internal/caps/testdata/keymail.service.conf`, `keymail.socket.conf`, `console.service.conf` |
| + |
| +**Interfaces:** |
| + |
| +```go |
| +// RenderService returns the [Service] drop-in body for the capabilities that |
| +// are BOTH declared by s and (for the operator-consented ones) granted by g, or |
| +// NIL when the effective set writes no [Service] directive at all. Nil means |
| +// "remove the generated file", not "write an empty stanza" — one generated file |
| +// per unit, and removal semantics are the reason (spec §3c). |
| +// |
| +// Directive order is FIXED and lists are sorted, so an unchanged pass writes no |
| +// bytes, triggers no daemon-reload, and restarts nothing. |
| +func RenderService(app string, s *Set, g Grants) []byte |
| + |
| +// RenderSocket returns the [Socket] drop-in body, or nil when no declared port |
| +// is also granted. |
| +func RenderSocket(s *Set, g Grants) []byte |
| +``` |
| + |
| +Exact output, pinned (these ARE the golden files): |
| + |
| +`carlos-app@keymail.dev.service.d/10-carlos-capabilities.conf` |
| +```ini |
| +# GENERATED by carlos adopt — do not hand-edit |
| +[Service] |
| +ExecStart= |
| +ExecStart=/opt/carlos/live/%i relay |
| +RuntimeDirectory=keymail |
| +RuntimeDirectoryMode=0700 |
| +RuntimeDirectoryPreserve=restart |
| +KillMode=control-group |
| +TasksMax=1024 |
| +MemoryMax=1G |
| +DynamicUser=no |
| +User=carlos-keymail |
| +Group=carlos-keymail |
| +``` |
| + |
| +`carlos-app@keymail.dev.socket.d/10-carlos-capabilities.conf` |
| +```ini |
| +# GENERATED by carlos adopt — do not hand-edit |
| +[Socket] |
| +ListenStream=25 |
| +``` |
| + |
| +`carlos-app@platform.carlosframework.com.service.d/10-carlos-capabilities.conf` |
| +```ini |
| +# GENERATED by carlos adopt — do not hand-edit |
| +[Service] |
| +DynamicUser=no |
| +User=carlos-console |
| +Group=carlos-console |
| +``` |
| + |
| +Rules the renderer encodes: |
| + |
| +- Section order: `command` block, then `runtime_dir`, then `children`, then `host_role`. Ports are the socket unit's alone. |
| +- `command` emits the empty `ExecStart=` reset FIRST. Without it the unit has two `ExecStart` lines and refuses to start; this is systemd's documented way to reset a list-valued directive from a drop-in. |
| +- The ExecStart target is the literal `/opt/carlos/live/%i`, **byte-identical to the template's own line** — the template is not parameterised by `CARLOS_OPT_DIR` either, and a drop-in that disagreed with the template about the path would be a silent split brain. |
| +- `daemon` emits **no directives**. It is enable/start/restart behaviour, not unit text (Task 6 owns it). |
| +- `data` emits **no directives**. `StateDirectory=carlos-app/%i` is already in the template and the manifest may not name a path; what `data` adds is which files are databases and who replicates them (Task 8). |
| +- `children` emits exactly `KillMode=control-group`, `TasksMax=1024`, `MemoryMax=1G` — a platform-chosen tier. `infinity` is unsayable and no number comes from a manifest. |
| +- `host_role` emits `DynamicUser=no`, `User=<AppUser(app)>`, `Group=<AppUser(app)>`, and only when `g.HostRole` is true. |
| +- Ports emit one `ListenStream=<port>` per granted port, **sorted ascending**. |
| +- Every body ends with exactly one trailing newline. |
| + |
| +- [ ] **Step 1: Failing tests.** |
| + - Three golden comparisons, byte for byte, against `testdata/`. Write the fixture files by hand from the block above — **do not generate them from the code you are testing.** |
| + - `TestRenderIsDeterministic`: render the keymail set twice from two independently-decoded `Set` values (one with `ports` given as `[{25,"mx"},{25,"mx"}]`… no — as `[{587,"submission"},{25,"mx"}]` with both granted) and assert identical bytes and ascending order. |
| + - `TestUngrantedCapabilitiesMaterializeNothing`: keymail's full set with `Grants{}` renders a service body with the command/runtime_dir/children lines but **no** `DynamicUser=no`/`User=`, and a **nil** socket body. keymail with no `port:25` grant serves HTTP and does not bind the MX; it does not refuse to start. |
| + - `TestEmptyEffectiveSetRendersNil`: `&Set{V:1, Daemon:true, Data:&Data{Replicate:"platform",DBs:[]string{"console.db"}}}` with `Grants{}` → both bodies nil. |
| + - `TestRenderStartsWithTheMarker` for every non-nil body. |
| +- [ ] **Step 2:** `go test ./internal/caps/ -count=1` — FAIL. **Step 3: Implement. Step 4:** PASS + `go build ./... && go vet ./...` clean. |
| +- [ ] **Step 5: Commit** `caps: deterministic drop-in renderer, golden-pinned for console and keymail` |
| + |
| +**What the reviewer should try to break:** an app name that would produce `User=carlos-` (empty app — `ValidApp` forbids it, but assert the renderer is never reachable with one). A `command` of `serve` (identical to the template's — the reset+rewrite is still correct and still deterministic). Whether `MemoryMax=1G` and the template's `MemoryMax=512M` interact as expected (the drop-in wins; `10-` sorts before an operator's `90-`, and both sort after the template). Whether a nil `*Set` renders nil rather than panicking. |
| + |
| +--- |
| + |
| +### Task 6: the converge pass — files, marker, users, systemd, and the box-wide claims |
| + |
| +**Model:** implement **Opus** (this is the multi-file, production-touching heart of the change); review **Opus**. |
| + |
| +**Files:** |
| +- Create: `internal/caps/converge.go`, `internal/caps/converge_test.go` |
| + |
| +**Interfaces:** |
| + |
| +```go |
| +// Ops is every piece of the outside world this package touches, injected so the |
| +// converge tests need no systemd, no root, and no /etc — the same seam shape |
| +// activator.UnitBackend.Systemctl uses. |
| +type Ops struct { |
| + EtcDir string // "" = DefaultEtcDir |
| + StateRoot string // "" = DefaultStateRoot |
| + Systemctl func(args ...string) error // nil = the real systemctl binary |
| + EnsureUser func(name string) error // nil = the real, idempotent useradd |
| + Logf func(format string, a ...any) // nil = fmt.Printf |
| +} |
| + |
| +const ( |
| + DefaultEtcDir = "/etc/systemd/system" |
| + DefaultStateRoot = "/var/lib/carlos-app" |
| + DropInName = "10-carlos-capabilities.conf" |
| +) |
| + |
| +// Plan is one route's converge input. |
| +type Plan struct { |
| + Host string |
| + App string |
| + Set *Set // NIL means the manifest did not resolve — LEAVE THE DROP-INS ALONE |
| + Grants Grants |
| + Hibernate bool |
| +} |
| + |
| +// Snapshot is what was on disk before Converge wrote, enough to put it back. |
| +type Snapshot struct { |
| + Host string |
| + Service, Socket []byte |
| + ServiceHad, SocketHad bool |
| +} |
| + |
| +type Result struct { |
| + Prev Snapshot |
| + Changed bool // any byte written or removed; the caller need not daemon-reload, Converge already did |
| +} |
| + |
| +// Converge materializes p and returns what it replaced. |
| +// |
| +// The three states are NOT interchangeable, and this is the same rule |
| +// appconfig.Sync uses for exactly the same reason: a bucket hiccup must not |
| +// strip :25 off a running mail server. |
| +// - p.Set == nil (absent or unparseable staged manifest): leave every drop-in |
| +// exactly as it is, log one line, return Changed=false. A capability is only |
| +// ever removed because a manifest that ACTUALLY RESOLVED no longer declares |
| +// it, or because a grant was revoked. |
| +// - p.Set non-nil, renders bytes: write them if they differ. |
| +// - p.Set non-nil, renders nil: REMOVE the generated file (marker-checked). |
| +func Converge(o Ops, p Plan) (Result, error) |
| + |
| +// Restore puts prev back and daemon-reloads — the drop-in half of adoptHost's |
| +// restart-failure rollback (the symlink half is release.PointLive's). |
| +func Restore(o Ops, prev Snapshot) error |
| + |
| +// Restart is the daemon route's restart: `systemctl restart`, and its error is |
| +// REAL. Non-daemon routes are not this function's business — restartRoute's |
| +// try-restart keeps owning them, byte-identically. |
| +func Restart(o Ops, p Plan) error |
| + |
| +// CheckClaims resolves box-wide exclusivity BEFORE anything is written, across |
| +// every plan on this box at once: a privileged port claimed by two routes, and |
| +// a runtime_dir claimed by two routes. Returns host -> refusal; a host absent |
| +// from the map cleared. Deterministic on ties: the route whose Host sorts first |
| +// keeps the claim, the other is refused — so a conflict does not flap between |
| +// passes. |
| +func CheckClaims(plans []Plan) map[string]error |
| + |
| +// DropInPaths returns the two generated file paths for a host. |
| +func DropInPaths(etcDir, host string) (service, socket string) |
| +// -> <etc>/carlos-app@<host>.service.d/10-carlos-capabilities.conf |
| +// -> <etc>/carlos-app@<host>.socket.d/10-carlos-capabilities.conf |
| +``` |
| + |
| +Converge's order of operations, pinned: |
| + |
| +1. **Refuse `daemon` + `Hibernate`.** A daemon the platform is asked to keep running and to stop when idle is a contradiction, and hibernation for these apps means an MX that sleeps. Write nothing, change no unit state, log one line per pass naming the exact fix: `carlos route -host <h> -hibernate=false`. Return `Changed=false`. |
| +2. **`host_role` state-dir shape check (spec §4b's pinned sharp edge).** `os.Lstat(StateRoot/<host>)`. With `DynamicUser=yes` systemd materialises the state dir under `/var/lib/private/` with a **symlink** at `/var/lib/carlos-app/<host>`; with a static `User=` it expects a real directory. So: turning `host_role` ON while that path is an existing symlink, or OFF while it is an existing real directory that the unit currently runs as a static user against, is **refused in place** — log, skip, leave everything alone, name the operator's fix (stop → move → start). An absent path is fine in both directions. Neither console nor keymail is a DynamicUser tenant today, so both migrate directly onto the static shape with no conversion — this guard is for the third app. |
| +3. **Create the user** when `host_role` is declared *and* granted: `EnsureUser(AppUser(app))`, idempotent, locked, no home, `nologin` — exactly as `host.ensureServiceUser` creates `carlos`. The name is derived by the platform, never taken from a manifest. A failure here refuses the whole plan (do not write a `User=` line for a user that does not exist — the unit would fail to start and `Restart=always` would spin it). |
| +4. **Snapshot** both paths. A file at a generated path **without** the marker on its first line is **never overwritten and never deleted**: it is reported (one line per pass) and that unit's half is skipped entirely, while the other half still converges. |
| +5. **Write or remove**, atomically (temp + rename), root-owned, `0644`, creating the `.d` directory `0755`. Byte-identical content is a no-op: no write, no reload, no restart. |
| +6. **`daemon-reload`** exactly once, and only if step 5 changed something. |
| +7. **Socket restart ordering, when the SOCKET drop-in changed** — this is the fiddly part and the spec does not spell it out. A running service holds the bound fd, so restarting the socket unit under it races into `EADDRINUSE`. The pinned sequence is: |
| + `systemctl stop carlos-app@<h>.service` → `systemctl restart carlos-app@<h>.socket` → `systemctl start carlos-app@<h>.service`. |
| + The listener is down only for the socket's own restart, during which inbound connections queue in the kernel backlog — which is what makes spec §4c's "an MX restart never refuses a connection" true rather than aspirational. |
| +8. **Enable + start, for `daemon` routes:** `systemctl enable carlos-app@<h>.service` (so it survives reboot) and, when a socket drop-in exists, `systemctl enable carlos-app@<h>.socket` too — **without this the `ListenStream` binds nothing**, because bootstrap does not enable tenant socket units and the template's own comment says "enable per tenant". Then `systemctl start` if inactive. `enable`/`start` are idempotent, so an unchanged pass is quiet. |
| +9. If the SERVICE drop-in changed and the route is a `daemon`, `Restart`. If it changed and the route is not a daemon, do nothing — the caller's existing `try-restart` owns it, byte-identically to today. |
| + |
| +- [ ] **Step 1: Failing tests** (all at the `Systemctl` seam, `t.TempDir()` as `EtcDir` and `StateRoot`, a recording `Systemctl`, a recording `EnsureUser`): |
| + - `TestGrantAddedWritesTheDropInAndRestarts`: keymail plan with `Grants{Ports:{25:true},HostRole:true}` → both files exist with the golden bytes, `daemon-reload` recorded once, the socket sequence recorded in the pinned order, `enable` recorded for **both** units. |
| + - `TestGrantRevokedRewritesTheDropIn`: same plan, `Grants{}` → the socket file is **removed**, the service file loses its `User=` lines, one `daemon-reload`. |
| + - `TestUnreadableManifestLeavesDropInsAlone`: converge once with a Set, then converge with `Set: nil` → both files byte-identical to before, `Changed == false`, no systemctl call at all, and one log line. |
| + - `TestHandWrittenDropInIsNeverOverwritten`: pre-place a file with no marker → converge leaves its bytes untouched, reports it, and still converges the other unit. |
| + - `TestUnchangedPassWritesNothing`: converge twice → the second call records **zero** systemctl invocations and `Changed == false`. |
| + - `TestDaemonPlusHibernateIsRefused`: nothing written, no systemctl, the log line names `-hibernate=false`. |
| + - `TestHostRoleFlipRefusedWhenTheStateDirIsTheOtherShape`: `os.Symlink` a fake `/var/lib/private/...` at `StateRoot/<host>`, converge with `host_role` granted → refused in place, nothing written. |
| + - `TestRestoreputsThePreviousDropInsBack`: converge A, converge B capturing `Result.Prev`, `Restore(prev)` → bytes equal A's, and a `daemon-reload` recorded. |
| + - `TestCheckClaimsRefusesAPortHeldByTwoRoutesAndARuntimeDirCollision`: two plans both claiming `port:25`, and two both claiming `runtime_dir: keymail` → the lexicographically-later host is refused in each case, deterministically across repeated runs. |
| +- [ ] **Step 2:** `go test ./internal/caps/ -count=1` — FAIL. **Step 3: Implement. Step 4:** PASS, then `go build ./... && go vet ./... && go test ./... -count=1` clean. |
| +- [ ] **Step 5: Commit** `caps: the converge pass — three-state, marker-safe, socket-ordered, operator-refusing` |
| + |
| +**What the reviewer should try to break:** a `Host` carrying a `/` or `..` (a registry row is trusted at the same level as `live/<host>`, but this one joins into a **root-written** `/etc/systemd/system` path — refuse anything where `rt.Host != filepath.Base(rt.Host)`, exactly as `appconfig.syncHost` does at :293). A drop-in directory that exists but is a file. A `Systemctl` that fails mid-sequence in step 7 (the service must not be left stopped — assert the start is attempted even when the socket restart failed, and that the error is still returned). `EnsureUser` succeeding while the user genuinely does not exist. Whether `Changed` can be true when only a *removal* happened. Whether two converges racing (hourly `RunAdopt` and a version-change converge inside `Pass`) can interleave — they are both in the same process on the same box, but `carlos adopt` run by hand while `carlos-adopt.timer` fires is two processes: state whether the atomic rename makes that safe (it does for the file; it does not for the systemctl sequence — say so and accept it, as the repo already accepts it for `appconfig.Sync`). |
| + |
| +--- |
| + |
| +### Task 7: converge inside `adoptHost` — before `PointLive`, rolled back with it |
| + |
| +**Model:** implement **Opus** (the adopt path, and the rollback is the thing that keeps a bad build off a mail server); review **Opus**. |
| + |
| +**Files:** |
| +- Modify: `internal/release/adopt.go` (`PassOption`/`passConfig` :29-36, `adoptHost` :275-348) |
| +- Modify: `internal/host/agent.go` (`restartRoute` :712-718, and the `Pass`/`PassFilter` call sites at :218, :302, :783) |
| +- Test: `internal/release/adopt_test.go` |
| + |
| +**Interfaces:** |
| + |
| +```go |
| +// adopt.go — a new PassOption, following NagExpiredCanaries' shape exactly. |
| +// |
| +// WithCapabilities arms the capability converge inside adoptHost. Unset (every |
| +// existing caller and every existing test) means converge never runs and this |
| +// package's behaviour is byte-identical to today. The Ops value carries the |
| +// seams, so a test injects a temp dir and a recording systemctl. |
| +func WithCapabilities(o caps.Ops) PassOption |
| + |
| +// passConfig gains: |
| +// capsOps *caps.Ops // nil = capability converge disabled for this pass |
| +``` |
| + |
| +`adoptHost`'s version-change path becomes, in order: |
| + |
| +``` |
| +ensureStaged (binary + sibling manifest) // unchanged except Task 3 |
| + ↓ |
| +converge capabilities for the NEW version // NEW |
| + ↓ |
| +PointLive // unchanged |
| + ↓ |
| +restart // daemon: caps.Restart; else the existing callback |
| + ↓ (on failure) |
| +poison the version + PointLive back + caps.Restore(prev) // NEW second half |
| +``` |
| + |
| +Guard rails on when converge runs at all, mirroring `adoptable` and `syncable` (spec §4e): |
| + |
| +```go |
| +// capabilityRoute reports whether this route may materialize capabilities: |
| +// a lifecycle-managed instance of a known app, unit-backed, non-platform, |
| +// ready. A platform ALIAS never materialises anything — an alias is a second |
| +// name for one process, and a second carlos-app@ unit claiming :25 would break |
| +// the real one. adoptable() already excludes them; this depends on that. |
| +func capabilityRoute(rt registry.Route) bool { |
| + return adoptable(rt) && rt.Backing == registry.BackingUnit |
| +} |
| +``` |
| + |
| +Because every route in every existing `release` test is exec-backed (`Backing == ""`), **no existing test can reach the converge path**, and no test can write to a real `/etc`. Say that in the commit message; it is the reason this task is safe to land independently. |
| + |
| +`restartRoute` (host) changes minimally: it keeps `systemctl try-restart` and its swallow-and-continue for every route it handles today — byte-identical — and gains nothing. The daemon restart does **not** go through it: `adoptHost` calls `caps.Restart` directly for a converged daemon plan, because that is the only call site that knows the plan and it is the only one whose error must be real. Today `restartRoute` returns `nil` unconditionally, which means the existing rollback path is dead for unit routes; this is what revives it, for daemons only. |
| + |
| +- [ ] **Step 1: Failing tests** (in `internal/release`, with `caps.Ops{EtcDir: t.TempDir(), StateRoot: t.TempDir(), Systemctl: recorder, EnsureUser: recorder}`): |
| + - `TestAdoptConvergesCapabilitiesBeforePointLive`: assert ordering by having the recording `Systemctl` capture `LiveVersion(opt, host)` at call time — the drop-in write and `daemon-reload` must be observed while the symlink still names the OLD version. |
| + - `TestRestartFailureRestoresBothTheSymlinkAndTheDropIns`: converge version A, then promote B whose `Restart` fails → after the pass, `LiveVersion == "A"`, both drop-in files byte-identical to A's, the version poisoned for the rest of the pass, and the printed line says the symlink was rolled back. |
| + - `TestExecBackedRouteWithCapabilitiesMaterializesNothing`: a `Backing: ""` route whose manifest declares the full keymail set → the `EtcDir` is empty after the pass, and adoption otherwise proceeds normally. |
| + - `TestPlatformAliasRouteMaterializesNothing`: `Platform: true` → same. |
| + - `TestPassWithoutWithCapabilitiesIsUnchanged`: a unit-backed route with a full capability manifest and **no** `WithCapabilities` option → `EtcDir` untouched, adoption proceeds. This is the byte-identical-to-today assertion. |
| +- [ ] **Step 2:** `go test ./internal/release/ -count=1` — FAIL. **Step 3: Implement.** Wire `WithCapabilities(caps.Ops{})` (the zero value = real paths, real systemctl) into `RunAdopt`'s `release.Pass` call at agent.go:783 **only**. Deliberately **not** into `adoptLoop` (:218) or `canaryAdoptLoop` (:302): both run inside the sandboxed `carlos-edge.service`, which has `ProtectSystem=strict` and `ReadWritePaths=/data /run/carlos` and therefore **cannot write `/etc/systemd/system` at all** — arming it there would produce a permission-denied log line every 2-3 seconds forever. Put that reasoning in a comment at both call sites, in the same loud style as the `NagExpiredCanaries` "DO NOT add this" comments already there. |
| +- [ ] **Step 4:** `go build ./... && go vet ./... && go test ./... -count=1` clean. **Step 5: Commit** `release: converge capabilities before PointLive, and roll both back together` |
| + |
| +**What the reviewer should try to break:** the canary fast loop picking up a capability-bearing version every 2-3 seconds and **not** converging it — meaning a freshly promoted canary runs the new binary against the old unit shape until the hourly root pass catches up. Decide and document: that window is accepted (the drop-in is repaired within the hour, and the alternative is the sandbox denial above), or the fast loop must skip capability routes entirely. **Recommended: accept, and log one line from the fast loop when it repoints a capability route so the window is visible.** Also: a hibernating unit route reaching `adoptHost` with `restart == nil` — converge must still run (its drop-ins are unit text, not a wake), but nothing may start it. |
| + |
| +--- |
| + |
| +### Task 8: `RunAdopt` — the grant-drift pass, the three advisories, and the DBPath stamp |
| + |
| +**Model:** implement **Opus** (multi-file, root-side, and one advisory prints a firewall command); review **Opus**. |
| + |
| +**Files:** |
| +- Modify: `internal/host/agent.go` (`RunAdopt` :745-790) |
| +- Create: `internal/host/caps_pass.go`, `internal/host/caps_pass_test.go` |
| +- Modify: `internal/host/adopt_advice.go`, `internal/host/adopt_advice_test.go` |
| + |
| +**Interfaces:** |
| + |
| +```go |
| +// caps_pass.go |
| + |
| +// capabilityPass is RunAdopt's step 2 (spec §4d): converge every adoptable |
| +// unit route from the STAGED MANIFEST OF ITS LIVE VERSION. This is what a |
| +// GRANT change moves through — a deleted drop-in, a fresh grant, a revoked |
| +// grant, a unit that is not enabled, a daemon that is not running — while |
| +// adoptHost's converge is what a VERSION change moves through. Both call the |
| +// same caps.Converge and running both is idempotent. |
| +// |
| +// Pure enough to test: the registry, the opt dir and the caps.Ops seams are all |
| +// arguments, so this needs no systemd and no /etc. |
| +func capabilityPass(reg *registry.Registry, optDir string, o caps.Ops) error |
| + |
| +// buildPlans is the pure half — routes + staged manifests -> plans + the |
| +// advisories that fall out of reading them. Extracted so the advisory wording |
| +// is table-testable without touching disk. |
| +func buildPlans(routes []registry.Route, load func(rt registry.Route) (*release.Manifest, error)) ([]caps.Plan, []string) |
| +``` |
| + |
| +`RunAdopt` becomes, in order (spec §4d): |
| + |
| +1. `appconfig.Sync` — unchanged, still first: a route's env exists before its process starts. |
| +2. `capabilityPass(reg, release.OptDir(), caps.Ops{})` — **NEW**. |
| +3. `release.Pass(..., release.NagExpiredCanaries(os.Stdout), release.WithCapabilities(caps.Ops{}))` — unchanged in shape. |
| +4. `adviseBackingMismatches(routes)` plus the new advisories. |
| + |
| +The three advisories, each **one line per pass**, never per tick, never per host-per-capability: |
| + |
| +```go |
| +// adopt_advice.go |
| + |
| +// adviseUngrantedCapabilities: a route whose live release DECLARES an |
| +// operator-consented capability that this box has not GRANTED. Not a failure |
| +// and not an adoption failure — the drop-in simply is not written. Names the |
| +// route, the capability, and the exact command. |
| +// "adopt: keymail.dev declares capability port:25 but this box has not granted it — |
| +// the MX is not bound; grant it with: carlos route -host keymail.dev -grant port:25" |
| + |
| +// adviseCapabilitiesOnExecBacking: an exec-backed route whose release declares |
| +// ANY capability. Materializes nothing (Task 7's guard) and gets the same shape |
| +// as the issue #14 backing advisory — this is the guided path through the §5 |
| +// migration, and the line an operator sees during the reversible dress rehearsal. |
| +// "adopt: route console.example.com declares capabilities but is exec-backed — |
| +// nothing is materialized; stamp backing='unit' (carlos route -host ... -backing unit) |
| +// once its data has been moved (see the app-capabilities design §5)" |
| + |
| +// adviseHostRoleGateMissing: host_role is granted, so the app runs as |
| +// carlos-<app>, but that uid is absent from the box's IMDS nftables gate. The |
| +// platform NEVER writes the firewall rule — the firewall is operator territory |
| +// — it prints the exact command, once per pass, for as long as the gap exists. |
| +// Naming the missing step every hour is the whole lesson of the keymail |
| +// rehearsal's finding F1, where the gate starved the relay's AWS clients and |
| +// the failure mode was silence, not an error. |
| +func adviseHostRoleGateMissing(routes []registry.Route, uidOf func(user string) (int, bool), gateUIDs func() (map[int]bool, bool)) |
| +``` |
| + |
| +`gateUIDs` returns `(uids, ok)`. Its production implementation shells `nft list ruleset` and parses `skuid <n>` occurrences — a pure `parseNftSkuids([]byte) map[int]bool` with a table test, plus thin OS glue, exactly the `parseIsActiveBatch`/`unitsAreActive` split already in this file. Two distinct lines, because "the uid is missing" and "I could not read the gate" are different claims and this repo does not fold them together: |
| + |
| +- `ok && !uids[uid]`: `adopt: carlos-keymail (uid 998) holds the host_role grant on keymail.dev but is not in this box's IMDS gate — its AWS clients will be starved SILENTLY; add it: nft add rule inet filter output ip daddr 169.254.169.254 skuid 998 accept (and add the uid to harden-host.sh so a re-run does not strip it)` |
| +- `!ok`: `adopt: carlos-keymail holds the host_role grant on keymail.dev but this box's IMDS gate could not be read — confirm by hand that the uid is allowed (see the app-capabilities design §4b)` |
| + |
| +The `ops.FleetDBs` stamp, inside `capabilityPass`: |
| + |
| +- `data.replicate == "platform"` with a non-empty `dbs`: sort the names, and `reg.SetDBPath(host, filepath.Join(caps.DefaultStateRoot, host, dbs[0]))` **when it differs from the current value**. This is how `ops.FleetDBs` learns console's database instead of depending on an operator having typed `-db` at `carlos add`, which the current code documents as "silently replicated by nobody". |
| +- `dbs[1:]`: one advisory line each per pass — `DBPath` is single-valued, so the extras are not replicated and the operator is told so by name. |
| +- `data.replicate == "app"` with a non-empty `DBPath`: **never clear it**, advise. Two replicators on one WAL fight over checkpointing and corrupt the replica — the hazard `ops.FleetDBs` already documents. Clearing it silently would be the platform making that call for the operator. |
| + |
| +- [ ] **Step 1: Failing tests.** |
| + - `TestCapabilityPassConvergesFromTheStagedManifestOfTheLiveVersion`: stage two versions, point live at the older, add a grant → the drop-in written reflects the **live** version's manifest, not the newest staged one. |
| + - `TestCapabilityPassStampsDBPathFromDataDBs` and `TestCapabilityPassNeverClearsDBPathForAppReplication`. |
| + - `TestBuildPlansAdvisesUngrantedDeclaredCapabilities` (table): exact wording, one line per (route, capability), and the command string is copy-pasteable. |
| + - `TestBuildPlansAdvisesExecBackedCapabilities`. |
| + - `TestParseNftSkuids` (table): a realistic `nft list ruleset` excerpt with two `skuid` rules; garbage → empty; and `TestAdviseHostRoleGateMissingSaysWhichClaimItIsMaking` for both the `ok` and `!ok` branches. |
| + - `TestCapabilityPassIsQuietWhenNothingChanged`: run twice, second run records zero systemctl calls and prints nothing. |
| +- [ ] **Step 2:** `go test ./internal/host/ -count=1` — FAIL. **Step 3: Implement. Step 4:** PASS, `go build ./... && go vet ./... && go test ./... -count=1` clean. |
| +- [ ] **Step 5: Commit** `host: grant-drift converge in RunAdopt, three advisories, and FleetDBs learns the app's databases` |
| + |
| +**What the reviewer should try to break:** `capabilityPass` running before `release.Pass` means a route mid-version-change converges twice in one pass — assert that is a no-op the second time (it must be: byte-identical content, no write, no reload). A route whose live symlink is dangling. A registry with zero routes (never authority to delete anything — mirror `appconfig.Sync`'s guard at :175). An `nft` binary that is absent (must be `!ok`, not a crash, and must not be an error that aborts the pass). Whether the advisory volume on a healthy fleet is genuinely zero — advice that fires every hour on a healthy fleet is advice nobody reads. |
| + |
| +--- |
| + |
| +### Task 9: channel neutrality, docs, verification, review, and the optional console render |
| + |
| +**Model:** implement **Sonnet** for the docs and the neutrality test; the review dispatch is **Opus**. |
| + |
| +**Files:** |
| +- Create: `internal/caps/neutrality_test.go` |
| +- Modify: `README.md` (only where the env/verb surface is already documented — add `carlos route -grant/-revoke` and the `-capabilities` ship flag) |
| +- Modify: `docs/superpowers/specs/2026-08-03-app-capabilities-design.md` — **only** to record implementation deviations (the ship-input surface, the socket restart ordering, the socket-unit enable). Record deviations; do not rewrite history. |
| +- Optional: `internal/console/view.go`, `internal/console/templates/app.html` |
| + |
| +- [ ] **Step 1: The channel-neutrality assertion**, in `laddercheck_test.go`'s style and with the same reasoning in its comment: |
| + |
| +```go |
| +// Nothing in the capability design consults or constrains a channel name. |
| +// canary/* in, canary/* out — the fleet runs on those channels and a capability |
| +// change that refused one would stop every deploy. |
| +func TestCapabilitiesNeverConsultAChannelName(t *testing.T) { |
| + // 1. Structural: no identifier in internal/caps mentions a channel. |
| + // grep the package's own source (go/parser over the package dir) for |
| + // "channel"/"Channel"/"canary" — must be zero hits outside comments. |
| + // 2. Behavioural: converge the same Plan for a route on each of |
| + // laddercheck's liveChannels and assert byte-identical drop-ins. |
| +} |
| +``` |
| +Put the behavioural half in `internal/caps` (which has no `registry`/`release` import) by parameterising only what a channel could plausibly reach: it cannot reach anything, and the test's job is to keep it that way. |
| + |
| +- [ ] **Step 2: Full gate.** `gofmt -l` over every touched file → empty. `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && go vet ./... && go test ./... -count=1` → all clean, **15 packages** now (`internal/caps` is new). |
| +- [ ] **Step 3: Optional console render — include ONLY if it is genuinely cheap.** `internal/console/view.go`'s `AppView.Routes` (:226) already carries `Hibernate`; add `Caps string` from the route row and render it read-only in `app.html` beside the existing per-route facts, as plain text ("granted: host-role, port:25", or nothing at all when empty). **Do not** add a route, a handler, a gate, or a write path; **do not** reorder or touch `handleApp`'s existing gate. If it does not fall out in under ~20 lines, skip it and say so in the PR body — it is explicitly optional. |
| +- [ ] **Step 4: Dispatch an Opus reviewer** over the full branch diff with the spec and this plan as context. This change grants box authority (`host_role`), binds privileged ports, and writes root-owned unit configuration, so the review is not optional and the tier is not negotiable. Fix everything at Critical/Important; use judgment on nits. |
| +- [ ] **Step 5: Re-run the full gate after fixes.** Then push and open a draft PR titled `capabilities: apps declare, the platform provides — closed vocabulary, per-box grants`, with a body covering: the manifest shape, the grant model and why it is per box, the fail-closed refusal directions, the deploy order (platform binaries first, capability-bearing releases second), what is NOT in this PR (the keymail `LISTEN_FDS` change, the migrations, the infra script deletions), and the standard attribution footer. |
| +- [ ] **Step 6: Do not deploy and do not migrate.** The §5 migrations are operator sessions and are out of scope for this plan. The PR body should carry the migration checklist verbatim from spec §5a-§5c so the operator has it in one place, plus the two pre-flight checks named in "Risks" below. |
| + |
| +--- |
| + |
| +## Risks and findings from reading the code (things the spec does not cover) |
| + |
| +Ordered by how much they would hurt if left unsaid. |
| + |
| +1. **The spec never says how a capability block gets into a manifest.** Neither `carlos ship` (cli.go:871) nor the console ship API (apicli.go:603) has any input for it — both build the `Manifest` literal from flags and multipart parts only. Without a declaration surface the feature is unreachable. Task 2 resolves it with a `-capabilities <file.json>` flag threaded through both doors. Flagging it because it is a spec gap, not a design change: the file's contents are validated by the same `ValidCapabilities` the box runs. |
| + |
| +2. **The console ship API's "identical content already shipped" short-circuit compares artifacts, not the manifest** (apicli.go:629-649). A second ship of the same version with a *changed capability block* would answer `200 shipped` and silently keep the first block — the immutability promise broken for exactly the field that decides unit shape. Task 2 closes it with a `409`. |
| + |
| +3. **A `ports` capability materialises nothing unless the socket unit is enabled.** `carlos bootstrap` does not enable `carlos-app@<host>.socket` (`enabledUnits` is edge + adopt timer only), and the template's own comment says "enable per tenant". Spec §4c assumes systemd is already binding. Task 6 step 8 enables it. |
| + |
| +4. **Changing `ListenStream` under a running service races into `EADDRINUSE`.** Spec §4c says the drop-in adds the line and moves on. The restart ordering has to be service-stop → socket-restart → service-start, or the MX comes back unbound. Pinned in Task 6 step 7. |
| + |
| +5. **`restartRoute` returns `nil` unconditionally** (agent.go:712-718), so `adoptHost`'s restart-failure rollback — the mechanism spec §4d relies on to "restore the previous drop-ins" — is currently dead for unit-backed routes. Task 7 revives it for daemon routes only, via `caps.Restart`, leaving every other route byte-identical. |
| + |
| +6. **The in-process adopt loops cannot write `/etc/systemd/system`.** `adoptLoop` and `canaryAdoptLoop` run inside `carlos-edge.service`, which is `ProtectSystem=strict` with `ReadWritePaths=/data /run/carlos`. Arming converge there would log a permission denial every 2-3 seconds. Consequence, which needs an explicit ruling in review: a freshly promoted **canary** runs the new binary against the old unit shape until the hourly root pass converges it. Task 7 recommends accepting the window and logging it. |
| + |
| +7. **`ensureStaged` short-circuits on a staged binary** (adopt.go:356), so without the back-fill in Task 3 the converge pass would be permanently blind on every version already running on every box — including the two versions this whole design exists to migrate. |
| + |
| +8. **`registry.Upsert` would silently revoke grants.** `carlos add` re-upserts and the domains reconciler re-mints from `mintedRoute`, neither of which knows about grants. The `environment` column already has the `CASE WHEN excluded... = ''` preservation for exactly this reason (IMPORTANT 3, 2026-08-02); `caps` needs the same or a re-add drops `:25`. |
| + |
| +9. **`Route.DBPath` is single-valued but `data.dbs` is a list.** Task 8 stamps `dbs[0]` (sorted) and advises on the rest rather than pretending the extras are replicated. Console has one database, so nothing is lost today — but the shape is honest about the limit instead of quietly dropping names. |
| + |
| +10. **Pre-migration checks the operator must run before the §5 cutover, neither of which is in the spec:** |
| + - Confirm the console's route row is **not** `Platform: true`. `release.adoptable` excludes platform routes, so a `Platform` console route would never materialize a capability and the cutover would appear to succeed while changing nothing. |
| + - Confirm the route's `Kind == "instance"` and `App != ""`. Both are `adoptable` preconditions and neither is obviously true of a row that has been hand-maintained. |
| + |
| +11. **`carlos route` has no `-hibernate` flag today** and `registry.SetHibernate` has no CLI caller at all — so spec §4e's "`daemon` + `hibernate` is refused by `carlos add` / `carlos route`" has nowhere to live and no operator fix path to name. Task 4 adds the flag (using the setter that already exists) so the refusal has a door on both sides. |
| + |
| +12. **A hostile or fat-fingered `Route.Host` now reaches a root-written `/etc` path.** `appconfig.syncHost` already guards this at :293 for `/etc/carlos/apps`; the same guard must exist in `caps.Converge` before any path is joined. Called out in Task 6's break-list because it is the one place where a registry row becomes a filename under `/etc/systemd/system`. |