amadan
spec: rework after the Fable review — two blockers, one found verifying it
The review found two blockers and I verified both against the code rather than taking them on trust. Verifying the second turned up a third that the review did not reach. BLOCKER 1 — §6 defeated the spec's own purpose. It had control-plane apps demand fresh auth to promote "exactly as stable does, at every rung". But stable's gate is a bare 403 with no API-side step-up (apicli.go:632-638) and carlos deploy log.Fatalfs on a failed promote AFTER the ship has landed (deploy.go:206-208) — so `carlos deploy -app console`, the command this spec exists to create, would have shipped and then died every time. Inverted: the flag now gates DESTRUCTION, not promotion. Deploying the console should be easy; destroying it should be hard — and that is where the danger actually was, since handlePurgeApp can delete the release history the fleet is currently running behind an ordinary confirmation. It also refuses promotion by a service principal, which closes the CI-rolls- the-console hole without needing a step-up flow at all. BLOCKER 2 — §4/§8 overclaimed the rollback. adoptHost rolls back only when systemctl try-restart exits non-zero, and a Type=simple unit exits 0 the moment the process is exec'd. A console that starts and then panics stamps the new build fleet-wide with no rollback. Now requires a readiness gate on console.service (a pinfra + cmd/console change this spec owns) and states the residual plainly instead of implying it is covered. BLOCKER 3, found while verifying 2 and not in the review — the rollback leaves the console DOWN and the NEXT pass then lies. adoptHost repoints the symlink but never starts the unit, and try-restart is a no-op on a unit that is not running, so pass 2 exits 0 and stamps SetVersions over a console that is not running at all. Adoption must START the unit on the restored version. This is a tenant fix: every unit-backed route has the hole, and it has stayed hidden because the pinfra script's explicit post-pass restart papers over it on the fleet's only stamped route. Also: §4c now rules that the stamped route must be the host named in ExecStart, not the canonical hostname — flagship has these disagreeing today, which is why the pinfra script's explicit restart is load-bearing; §3a names the build probe's storage and consumer list and rules it onto the app row rather than provision.Record (read-modify-write would drop it); §3b closes the ldflags trap that would have made a healthy deploy report failure forever; §8 documents the migrateAppsKey column-drop trap; §2 pins ownerClaim false for service principals and corrects the claim that derp/meep are unblocked (they hold grants); §10 reorders so the flag exists before the claim and the code reaches the boxes before the first self-deploy, and adds the tito survey it was missing. Step 0 resolved in the spec's favour: no CLI grant on ops/console, so the owner exemption is sufficient. Flagship's readings are in a new appendix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 file changed,
+438
−243
docs/superpowers/specs/2026-08-23-console-as-an-app-design.md+438 −243
diff --git a/docs/superpowers/specs/2026-08-23-console-as-an-app-design.md b/docs/superpowers/specs/2026-08-23-console-as-an-app-design.md| index a3bf61bb7..6a9b61c62 100644 |
| --- a/docs/superpowers/specs/2026-08-23-console-as-an-app-design.md |
| +++ b/docs/superpowers/specs/2026-08-23-console-as-an-app-design.md |
| @@ -1,6 +1,7 @@ |
| # The console is an app — retiring the pinfra roll |
| -**Date:** 2026-08-23 · **Status:** approved (Paul, in-session) · **Slice:** 1 of 4 |
| +**Date:** 2026-08-23 · **Status:** approved (Paul, in-session); reworked after a |
| +Fable review · **Slice:** 1 of 4 |
| ## Problem |
| @@ -15,14 +16,18 @@ platform: roll-console.sh <platform-checkout> both # pinfra + AWS + SSM |
| That inversion is the thing CLAUDE.md's own rule complains about ("every |
| `carlos` command is designed for a user with no access to the underlying |
| -infra"), and it has a cost beyond inconvenience: a special door is where our |
| -own deficiencies hide. Everything that makes rolling the console feel |
| -dangerous — a version header that lies, a restart nobody trusts, a rollback |
| -verified by hand — is a deficiency **tenants have too**. They just complain |
| -less. While the platform team has its own door, nothing forces us to feel it. |
| +infra"), and it costs more than convenience: a special door is where our own |
| +deficiencies hide. Everything that makes rolling the console feel dangerous — a |
| +version header that lies, a restart nobody trusts, a rollback verified by hand |
| +— is a deficiency **tenants have too**. They just complain less. While the |
| +platform team has its own door, nothing forces us to feel it. |
| This slice makes the console an ordinary app, deployed by the ordinary verb. |
| +Three of the five fixes below (honest build verification §3, the dirty-tree |
| +guard §7, and the restart-recovery fix §4b) are **tenant** fixes that this work |
| +merely forces us to make. |
| + |
| ### Not in this slice |
| `carlos system` is **deferred, not rejected** (Paul, 2026-08-23). It earns its |
| @@ -30,9 +35,7 @@ place for what genuinely cannot be an app: the root `carlos` binary, the edge |
| router, the activator, the provisioning reconciler. Those have no route, no |
| channel to be adopted from and no upstream — they are the substrate that does |
| the adopting and routing. `update-platform-binary.sh` is 85 lines of exactly |
| -that work (stage embedded units, sysusers *before* tmpfiles, daemon-reload, |
| -*arm* timers and path units rather than merely enable them), none of which has |
| -an app-shaped equivalent. That is slice 3. |
| +that work, none of which has an app-shaped equivalent. That is slice 3. |
| The dividing line, stated once so later work can apply it: **`carlos system` is |
| for what is not an app. Anything adoptable through a route is an app.** |
| @@ -41,328 +44,520 @@ for what is not an app. Anything adoptable through a route is an app.** |
| `roll-console.sh` and `verify-console.sh` encode failure modes this project has |
| already paid for. A replacement that drops any of them is a regression wearing |
| -better clothes. Enumerated so the plan can check them off: |
| +better clothes. |
| -| # | Failure mode | Where it is handled after this slice | |
| +| # | Failure mode | Covered by | |
| |---|---|---| |
| -| 1 | amd64 artifact shipped to Graviton boxes | `carlos deploy -target linux-arm64` (already the default) + §5 server-side refusal | |
| -| 2 | Adoption swaps the symlink; the old process keeps serving | `makeRestartRoute` arm 1, already built (§4) | |
| -| 3 | SSM shells missing the box env (`set -a; source host.env`) | No SSM at all | |
| -| 4 | "Verified" reported from a stale binary | §3, self-reported build | |
| -| 5 | A dirty tree stamping a version that names no real commit | §6 | |
| +| 1 | amd64 artifact shipped to Graviton boxes | §5 ELF refusal (`-target` default only helps the careful caller) | |
| +| 2 | Adoption swaps the symlink; the old process keeps serving | §4a — the *right* route stamped | |
| +| 3 | SSM shells missing `set -a; source host.env` | No SSM at all | |
| +| 4 | "Verified" reported from a stale binary | §3 build probe | |
| +| 5 | A dirty tree stamping a version naming no real commit | §7 | |
| | 6 | sysusers before tmpfiles | Not reached — root-binary concern, slice 3 | |
| +| 7 | **The console artifact is `cmd/console`, not the root binary** (8-minute outage, 2026-08-08) | §5 embedded-build refusal | |
| ## 1. What is actually in the way |
| -Four findings from the live estate (2026-08-23), because the gap is smaller |
| -than it looks and the remaining pieces are specific. |
| +Verified against the live estate on 2026-08-23. See the appendix for the raw |
| +readings. |
| -**The console is already structurally an app.** Its releases are at |
| -`apps/bab/console/` — properly account-qualified under `ops` — on channel |
| +**The console is already structurally an app.** Releases at |
| +`apps/bab/console/`, properly account-qualified under `ops`, on channel |
| `canary/console`, adopted by the same pass as every tenant, served through |
| -routes, with one route already naming `console.service`. |
| +routes, one route already naming `console.service`. |
| -**But it is not an app *record*.** `ops` on flagship holds exactly two app |
| -rows: `deploy-probe` and `platform-site`. There is no `console` row, because |
| +**But it is not an app *record*.** `ops` on flagship holds two app rows: |
| +`deploy-probe` and `platform-site`. There is no `console` row, because |
| `ship-app.sh` ships direct-to-bucket and direct mode deliberately skips the |
| -claim. Without a row the console API cannot see the app at all: `carlos |
| -channels -app console -account ops` answers the byte-identical 404. |
| +claim. Without a row the console API cannot see the app at all. |
| -**Claiming the name is currently refused.** `claimApp` |
| -(`internal/console/server.go`) lists the app's prefix and returns `claimTaken` |
| -when anything is there. This is issue **#79**. It also blocks `carloku`, |
| -`carlosframework`, `derp`, `hello`, `meep` and `apt-carlosframework`, all |
| -bucket-only apps under `ops`. |
| +**Claiming the name is refused today** by `claimApp` |
| +(`internal/console/server.go:2426`) because the prefix has objects — issue |
| +**#79**. |
| -**The other three pinfra apps need nothing.** Contrary to CLAUDE.md's framing |
| +**The other three pinfra apps need no claim.** Contrary to CLAUDE.md's framing |
| of four equivalent apps, `kass`, `keymail` and `platform-site` are already |
| -claimed rows with live channels: |
| - |
| -``` |
| -kass/kass claimed edge f1479f7 · canary/rehearsal aedcae3-r2 |
| -keymail/keymail claimed beta 053baf4 · canary/keymail 9825fd0 |
| -ops/platform-site claimed canary/site site1 |
| -ops/console NO ROW ← the only #79 case |
| -``` |
| - |
| -They are on `ship-app.sh` out of habit, not necessity. Proving `carlos deploy` |
| -on them and deleting the habit is part of this slice and costs almost nothing. |
| +claimed rows with live channels. They are on `ship-app.sh` out of habit. |
| ## 2. Claiming an app whose objects already exist (#79) |
| `claimApp`'s bucket guard is right for a stranger and wrong for the account |
| that owns the prefix. `apps/<sqid>/<app>/` is namespaced **by account**, so |
| objects under `apps/bab/` can only be `ops`'s. An owner of `bab` claiming a |
| -name under `bab`'s own prefix is adopting their own objects, not inheriting a |
| -stranger's. |
| - |
| -The change is one condition: |
| +name under `bab`'s own prefix is adopting their own objects. |
| ```go |
| // today — always refuses |
| if len(keys) > 0 { return claimTaken, "" } |
| -// after — refuses unless the claimer owns the account the prefix belongs to |
| +// after — unless the claimer owns the account the prefix belongs to |
| if len(keys) > 0 && !ownerClaim { return claimTaken, "" } |
| ``` |
| -`ownerClaim` is passed in by the caller, which already resolves the principal. |
| -It is **owner**, not member: adopting existing release history is an ownership |
| -act, and both claim doors (`handleAPICreateApp`, `handleNewApp`) know the role. |
| - |
| -Four things this must NOT change: |
| - |
| +`ownerClaim` is passed by the caller. Five constraints, each of which a naive |
| +implementation gets wrong: |
| + |
| +- **Owner, not member.** Adopting existing release history is an ownership act. |
| +- **`ownerClaim` is FALSE for every service principal.** A service has no role |
| + at all, and an account-scoped `PrincipalService` with `App == ""` |
| + legitimately reaches `claimApp` (`apicli.go:346–350` blocks only app-scoped |
| + ones). Without an explicit pin, the exemption silently hands every CI |
| + credential the power to adopt release history — exactly the widening the |
| + svcrole work exists to prevent. |
| +- **`handleNewApp` (the browser door) must be changed deliberately, not |
| + incidentally.** It currently *discards* the role |
| + (`addr, raw, acct, _, ok := s.account(…)`, `server.go:2524`) and its doc |
| + comment says "Any member may claim." The exemption changes that door's stated |
| + policy; the comment changes with it. |
| - **The legacy prefix still blocks, always.** For `ops`, `claimApp` also checks |
| `release.LegacyAppPrefix(name)` — `apps/<app>/`, unqualified. Those objects |
| - predate account scoping and are owed an `accounts migrate -from legacy`; |
| - handing the name out would put an app in the way of that migration. The |
| - owner exemption applies **only** to the account-qualified prefix. |
| - (Verified 2026-08-23: no `apps/console/` exists on flagship, so the console |
| - does not depend on this exemption being widened.) |
| + predate account scoping and are owed an `accounts migrate -from legacy`. |
| + The exemption applies **only** to the account-qualified prefix. |
| - **`HasOwners(name)` still blocks.** A CLI-granted app in `ops` stays |
| - claim-proof. If `console` turns out to carry a grant, that is a separate |
| - refusal and the plan's step 0 checks for it before anything else. |
| -- **Refusals stay byte-identical for non-members.** The console's gate order is |
| - session → CSRF → owner (byte-identical 404) → `requireFreshAuth` last. A |
| - non-member must not be able to tell "this name has objects" from "this |
| - account does not exist". The new branch runs *after* the owner gate, so it is |
| - only ever reached by someone who has already proved ownership. |
| -- **The claim is still once, ever.** Nothing here makes a claimed name |
| - re-claimable. |
| + claim-proof. |
| + |
| +**Refusals stay byte-identical for non-members.** The new branch runs after the |
| +owner gate, so only someone who has already proved ownership can reach it. |
| + |
| +**What this does and does not unblock.** Verified: the `owners` table holds |
| +grants for `derp`, `meep` and `platform-site` only. So the exemption unblocks |
| +`console`, `carloku`, `carlosframework`, `keymail` (the `bab` prefix), |
| +`hello` and `apt-carlosframework`; `derp` and `meep` stay claim-proof by |
| +design. An earlier draft claimed otherwise and was wrong. |
| + |
| +**One nuance to absorb:** a *trashed* app passes `HasAppIn` (which filters |
| +`deleted_at = 0`) and is caught later by `CreateAppIn`'s |
| +`ON CONFLICT DO NOTHING` (`store.go:1120–1128`) → `claimTaken`. No hole, but |
| +after the exemption a trashed-name claim burns a bucket listing before hitting |
| +that refusal. Acceptable; noted so it is not mistaken for a bug. |
| **Test:** an owner claims a name whose account-qualified prefix has objects and |
| -succeeds, with the app's existing release history immediately visible through |
| -`channels`; a member who is not an owner gets `claimTaken`; a non-member gets |
| -the byte-identical 404; a name whose *legacy* prefix has objects is still |
| +succeeds, release history immediately visible through `channels`; a non-owner |
| +member gets `claimTaken`; **a service principal gets `claimTaken` whatever its |
| +scope**; a non-member gets the byte-identical 404; a name whose *legacy* prefix |
| +has objects is refused even for an owner; a name with an `owners` row is |
| refused even for an owner. |
| ## 3. Honest verification: the console reports its own build |
| -`roll-console.sh` reads `readlink /proc/<pid>/exe` over SSM, and |
| -`verify-console.sh` explains why in fifteen lines: both cheap answers lie. |
| -`X-Carlos-Version` is stamped by the edge from the **route row**, so adoption |
| -sets it while the old process keeps serving. `systemctl is-active` reports |
| -"active" for a process started from the previous version just as happily as |
| -for the current one — on 2026-08-09 tito said "active" while `/proc` said it |
| -was still on 9668d30, seconds after adopt and restart both reported success. |
| +Both cheap answers lie. `X-Carlos-Version` is stamped by the edge |
| +(`edge.go:1336–1341`) from the **route row**, so adoption sets it while the old |
| +process keeps serving. `systemctl is-active` reports "active" for a process |
| +started from the previous version just as happily — on 2026-08-09 tito said |
| +"active" while `/proc` said 9668d30, seconds after adopt and restart both |
| +reported success. |
| -A `carlos deploy -app console` that waited on `X-Carlos-Version` would be |
| -**less trustworthy than the bash it retires**. So the console reports its own |
| -build, from the process, compiled in: |
| +A `carlos deploy -app console` waiting on `X-Carlos-Version` would be **less |
| +trustworthy than the bash it retires**. So the console reports its own build, |
| +from the process, compiled in: |
| ``` |
| GET /healthz → ok <build> # today: "ok" |
| ``` |
| -This is at least as strong as the `/proc` check and arguably stronger: `/proc` |
| -proves which *path* is executing, this proves which *code* is, since the value |
| -comes from `internal/version.Build` set by ldflags in the same build that |
| -produced the binary. It cannot be stamped by adoption, because nothing but the |
| +At least as strong as the `/proc` check and arguably stronger: `/proc` proves |
| +which *path* is executing, this proves which *code* is, from |
| +`internal/version.Build` set by ldflags in the same build. Nothing but the |
| running process can answer it. |
| -`/healthz` is served by the console's own mux (`server.go`), not by the edge, |
| -so the answer is the process's. It is unauthenticated and already public; a |
| -build id is not a secret (the same value is already on every response as |
| -`X-Carlos-Version`, and in `BuiltBy` on ship responses). |
| +`/healthz` is served by the console's own mux (`server.go:794`), not the edge. |
| +Existing consumers (`internal/vet`, the edge health probe) check **status |
| +only**, so appending the build is additive-safe. The build id is not a secret — |
| +`X-Carlos-Version` already exposes it on every response. |
| -**`carlos deploy` gains a verification mode, not a special case for one app.** |
| -Today the watch (`deploy.go`, the `X-Carlos-Version` loop) has one exit |
| -criterion. It gains a second: when the app declares a **build probe** — a path |
| -and the expectation that the response body carries the build id — the watch |
| -polls that instead of the header. The console is the first user. Any tenant |
| -whose app can report its own build gets the same honesty, which is the point: |
| -this is a tenant feature we need first. |
| +### 3a. Where the probe is declared, and who parses it |
| -The probe is declared on the app, alongside the health probe the instance API |
| -already carries (`{"path":"/healthz","status":200}` — `apiinstances.go`), not |
| -hard-coded to the console: |
| +`carlos deploy`'s watch gains a second exit criterion: when the app declares a |
| +**build probe**, poll it instead of the header. |
| ```json |
| {"build_probe": {"path": "/healthz"}} |
| ``` |
| -Deliberately minimal. The **path** is the only thing that varies; the contract |
| -is fixed: a 2xx whose body contains the build id as a whitespace-delimited |
| -token. Not a regex, not a JSON pointer, not a header name — every one of those |
| -is a knob whose wrong setting produces a watch that passes while reporting |
| -nothing, which is the exact failure this section exists to remove. An app that |
| -cannot meet that contract declares no probe and keeps the header behaviour. |
| - |
| -Matching on a whitespace-delimited token rather than a substring is |
| -load-bearing: a substring match would let build `abc123` satisfy a watch |
| -waiting for `abc1234`, and short git shas make that a real collision, not a |
| -theoretical one. |
| +The **path** is the only thing that varies. The contract is fixed: a 2xx whose |
| +body contains the build id as a **whitespace-delimited token**. Not a regex, |
| +not a JSON pointer, not a header name — each is a knob whose wrong setting |
| +produces a watch that passes while proving nothing. Token-matching rather than |
| +substring is load-bearing: a substring match lets build `abc123` satisfy a |
| +watch waiting for `abc1234`, and short git shas make that a real collision. |
| + |
| +**Storage, and the consumer list** — CLAUDE.md's rule is that when a storage |
| +shape changes, the consumer list is everything that PARSES the shape: |
| + |
| +The probe is **app-level, not per-instance**. The health probe it resembles |
| +lives in `provision.Record` (`internal/provision/provision.go:33`, wire key |
| +`"probe"`, at `apps/<sqid>/<app>/instances/<host>.json`) and that is the wrong |
| +home for this: `handleAPIInstanceUpdate` is a **read-modify-write** of the |
| +record, so an older console rewriting a newer record would silently drop the |
| +field. The build probe describes the *app*, not one instance of it, and every |
| +instance would carry an identical copy. |
| + |
| +It therefore lives on the **app row**, served to the CLI on the existing |
| +per-app API the deploy path already calls, and is not a bucket object at all. |
| +Parsers: the app row's own query (which names columns explicitly — verified, no |
| +`SELECT *`), the API handler that serves it, and `carlos deploy`'s watch. Three, |
| +all in-repo, none of them a wire shape a box parses. That is the point of |
| +choosing the row over the record. |
| + |
| +### 3b. Two ways the watch can still lie, and what closes them |
| + |
| +**The ldflags trap.** `carlos deploy` ships a binary the *operator* built. |
| +`-version` defaults to `gitShortHead()` (`deploy.go:52`) but `version.Build` |
| +comes from ldflags the operator must remember (default `"dev"`, |
| +`internal/version/version.go:6`). Build without the flag and the roll |
| +**succeeds** while the probe reports `ok dev` forever — deploy reports failure |
| +on a healthy deploy, the exact inverse of the lie §3 exists to kill. Closed |
| +twice: §5's ship-time refusal when the embedded build ≠ `-version`, and a |
| +watch timeout that **names what the probe actually said** (a body of `dev` is |
| +then self-diagnosing). |
| + |
| +**The edge cache.** `watchVersion` cache-busts every poll because the edge |
| +caches proxied 2xx responses up to five minutes (`deploy.go:1013–1022`, |
| +`edge.go:933`). The console's `/healthz` sends no `Cache-Control` so it is safe |
| +today, but §3 is a tenant feature and a tenant's probe path may not be. The |
| +probe leg inherits the buster, and reads the body through the default transport |
| +so gzip is transparently decoded. |
| **Test:** the watch reports live only once the probe body carries the shipped |
| -build; a probe that keeps answering the previous build times out with a |
| -diagnosis naming both builds; an app with no probe declared keeps today's |
| -header behaviour byte for byte. |
| +build as a whole token; `abc123` does not satisfy a watch for `abc1234`; a |
| +probe stuck on the previous build times out with a diagnosis naming both the |
| +expected build and the body received; an app with no probe keeps today's header |
| +behaviour byte for byte; the probe leg sends a cache-buster. |
| -## 4. Restart and rollback — already built, needs stamping |
| +## 4. Restart, rollback, and the recovery hole |
| -`makeRestartRoute` (`internal/host/agent.go`) arm 1 covers this completely, and |
| -the reasoning is worth not re-deriving: a route that names its unit is |
| -try-restarted by name, and **arm 1 returns the failure** rather than swallowing |
| -it, so `adoptHost` rolls the symlink back to the version that IS running, skips |
| -`SetVersions`, and leaves the header honest on the old version. A console that |
| -will not come back cannot produce a lying stamp. |
| +`makeRestartRoute` arm 1 (`internal/host/agent.go:2041–2050`) try-restarts a |
| +route that names its unit and **returns** the failure, so `adoptHost` poisons |
| +the build, repoints the symlink back and skips `SetVersions` |
| +(`adopt.go:779–818`). That much is built, and it is what makes a lying header |
| +impossible *in that case*. |
| -Nothing to build. What is missing is **fleet state**: only one of the console's |
| -four routes is stamped. |
| +**It is a much narrower guarantee than it looks, and an earlier draft of this |
| +spec overclaimed it.** Two holes, both verified. |
| -``` |
| -platform.carlosframework.com | console | canary/console | (no unit) |
| -console.bab.oncarlos.com | console | (alias) | (no unit) |
| -dashboard.carloku.com | console | canary/console | (no unit) |
| -console.carloku.com | console | canary/console | console.service |
| -``` |
| +### 4a. The guarantee only covers systemd-visible start failure |
| + |
| +`systemctl try-restart` on a `Type=simple` unit succeeds the moment the process |
| +is exec'd. A console that starts and then panics on its first request, exits |
| +after a half-applied migration, or listens and 500s, **exits 0**. Then |
| +`SetVersions` stamps the new build fleet-wide, the §3 probe correctly times |
| +out — and nothing rolls back. The symlink stays on the broken build, the next |
| +pass early-returns on `cur == ver` (`adopt.go:637`), and the repair door is the |
| +thing that is down. |
| + |
| +**Requirement:** `console.service` gains a **readiness gate** so that "started |
| +but not serving" becomes a failed start. `Type=notify` with the console |
| +signalling readiness *after* migrations have applied and the listener is up is |
| +the precise form; an `ExecStartPost` readiness poll is the cheap form. |
| -### 4a. One process, four hostnames |
| +**This is a pinfra change plus a `cmd/console` change**, and it is a |
| +cross-repo dependency this spec owns. It cannot be assumed: `console.service` |
| +is defined in pinfra, not in `internal/host/units/`, and its current `Type=` is |
| +not recorded anywhere in this repo. Step 0 reads it. |
| -This is the one place the app model genuinely strains, and it needs a ruling |
| -rather than a silent choice. Adoption is **per host**: four routes mean four |
| -`adoptHost` calls. Stamping all four with `console.service` would try-restart |
| -the same unit four times in one pass — four connection drops for one deploy. |
| -Leaving three unstamped means three rows stamp `Version` (and therefore |
| -`X-Carlos-Version`) with no restart behind them. |
| +The residual after this — a console that starts, serves, passes readiness and |
| +is *behaviourally* wrong — has no automatic recovery and is break-glass |
| +(§9). Say so out loud rather than implying the rollback covers it. |
| -**Ruling: leave exactly one route stamped, and stop relying on the header.** |
| -One stamped route is sufficient to cycle the process, which is the only thing |
| -that has to happen. The three unstamped rows' headers become cosmetic — and |
| -harmless, because §3 moves the exit criterion off the header and onto the |
| -process's own answer. `console.bab.oncarlos.com` is an alias and correctly |
| -carries no version at all (a minted alias serves 200 with no |
| -`X-Carlos-Version` by design). |
| +### 4b. Rollback repoints the symlink but leaves the console DOWN — and the next pass then lies |
| -The alternative — teaching adoption to dedupe restarts by unit within a pass — |
| -is a real improvement and explicitly **out of scope here**. It should be filed: |
| -it is the general fix for any bespoke service behind several hostnames, and |
| -this slice does not need it. |
| +This is the more serious of the two and it is **not** in the review that |
| +prompted this rework; it was found verifying that review. |
| -The stamped route is `console.carloku.com` (already stamped, and the canonical |
| -console host since 2026-08-12). `carlos deploy -app console -host |
| -console.carloku.com` scopes the watch to it; `-host` already exists. |
| +On restart failure `adoptHost` repoints the symlink to `cur` and returns |
| +(`adopt.go:781–815`). It does **not** start the unit again. For a `try-restart` |
| +that failed to bring the service back, systemd leaves it **failed** — so the |
| +box is left with a correct symlink and no console. |
| -## 5. Server-side arch refusal |
| +It gets worse on the next pass. `try-restart` is by definition a **no-op on a |
| +unit that is not running** (the property `makeRestartRoute`'s comment relies on |
| +so that a deliberately-stopped console stays stopped). So: |
| -Failure mode #1 in the table above — an amd64 binary on a Graviton box — is |
| -covered on the happy path by `-target linux-arm64` being `carlos deploy`'s |
| -default. That guard only protects callers who use the flag correctly. |
| +``` |
| +pass 1: PointLive(new) → try-restart → FAILS → poison, PointLive(old), no stamp |
| + unit: failed. console: DOWN. header: honest (old). |
| +pass 2: PointLive(new) → try-restart on a NOT-RUNNING unit → no-op, exit 0 |
| + → SetVersions(new) |
| + unit: still failed. console: STILL DOWN. header: claims the new build. |
| +``` |
| -For an app flagged as control-plane (§6), the ship door additionally **reads |
| -the artifact's ELF header** and refuses a machine type that does not match the |
| -declared target. This closes the path a hand-run `carlos ship`, a CI job or a |
| -future script could still take into the same channel. |
| +The second pass converts an honest failure into the exact lie the whole |
| +mechanism exists to prevent, over a console that is not running at all. |
| -Scoped to control-plane apps deliberately: a tenant may have entirely |
| -legitimate reasons to ship an artifact whose ELF machine we would guess wrong |
| -about, and refusing their ship on our inference is not our call. For the app |
| -that *is* the control plane, on boxes we own and know are arm64, it is. |
| +**Requirement:** when arm 1's try-restart fails and the symlink has been rolled |
| +back, adoption must **start** the named unit on the restored version — a |
| +`systemctl start`, not a try-restart, because the whole point is that the unit |
| +is down. If that start also fails, the version stays poisoned and unstamped and |
| +the failure is printed; the box is then genuinely broken and break-glass is |
| +correct. |
| -**Test:** an amd64 ELF shipped to a control-plane app with `-target |
| -linux-arm64` is refused, naming both the declared target and what was found; a |
| -matching artifact ships; a non-control-plane app is unaffected; a non-ELF |
| -artifact (a static site tarball) is unaffected. |
| +This is a **tenant** fix. Any bespoke unit-backed route has this hole today. |
| +It is the strongest argument in the whole slice for making the platform eat its |
| +own food: the hole has presumably been there since #110 and nobody found it, |
| +because the pinfra script's explicit post-pass `systemctl restart` has been |
| +papering over it on the only unit-stamped route in the fleet. |
| -## 6. The control-plane flag |
| +**Test:** a failed named-unit restart leaves the unit RUNNING on the previous |
| +version, the symlink on the previous version, and the row unstamped; a second |
| +pass over the same still-broken build does not stamp; a failed rollback start |
| +is reported and still does not stamp. |
| -Replacing the console is not the same act as replacing `platform-site`. That |
| -belongs on the **app**, not on a verb: |
| +### 4c. One process, several hostnames — the stamped route is not a free choice |
| -``` |
| -apps.control_plane BOOLEAN NOT NULL DEFAULT 0 |
| -``` |
| +The console is one `console.service` behind several hostnames. Adoption is |
| +**per host** (`reg.List()`, `ORDER BY host`, `registry.go:604`), and each |
| +`adoptHost` repoints `live/<that host>` (`PointLive(optDir, rt.Host, …)`) and |
| +*then* fires the restart callback. |
| -Additive with a default that means what every existing row already means — the |
| -idiom the `kind` and service-principal columns used. |
| +So the restart fires inside **one** host's adoption, while the other hosts' |
| +symlinks have not moved yet. Which means: |
| -An app with the flag set: |
| +> **The route that must carry the `Unit` stamp is the host whose symlink the |
| +> unit's `ExecStart` resolves — not the canonical hostname, not the first |
| +> alphabetically.** |
| -- requires **fresh auth** to promote, exactly as `stable` already does |
| - (`apicli.go` answers `403` with `console_url` and the CLI sends the operator |
| - to finish in the browser). The blast radius, not the rung, is what is being |
| - gated, so it applies at every rung. |
| -- gets the §5 arch refusal on ship. |
| +Flagship today gets this **wrong**, and the pinfra script has been hiding it: |
| -Why a flag and not a verb: a verb protects only the path that goes through the |
| -verb. A flag on the row also protects a hand-run `carlos promote -app console`, |
| -a service principal, a CI job and the console's own app page. The gate should |
| -sit where the danger is, which is the app. |
| +``` |
| +ExecStart = /opt/carlos/live/platform.carlosframework.com serve |
| +stamped = console.carloku.com ← sorts BEFORE platform.… |
| +``` |
| -Set on `ops/console` in both deployments. Nothing else, for now. |
| +The pass restarts at `console.carloku.com` (2nd of 4 by host order), when |
| +`live/platform.carlosframework.com` still points at the **old** build; that |
| +symlink moves last, under an already-running process. Every roll to date has |
| +worked only because `roll-console.sh` runs an explicit `systemctl restart |
| +console` **after** the whole pass. Remove that line — which is precisely what |
| +"the console is an ordinary app" means — and the console keeps serving the old |
| +build while all its routes advertise the new one. |
| + |
| +**Requirement:** step 0 verifies, per box, that the `Unit`-stamped route is the |
| +host named in `ExecStart`, and restamps if not. Exactly one route is stamped: |
| +one is sufficient to cycle the process, and stamping several would try-restart |
| +the same unit several times in one pass. |
| + |
| +The unstamped rows' `X-Carlos-Version` then becomes cosmetic, which §3 makes |
| +harmless. One honesty note: on restart failure the poison map only stops |
| +**later** hosts in the pass from stamping (`adopt.go:648`), so whether the |
| +cosmetic headers stay honest depends on alphabetical order relative to the |
| +stamped host — protection by accident, not by design. The real fix is the |
| +follow-up below. |
| + |
| +**Follow-up to file, explicitly out of scope:** teach the pass to collect |
| +named-unit restarts and run them **once, after every host's symlink has |
| +moved**. That makes the stamp/ExecStart coupling and the alphabetical accident |
| +both irrelevant, and it is the general fix for any bespoke service behind |
| +several hostnames. |
| + |
| +## 5. Ship-time refusals for a control-plane app |
| + |
| +Two refusals, both at the ship door, both scoped to apps carrying the §6 flag. |
| + |
| +**Architecture.** Read the artifact's ELF header and refuse a machine type that |
| +does not match the declared `-target`. `-target linux-arm64` being `carlos |
| +deploy`'s default only protects the careful caller; this closes the path a |
| +hand-run `carlos ship`, a CI job or a future script takes into the same channel. |
| + |
| +**Embedded build.** Refuse a binary whose embedded `internal/version.Build` |
| +does not equal the `-version` being shipped. This closes §3b's ldflags trap at |
| +the door instead of at a timeout, and it incidentally closes failure mode #7: |
| +the root `carlos` binary and `cmd/console` stamp the same `version.Build`, so |
| +this does not catch a wrong-artifact ship on its own — but combined with the |
| +probe (only `cmd/console` serves `/healthz`) the pair does. |
| + |
| +Scoped to control-plane apps deliberately: a tenant may legitimately ship an |
| +artifact we would guess wrong about, and refusing their ship on our inference |
| +is not our call. For the app that *is* the control plane, on boxes we own and |
| +know are arm64, it is. |
| + |
| +**Test:** an amd64 ELF with `-target linux-arm64` is refused naming both the |
| +declared target and what was found; a binary whose embedded build ≠ `-version` |
| +is refused naming both; a matching artifact ships; a non-control-plane app is |
| +unaffected; a non-ELF artifact (a static tarball) is unaffected. |
| + |
| +## 6. The control-plane flag gates DESTRUCTION, not promotion |
| + |
| +```sql |
| +ALTER TABLE apps ADD COLUMN control_plane INTEGER NOT NULL DEFAULT 0; |
| +``` |
| -**Test:** promoting a control-plane app without fresh auth is refused with the |
| -existing `stable`-shaped 403; with fresh auth it succeeds; an ordinary app at a |
| -non-stable rung is unaffected; the refusal does not leak the flag's existence |
| -to a non-member (it sits behind the same 404). |
| +An earlier draft had this flag demand fresh auth to **promote**, "exactly as |
| +stable already does, at every rung." That was wrong, and wrong in a way that |
| +defeated the spec's own purpose. `stable`'s API gate is a bare 403 with a |
| +`console_url` and **no API-side step-up** (`apicli.go:632–638`), and |
| +`carlos deploy` `log.Fatalf`s on a failed promote *after the ship has already |
| +landed* (`deploy.go:206–208`). Extending it to every rung would mean |
| +`carlos deploy -app console` ships the artifact and then dies, every time — |
| +the exact command this spec exists to create. |
| + |
| +**Invert it.** Deploying the console should be easy; destroying it should be |
| +hard. That is also where the real danger is: `handlePurgeApp` (`ownership.go:293`) |
| +can delete `apps/bab/console/**` — the release history the fleet is *currently |
| +running* — behind an owner check, a typed name and fresh auth, none of which |
| +knows this app is the control plane. |
| + |
| +An app carrying the flag: |
| + |
| +- **refuses trash, purge, rename and transfer outright.** Not "with fresh |
| + auth" — refused, with a message saying the flag must be cleared first. The |
| + console's release history is not something to delete behind a confirmation |
| + dialog. |
| +- **refuses promotion by a service principal.** A person with ops membership |
| + can roll the console; a CI credential cannot. This closes the hole §10 step |
| + ordering would otherwise leave open, without a step-up flow. |
| +- gets §5's two ship-time refusals. |
| + |
| +Promotion by a **person** is gated by ops membership, which is already the |
| +operator boundary, and by nothing else. That is what keeps |
| +`carlos deploy -app console` working as an ordinary command. |
| + |
| +**How the flag is set.** Not by a box-local sqlite write — that contradicts the |
| +no-box-commands corollary. It is set at claim time: `carlos apps create` gains |
| +`-control-plane`, owner-only, and the console's app settings page can clear it |
| +(clearing is the dangerous direction, so clearing requires fresh auth; setting |
| +does not). |
| + |
| +**Test:** trash/purge/rename/transfer on a flagged app are refused naming the |
| +flag; a service principal's promote is refused; a person's promote at any rung |
| +succeeds with ordinary membership; clearing the flag requires fresh auth; |
| +setting it requires owner; an unflagged app is unaffected in every case; the |
| +refusal sits behind the existing 404 so it does not leak the flag's existence |
| +to a non-member. |
| ## 7. The version stamp and the dirty tree |
| `roll-console.sh` refuses a dirty tree because the version stamp is the git |
| -short sha, and a stamp naming a commit the binary does not match is exactly how |
| -a false "verified" happens. `carlos deploy` already defaults `-version` to |
| -`gitShortHead()`, so it inherits the convention — but not the guard. |
| +short sha, and a stamp naming a commit the binary does not match is how a false |
| +"verified" happens. `carlos deploy` already defaults `-version` to |
| +`gitShortHead()` and inherits the convention — but not the guard. |
| + |
| +`carlos deploy` refuses to ship when `-version` was **derived from git** and |
| +the tree is dirty. An explicit `-version` is the operator saying they mean it, |
| +and is not second-guessed. |
| -`carlos deploy` refuses to ship when `-version` was **derived from git** (not |
| -passed explicitly) and the tree is dirty. An explicit `-version` is the |
| -operator saying they mean it, and is not second-guessed. |
| +A general fix: any tenant using the default stamp today can ship a binary whose |
| +sha names a commit it was not built from. |
| -This is a general fix, not a console one: any tenant using the default stamp |
| -today can ship a binary whose sha names a commit it was not built from. |
| +**Test:** dirty tree + derived version refuses before anything ships; dirty |
| +tree + explicit `-version` ships; clean tree ships. |
| -**Test:** a dirty tree with a derived version refuses before anything ships; a |
| -dirty tree with an explicit `-version` ships; a clean tree ships. |
| +## 8. The migration has an ordering trap |
| -## 8. What happens to the pinfra scripts |
| +`store.go`'s `migrate()` runs its `ALTER TABLE apps` block (lines 198–219) |
| +**before** `migrateAppsKey()` (line 231), whose rebuild copies an **enumerated |
| +column list** into `apps_new` (271–288) — and the code already carries a |
| +comment about this hazard at line 280. |
| -`roll-console.sh` and `verify-console.sh` **stay**, reclassified in their own |
| +Add `control_plane` to that ALTER block and any database that has not yet been |
| +through the rebuild — a self-hoster jumping versions — has the column |
| +**silently dropped**, after which every `SELECT control_plane` errors. |
| + |
| +**Requirement:** either add the ALTER *after* `migrateAppsKey()`, or add the |
| +column to `apps_new` and its copy list. The plan picks one and tests the |
| +jumping-a-version path explicitly. |
| + |
| +Otherwise the column is safe: every apps query names its columns (verified — |
| +no `SELECT *`), so no row parser breaks. |
| + |
| +## 9. The pinfra scripts stay |
| + |
| +`roll-console.sh` and `verify-console.sh` **stay**, reclassified in their |
| header comments as **break-glass**: the path that works when the console is |
| down. That is the honest answer to the bootstrap question — the console |
| -mediates its own replacement on the happy path, and §4's automatic rollback |
| -covers a build that will not start, but a console that is *already* down cannot |
| -be the door to its own repair. |
| +mediates its own replacement on the happy path, §4 covers a build that fails |
| +readiness, and §4a's residual (starts, serves, behaves wrong) has no automatic |
| +recovery at all. A console that is already down cannot be the door to its own |
| +repair. |
| `ship-app.sh` and `promote-app.sh` lose their console/kass/keymail/ |
| -platform-site callers. They are not deleted — `promote-app.sh` still serves |
| +platform-site callers but are not deleted; `promote-app.sh` still serves |
| direct-bucket promotes for apps outside any console's knowledge. |
| -## 9. Order of work |
| - |
| -Each step leaves the estate working. |
| - |
| -0. **Check** `HasOwners("console")` on both deployments, and confirm no |
| - `apps/console/` legacy prefix on either. If either is true, §2's exemption |
| - is not sufficient and the plan needs a step before this one. |
| -1. §2 — the #79 owner exemption, with tests. |
| -2. Claim `ops/console` on both deployments. Verify existing release history is |
| - visible through `carlos channels -app console -account ops`. **Nothing is |
| - deployed yet**; this is a read-only change in visible state. |
| -3. §3 — `/healthz` carries the build; the app-declared build probe; the watch's |
| - second exit criterion. |
| -4. §6 — the flag, the fresh-auth gate, set on `ops/console`. |
| -5. §5 — the arch refusal. |
| -6. §7 — the dirty-tree guard. |
| -7. **First `carlos deploy -app console`** on tito, verified against the build |
| - probe. Tito first: it is the smaller blast radius and the deployment whose |
| - console going down does not take carloku with it. |
| -8. The same on flagship. |
| -9. Prove `carlos deploy` on `kass`, `keymail`, `platform-site`; update |
| - CLAUDE.md's "Deploy reality" section, which currently tells the reader to |
| - use the pinfra scripts for all four. |
| -10. Reclassify the pinfra scripts as break-glass in their header comments. |
| -11. File the adoption restart-dedupe follow-up from §4a. |
| - |
| -## 10. Open questions |
| - |
| -- **Does `ops/console` carry a CLI grant?** Step 0. If it does, `HasOwners` |
| - refuses the claim before the bucket check is reached and §2 does not help. |
| -- **Which console host is canonical for the deploy watch?** |
| - `console.carloku.com` is assumed (§4a) because it is already the stamped one. |
| - `roll-console.sh`'s `coords()` names `platform.carlosframework.com` as the |
| - flagship origin, so these two disagree today and the spec picks the stamped |
| - one. Worth a second look during step 7. |
| -- **Does the console's unit ExecStart resolve through a per-host `live/<host>` |
| - symlink, and if so which host's?** Verified only that |
| - `/proc/<pid>/exe` resolves to `/opt/carlos/versions/<sha>/console`. If the |
| - unit is pinned to one host's symlink, §4a's ruling is not merely convenient |
| - but required, and the plan should say so. |
| +## 10. Order of work |
| + |
| +Reordered after review: the flag must exist **before** the claim, and the code |
| +must reach the boxes **before** the first self-deploy. |
| + |
| +**0. Survey both boxes** (read-only, no changes). Per box: `console.service`'s |
| + `ExecStart` path and its `Type=`; which route carries the `Unit` stamp; |
| + every console route and channel (tito is entirely unsurveyed — it has |
| + `console.platform.tito.io` and the still-serving `carlos.tito.io`); |
| + `HasOwners("console")`; any legacy `apps/console/` prefix; the bucket |
| + production flag on `console`. Flagship's readings are in the appendix; |
| + **tito's are not, and step 7 deploys there first.** |
| + |
| +1. §2 owner exemption, with the service-principal pin and the `handleNewApp` |
| + policy change. |
| +2. §8 migration + §6 flag, its destructive-verb refusals, its service-principal |
| + promote refusal, and `-control-plane` on `apps create`. |
| +3. §3 `/healthz` carries the build; the app-level build probe; the watch's |
| + second exit criterion with cache-busting and a naming timeout. |
| +4. §4b restart-recovery: start the unit on the restored version after a rolled |
| + back adoption. **Tenant fix; lands independently of everything else here.** |
| +5. §5 ship-time refusals. §7 dirty-tree guard. |
| +6. **`cmd/console` readiness signalling** (§4a) — the code half of the |
| + `Type=notify` change. |
| +7. **Delivery roll.** One last `roll-console.sh` on both boxes carrying steps |
| + 1–6, plus a CLI release carrying the new watch. Without this, step 9 runs |
| + against a console that predates its own probe and fails. |
| +8. **pinfra:** `console.service` gains the readiness gate (§4a). Applied to |
| + both boxes. |
| +9. **Stamp correctly** (§4c): on each box, move the `Unit` stamp to the host |
| + named in `ExecStart`, exactly one route. Verify with a no-op adopt. |
| +10. **Claim `ops/console` with the flag set, in one step**, on both |
| + deployments. Not two: a row without the flag is a window in which any ops |
| + member or `Publish` service principal can roll the production console. |
| +11. **First `carlos deploy -app console` on tito**, verified against the build |
| + probe. Tito first: smaller blast radius, and its console going down does |
| + not take carloku with it. |
| +12. The same on flagship. |
| +13. Prove `carlos deploy` on `kass`, `keymail`, `platform-site` — each with |
| + step 9's precondition first. `kass` and `keymail` are addr-backed bespoke |
| + services whose `Unit` stamps are unsurveyed; an unstamped `keymail` proof |
| + run is the #110 shape again (symlink swapped, nothing restarted, header |
| + lies) and would "prove" a deploy that did not happen. |
| +14. Update CLAUDE.md's "Deploy reality", which currently sends the reader to |
| + the pinfra scripts for all four apps. Reclassify the scripts as break-glass. |
| +15. File the two follow-ups: the restart-dedupe pass (§4c), and how a **fresh |
| + deployment** gets its `ops/console` row (§11). |
| + |
| +## 11. Open questions |
| + |
| +- **How does a fresh deployment or a self-hoster get the `ops/console` row?** |
| + Nothing creates it — not provisioning, not `materializeQuietly`, not |
| + `BackfillOpsMemberships`. Steps 10 backfills our two boxes by hand. The |
| + general answer belongs to slice 3 or to #135, and is filed at step 15, but it |
| + should be named as an unsolved product gap rather than left implicit. |
| +- **What alarms on a console that is down?** §4a's residual and §4b's |
| + rollback both print into the root pass's journal — box-only. The CLI's |
| + timeout tells the operator who ran the deploy and nobody else. Out of scope |
| + to build here; named so it is not mistaken for covered. |
| +- **`carlos deploy -app console` run against the console being deployed** is |
| + expected to work — the watch tolerates the restart — but its *diagnostics* |
| + leg reads the platform log stream through the console API, so it will error |
| + precisely when it is most wanted. Confirm at step 11 and degrade the |
| + diagnostics gracefully rather than failing the deploy. |
| + |
| +## Appendix — flagship readings, 2026-08-23 |
| + |
| +``` |
| +apps rows in ops (bab): deploy-probe, platform-site (no console) |
| +bucket prefixes under apps/bab/: apt-carlosframework, carloku, carlosframework, |
| + console, deploy-probe, derp, hello, keymail, |
| + meep, platform-site |
| +owners table (grants): derp, meep, platform-site (no console) |
| +legacy apps/console/: absent |
| + |
| +console routes: |
| + platform.carlosframework.com | console | canary/console | (no unit) |
| + console.bab.oncarlos.com | console | (alias) | (no unit) |
| + dashboard.carloku.com | console | canary/console | (no unit) |
| + console.carloku.com | console | canary/console | console.service |
| + |
| +console.service ExecStart: /opt/carlos/live/platform.carlosframework.com serve |
| +console.service Type: not read — step 0 |
| +CONSOLE_ORIGIN: https://console.carloku.com |
| +CONSOLE_DB: /data/console/console.db |
| +``` |
| + |
| +The `ExecStart` host and the stamped route disagree. That is §4c. |