| index 2dea7b998..afcb1b130 100644 |
| --- a/docs/superpowers/specs/2026-09-07-named-unit-config-delivery-design.md |
| +++ b/docs/superpowers/specs/2026-09-07-named-unit-config-delivery-design.md |
| @@ -1,13 +1,14 @@ |
| # Config delivery for routes that name their own unit |
| |
| **Date:** 2026-09-07 |
| -**Status:** approved in chat (Paul, 2026-09-07), written up for a codex |
| -second read before the plan. Every choice below is marked **(decision)** |
| -and can be overturned cheaply; the two in §8 are Paul's **(ruling)**s of |
| -2026-09-07 and are marked where they land. Drafted by Claude Fable from |
| -amadan discussion #114 (carlos/platform), which follows #16 (delivered |
| -but not bounced) and issue #110 (the `Unit` column). Line numbers cite |
| -`origin/main` at `8e16153`. |
| +**Status:** approved in chat (Paul, 2026-09-07), revised after codex |
| +round 1 (§9), awaiting Paul's read of the file before the plan. Every |
| +choice below is marked **(decision)** and can be overturned cheaply; the |
| +two in §8 are Paul's **(ruling)**s of 2026-09-07 and are marked where |
| +they land. Drafted by Claude Fable from amadan discussion #114 |
| +(carlos/platform), which follows #16 (delivered but not bounced) and |
| +issue #110 (the `Unit` column). Line numbers cite `origin/main` at |
| +`8e16153`. |
| |
| ## 1. The problem, as found |
| |
| @@ -66,209 +67,299 @@ needed for the drop-in in §3.2. |
| symlink back (`makeStartRoute`, `agent.go:2593`). Sync is handed the same |
| callback (`agent.go:2950`), and a comment there records that the arm is |
| unreachable from Sync today only because Sync skips every route whose |
| -`Backing` is not `unit` (`sync.go:164-166`). This spec makes it reachable. |
| - |
| -**`Unit` and `Backing=unit` are exclusive.** `carlos route --unit` refuses |
| -a unit-backed row and `--backing unit` refuses a stamped one |
| -(`cli.go:483`, `cli.go:612-630`); the column's doc comment |
| +`Backing` is not `unit` (`sync.go:164-166`). This spec gives Sync its own |
| +way to that unit (§3.2) rather than routing through the host-keyed |
| +callback. |
| + |
| +**`Unit` and `Backing=unit` are exclusive.** `unitRail` (`cli.go:1524`) |
| +refuses a stamp on a unit-backed row and `backingRail` (`cli.go:1442`) |
| +refuses unit backing on a stamped row. `unitRail` deliberately does NOT |
| +refuse `kind: service` rows or `Platform` rows: a service route is |
| +"precisely a sibling process the platform only proxies, and naming the |
| +unit that owns it is a true statement", and a deployment's own console |
| +can be a platform-minted row. The column's doc comment |
| (`internal/registry/registry.go:148-170`) says a stamped route is an |
| addr-backed route whose process no backing-dispatched restart could reach. |
| -`ValidUnitName` (`registry.go:301-316`) pins the name to |
| -`^[A-Za-z0-9@_][A-Za-z0-9@_.\-]{0,63}\.service$`, so a stamp can never |
| -carry a path separator into a root-written path. |
| +`Upsert` validates nothing (`registry.go:160-163`); the rails live at the |
| +doors, and `ValidUnitName` (`registry.go:301-316`) is the door's rail: |
| +`^[A-Za-z0-9@_][A-Za-z0-9@_.\-]{0,63}\.service$`. That rail is about a |
| +typo, not about a root-written path; §3.2 adds the rail this spec needs at |
| +the point of the root write. |
| |
| **The two config halves share one set of predicates, in one package.** |
| `syncable` (`sync.go:284`) is `release.Adoptable` |
| -(`internal/release/adopt.go:816`); `execRouteBase` (`syncexec.go:478`) |
| -narrows it to non-unit backing; `UnitConfigApps` (`sync.go:299`), |
| -`ExecConfigApps` (`syncexec.go:533`), `ConfigRouteFingerprints` |
| -(`internal/appconfig/routefingerprint.go:58`) and `AwaitingRestart` |
| -(`syncexec.go:679`) all compose those two. Each says why it lives beside |
| -its predicate: an app the fast loop watched but the sweep skipped burns |
| -reads forever, and one it did not watch falls back to the hourly timer. |
| -The change in §3.1 is one predicate edit on each side for that reason. |
| +(`internal/release/adopt.go:816`), which admits `kind: instance` only; |
| +`execRouteBase` (`syncexec.go:478`) narrows it to non-unit backing; |
| +`UnitConfigApps` (`sync.go:299`), `ExecConfigApps` (`syncexec.go:533`), |
| +`ConfigRouteFingerprints` (`internal/appconfig/routefingerprint.go:58`) |
| +and `AwaitingRestart` (`syncexec.go:679`) all compose those two. Each says |
| +why it lives beside its predicate: an app the fast loop watched but the |
| +sweep skipped burns reads forever, and one it did not watch falls back to |
| +the hourly timer. §3.1 adds one predicate on each side for that reason. |
| |
| **The set-time notice has three states, not two.** `RestartPending` |
| (`syncexec.go:647`) separates hosts delivered to from hosts it could not |
| decide about, and the CLI renders "could not check" as its own line |
| (`appconfig_cli.go:94-120`, copy at `:152-156`); a console predating the |
| -field sends no `restart` key and the CLI prints nothing. #16's history is |
| -the reason: "nothing needs a restart" and "I could not tell" rendered |
| -identically is how that discussion got filed. §3.4 adds a state and keeps |
| -the discipline. |
| +field sends no `restart` key and the CLI prints nothing. The CLI carries |
| +its own copy of the wire type (`apimode.go:2190-2207`) and converts it in |
| +`restartFromAPI` (`appconfig_cli.go:43`). #16's history is the reason for |
| +the three states: "nothing needs a restart" and "I could not tell" |
| +rendered identically is how that discussion got filed. §3.4 adds a state |
| +and keeps the discipline. |
| + |
| +**An opted-out route keeps its old file, by ruling.** `SyncExec` leaves |
| +`<host>.config.env` exactly as it is when a route leaves scope |
| +(`syncexec.go:420-440`, "RULING: do not auto-delete it"): removing it |
| +would change what the app's own supervisor layers in on its next respawn, |
| +at a moment carlos does not control. §3.3 honours that ruling for the |
| +ownership move too. |
| |
| **Systemd reads `EnvironmentFile` as pid 1.** The rendered file is |
| root-owned `0600` (`sync.go:425`), which is fine for a unit running as any |
| -user, because systemd reads the file before it drops privileges. And |
| -`EnvironmentFile=` settings override `Environment=` lines in the same |
| -unit, so a rendered file wins over the hand-copied |
| -`Environment=WOODSTAR_APPLE_APP_ID=…` the Woodstar units carry today. |
| +user, because systemd reads the file before it drops privileges. |
| +`EnvironmentFile=` settings override `Environment=` lines, so a rendered |
| +file wins over the hand-copied `Environment=WOODSTAR_APPLE_APP_ID=…` the |
| +Woodstar units carry today. Drop-ins are applied in lexical order of file |
| +name across all search directories, and a later `EnvironmentFile=` wins |
| +over an earlier one for the same key; §3.2 chooses the file name with that |
| +in mind. |
| |
| ## 3. The change |
| |
| ### 3.1 Ownership rule: a route that names its unit is the root pass's |
| |
| **(decision)** A route with `Unit != ""` is delivered to, and restarted, |
| -by the root pass, exactly as a `Backing=unit` route is. The agent's exec |
| -sweep never touches it. |
| +by the root pass. The agent's exec sweep never delivers to it. |
| |
| Concretely, in `internal/appconfig`: |
| |
| -- a new predicate `unitDelivered(rt)` = `syncable(rt) && (rt.Backing == |
| - registry.BackingUnit || rt.Unit != "")`, used by `Sync`'s per-route skip |
| - (`sync.go:164`), by `UnitConfigApps` (`sync.go:303`) and by the unit |
| - half of `ConfigRouteFingerprints` (`routefingerprint.go:71`); |
| +- a new predicate `stamped(rt)` = `rt.Unit != "" && rt.App != "" && |
| + (rt.Kind == "instance" || rt.Kind == "service") && (rt.Status == "" || |
| + rt.Status == "ready")`. It is NOT `Adoptable` narrowed: `unitRail` |
| + admits service and platform rows to the stamp (§2), so delivery must |
| + admit them too, or a supported stamp would belong to neither half |
| + (codex R1-6). Aliases carry no stamp; a row that is not ready is left |
| + alone as everywhere else; |
| +- `unitDelivered(rt)` = `(syncable(rt) && rt.Backing == |
| + registry.BackingUnit) || stamped(rt)`, used by `Sync`'s per-route |
| + eligibility (`sync.go:100` and `:137`, both of which test `syncable` |
| + today, and the skip at `:164`), by `UnitConfigApps` (`sync.go:303`) and |
| + by the unit half of `ConfigRouteFingerprints` |
| + (`routefingerprint.go:71`); |
| - `execRouteBase` (`syncexec.go:478-482`) gains `&& rt.Unit == ""`, which |
| removes stamped routes from `execSyncable`, `execAddrCandidate`, |
| - `ExecConfigApps`, the exec fingerprint and `AwaitingRestart` at once. |
| + `ExecConfigApps`, the exec fingerprint and `AwaitingRestart`'s |
| + delivered-to set at once. §3.4 keeps stamped routes in the notice's |
| + "could not tell" set through a separate test. |
| |
| `Sync`'s skip message (`sync.go:165`) stays for the routes it still skips, |
| which after this are exec routes with no stamp. The comment at |
| `agent.go:2899-2907` that explains why Sync never reaches the named-unit |
| -arm is rewritten to say that it now does. |
| +arm is rewritten to say that Sync now restarts stamped units itself. |
| |
| -`release.Adoptable` is untouched; a stamped route was adoptable before and |
| -still is. `restartApps` (`internal/host/restart.go:72`) and the |
| -`-restart-only` pass are untouched: they already admit stamped routes and |
| -dispatch through the same arm. |
| +`release.Adoptable`, `restartApps` (`internal/host/restart.go:72`) and |
| +the `-restart-only` pass are untouched. A consequence worth writing down: |
| +`carlos restart` cycles a stamped `kind: instance` route and not a stamped |
| +`kind: service` one, because `restartApps` is `Adoptable`; that is today's |
| +behaviour and this spec does not widen the restart verb. |
| |
| -### 3.2 Root delivery for a named-unit route |
| +### 3.2 Root delivery for a stamped route |
| |
| -**(decision)** For a `unitDelivered` route with a stamp, `Sync` renders |
| +**(decision)** For a stamped route, `Sync` renders |
| `<envDir>/<host>.env` through `syncHost` (`sync.go:331`) unchanged: the |
| -same resolution, the same key-loss guard for a bound route whose app has no |
| -default bundle, the same atomic write, the same `applyAgentVars` line. The |
| -only addition is that, before the restart, the pass makes sure the unit |
| -reads the file: |
| +same resolution, the same key-loss guard for a bound route whose app has |
| +no default bundle, the same atomic write, the same `applyAgentVars` line. |
| +`syncHost` is split so that it returns whether it wrote, and the caller |
| +decides what follows; for `carlos-app@` routes what follows is exactly |
| +today's `restart(rt.Host)`. |
| + |
| +For a stamped route the pass then makes sure the unit reads the file, at |
| +a fixed location: |
| |
| ``` |
| -/etc/systemd/system/<unit>.d/carlos-env.conf |
| +/etc/systemd/system/<unit>.d/zz-carlos-env.conf |
| |
| -# Managed by carlos: config delivery for the route(s) naming this unit. |
| +# Managed by carlos: config delivery for the route naming this unit. |
| # Edit the route's config with `carlos env` / `carlos secrets`, not this file. |
| [Service] |
| EnvironmentFile=-/etc/carlos/apps/<host>.env |
| ``` |
| |
| -**(decision)** The drop-in file name is fixed, `carlos-env.conf`, and the |
| -directory is `<unit>.d` beside the unit, which is how systemd finds |
| -drop-ins for a unit whether the unit itself lives in `/etc/systemd/system` |
| -or `/usr/lib/systemd/system`. The pass never reads or edits the unit file |
| -itself and never touches any other drop-in. |
| - |
| -**(decision)** One drop-in per unit, one `EnvironmentFile=` line per route |
| -naming that unit, sorted by host. Two routes on one unit is not a shape |
| -anything provisions today, but the file must be deterministic if it |
| -happens, and the alternative, refusing the second route, would leave one |
| -of them silently undelivered, which is the failure this spec exists to |
| -end. The drop-in's content is a pure function of the set of stamped, |
| -unit-delivered routes naming the unit, so it is computed once per pass per |
| -unit, not per route. |
| - |
| -**(decision)** The drop-in is written only when a file is being rendered |
| -for at least one route naming the unit: an app that has never been |
| -configured (`Sync`'s "no manifest object at all" state, `sync.go:114` and `:153`) |
| -gets no drop-in, so a console route stamped under #110 for an app with no |
| -config is left exactly as it is. Once a route has config, its drop-in |
| -exists for as long as the route does; the `-` prefix makes an absent env |
| -file harmless in between. |
| +**(decision)** The location is `/etc/systemd/system/<unit>.d/` always, |
| +never "beside the unit": systemd reads that directory's drop-ins for a |
| +unit whether the unit file lives in `/etc/systemd/system` or |
| +`/usr/lib/systemd/system` (codex R1-5). The file name starts with `zz-` |
| +so that it sorts after `override.conf` and every conventionally named |
| +drop-in; a later `EnvironmentFile=` overrides an earlier one, so carlos's |
| +values win over an operator's earlier file for the same key without |
| +carlos editing anything of the operator's. An operator who adds a drop-in |
| +sorting after `zz-carlos-env.conf` that resets `EnvironmentFile=` has |
| +chosen to; the pass does not read back the unit's effective environment |
| +(§5). |
| + |
| +**(decision)** The rail at the root write (codex R1-1, R1-2). Before the |
| +pass derives any path or writes any byte for a stamped route it checks, |
| +and refuses loudly every pass on failure: |
| + |
| +- `rt.Unit` passes `ValidUnitName`, contains no `@`, and the name before |
| + `.service` does not end in `-`. A template name (`carlos-app@.service`) |
| + has a drop-in directory systemd applies to every instance, and a prefix |
| + name (`carlos-.service`) has one systemd applies to every unit sharing |
| + the prefix; either would deliver one route's bundle to unrelated |
| + processes. An instance name (`x@y.service`) is refused with them for |
| + simplicity: nothing stamps one today, and a template instance that |
| + wants carlos config is `carlos-app@`; |
| +- `rt.Host` is a bare hostname: labels of `[A-Za-z0-9-]`, joined by `.`, |
| + no label empty or starting or ending with `-`, at most 253 bytes. The |
| + existing basename check (`sync.go:335`) stops a path separator; it does |
| + not stop a newline, and a host is about to be written into a file |
| + systemd parses as directives. This check is the one that stops |
| + `relay\nUser=root` being a hostname; |
| +- `/etc/systemd/system/<unit>.d` is created `0755` if absent and, present |
| + or created, is a directory and not a symlink (`Lstat`). A symlinked |
| + drop-in directory would let a root write land somewhere else. |
| + |
| +The same host rail applies to the env file path for stamped routes, in |
| +addition to the basename check every route already gets. |
| + |
| +**(decision)** Ownership of the drop-in is proven by content, not by |
| +name (codex R1-3). The first line of the file is the fixed marker |
| +comment above. The pass replaces or removes only a file whose first line |
| +is that marker; a file at that path without it is left alone and logged |
| +every pass ("not carlos's file; delivery to <host> is blocked until it is |
| +moved"). The marker is not decorative: it is the evidence for every |
| +destructive action the pass takes in that directory. |
| + |
| +**(decision)** One delivered route per unit per box (codex R1-7). If two |
| +stamped routes on the box name one unit, the pass delivers to neither and |
| +says so every pass, naming both hosts, exactly as the key-loss guard |
| +refuses a route rather than guess. A unit has one process environment; |
| +two routes resolving different values for one key cannot both be |
| +delivered, and a rule that picks one silently is the failure this spec |
| +exists to end. Nothing provisions that shape today. |
| |
| **(decision)** Writes are atomic (temp file, rename) and mode `0644`: the |
| -drop-in carries a path, not a secret. A changed or new drop-in is followed |
| -by one `systemctl daemon-reload`, batched once per pass after all drop-ins |
| -are written and before any restart, so a pass that touches several units |
| -reloads once. `daemon-reload` failing is logged per unit and the restart |
| -is skipped for that unit, because a restart that does not load the new |
| -file would stamp nothing true. |
| - |
| -**(decision)** The restart fires when the env file changed, as today, and |
| -also when the drop-in was created or changed while the env file did not, |
| -because a unit that has never loaded the file is running without it |
| -however old the file is. `syncHost` therefore reports whether it wrote, |
| -and the caller combines that with the drop-in result before calling |
| -`restart(rt.Host)`. The callback is the existing `rootRestartRoute` |
| -(`agent.go:2684`); its first arm reaches `try-restart <unit>` and returns |
| -the failure, which Sync already prints as "config updated but restart |
| -failed" (`sync.go:436-438`). |
| +drop-in carries a path, not a secret. |
| + |
| +**(decision)** Application is tracked separately from file content |
| +(codex R1-4). Beside the drop-in, `carlos-env.pending` (not `.conf`; |
| +systemd ignores it) marks that the unit owes a reload and a restart: |
| + |
| +1. When the env file or the drop-in changed, the pass writes the marker |
| + first, then the files. |
| +2. After the route loop, if any marker exists, the pass runs one |
| + `systemctl daemon-reload`. If it fails, every marker stays and every |
| + marked unit's restart is skipped this pass; the failure is logged per |
| + unit. |
| +3. For each marked unit the pass runs `systemctl restart <unit>`, not |
| + `try-restart`: a marker means carlos changed this unit's config and |
| + owes it a restart, and after a restart that failed the unit is |
| + inactive, where `try-restart` is a no-op and would leave it down for |
| + ever. On success the marker is removed; on failure it stays and the |
| + failure is logged with systemd's words, exactly as |
| + `systemctlTryRestartUnit` does. |
| +4. A pass that finds a marker with nothing changed still runs steps 2 |
| + and 3. `carlos env sync` requests a root pass, so an operator who has |
| + fixed a bad value can retry without a box. |
| + |
| +The restart is bound to the unit the pass prepared (codex R1-12): `Sync` |
| +calls a `restartUnit(unit)` callback with `rt.Unit` as read at the start |
| +of the pass, not the host-keyed `restart(host)`, which re-reads the row |
| +and could cycle a unit re-stamped since. `rootRestartRoute` stays as it is |
| +for `carlos-app@` routes and for the release pass. A re-stamp between two |
| +passes is handled by the fingerprint (§3.5) and the reaper below. |
| + |
| +Hibernation needs no special case: `provision` refuses the |
| +addr-with-hibernate shape (`internal/provision/provision.go:438`), and a |
| +stamped route is addr-backed by construction. |
| |
| **(decision)** Reaping. After the route loop, the pass lists |
| -`/etc/systemd/system/*.d/carlos-env.conf`, and removes every one whose |
| -unit is not named by any route row on the box. The same rule as `reap` |
| -for env files (`sync.go:196`): keyed off the full route list, not the |
| +`/etc/systemd/system/*.service.d/zz-carlos-env.conf` and removes every |
| +one that carries the marker and whose unit is named by no route row on the |
| +box, together with its `carlos-env.pending`. The same rule as `reap` for |
| +env files (`sync.go:196`): keyed off the full route list, not the |
| delivered subset, so a route mid-provisioning or unresolved keeps its |
| drop-in; and an empty route list is never authority to delete anything |
| -(`sync.go:175-178`). A removed drop-in also triggers the batched |
| -daemon-reload. A unit that was unstamped (`carlos route --unit -`) or |
| -whose route was removed is therefore left with no carlos line in it on the |
| -next pass; the unit keeps running on whatever it loaded until something |
| -else cycles it, which is right, because an unstamp says carlos no longer |
| -owns that process. Only files named exactly `carlos-env.conf` inside a |
| -`*.service.d` directory are ever candidates; the pass never removes the |
| -directory. |
| - |
| -Hibernation needs no special case: `try-restart` is a no-op on an |
| -inactive unit, as `syncHost` already notes, and provisioning refuses the |
| -addr-with-hibernate shape (`agent.go:2588-2590`). |
| - |
| -### 3.3 The exec side lets go |
| - |
| -**(decision)** `SyncExec` (`syncexec.go:65`) removes |
| -`<dataDir>/<host>.config.env` for a stamped route it would otherwise have |
| -delivered to: the route is in the box's registry with a `Unit`, and the |
| -file in sync's own namespace holds decrypted secrets that nothing on the |
| -agent's side will ever refresh again. `reapConfigEnv` (`syncexec.go:934`) |
| -only reaps hosts with no route row, so a route that moved halves would |
| -keep a stale secrets file forever without this. The removal goes through |
| -the same path the "never configured" state uses, which already removes |
| -rather than truncates; no bounce follows, because there is no child to |
| -bounce for an addr route and the root pass restarts the unit itself. |
| +(`sync.go:175-178`). A removal is followed by the batched daemon-reload. |
| +A file without the marker is never removed. The directory is never |
| +removed. A unit that was unstamped or whose route was removed is left |
| +with no carlos line in it on the next pass; it keeps running on whatever |
| +it loaded until something else cycles it, which is right, because an |
| +unstamp says carlos no longer owns that process. |
| + |
| +### 3.3 The exec side lets go, and leaves the old file alone |
| + |
| +**(decision)** `SyncExec` (`syncexec.go:65`) neither writes nor removes |
| +`<dataDir>/<host>.config.env` for a stamped route: after §3.1 the route is |
| +out of its scope, and the existing ruling at `syncexec.go:428` governs the |
| +file it may have written earlier. Removing it would change what a |
| +supervisor that reads it (the #16 relays are exactly that shape, and could |
| +be stamped later) layers in on its next respawn, at a moment carlos does |
| +not control, and a hand-built unit that names the file without the `-` |
| +prefix would fail to start on its absence. The file goes stale, holds the |
| +values it held, and is reaped when the route row goes, as today. §4 says |
| +so; the first draft of this spec removed it (codex R1-11 found the path |
| +could not even reach it) and the ruling is the better answer. |
| |
| The `exec-delivery: file` opt-in on `woodstar-relay`'s bundle becomes |
| -inert for its stamped routes and stays where it is; an operator may turn |
| -it off, and nothing depends on either. |
| +inert for its stamped route and stays where it is. |
| |
| ### 3.4 The set-time notice: a write that reached nothing |
| |
| -**(decision)** `RestartPending` gains `Undelivered []string`: the hosts of |
| -routes the write reached whose process reads no file carlos writes. The |
| -predicate is `execAddrCandidate(rt)` (which after §3.1 excludes stamped |
| -routes) with the opt-in resolved to off, including the `Unknown` case |
| -`resolveExecDelivery` (`syncexec.go:596`) already decides as off. The |
| -loop in `AwaitingRestart` (`syncexec.go:722-770`) already computes exactly |
| -this verdict per route and `continue`s on `!v.On` with the comment |
| -"nothing to say"; that branch appends to `Undelivered` instead. Routes |
| -whose bundle would not load stay in `Unresolved`, never in `Undelivered`: |
| -"could not tell" is still never rendered as "not delivered". |
| +**(decision)** `RestartPending` gains `Undelivered []string`: hosts of |
| +routes the write reached that no pass delivers a file to on its account. |
| +The predicate is `execAddrCandidate(rt)` (which after §3.1 excludes |
| +stamped routes) with the opt-in resolved to off, including the `Unknown` |
| +case `resolveExecDelivery` (`syncexec.go:596`) already decides as off. |
| +The loop in `AwaitingRestart` (`syncexec.go:722-770`) already computes |
| +exactly that verdict per route and `continue`s on `!v.On`; that branch |
| +appends to `Undelivered` instead. |
| + |
| +What the verdict establishes, precisely (codex R1-9): this write was not |
| +delivered to that route by any pass. It does not say no file exists; an |
| +opted-out route may hold a file from before, by the ruling in §2. The |
| +note's wording says "this write" rather than "no file". |
| + |
| +**(decision)** Stamped routes stay in the "could not tell" set (codex |
| +R1-8). The loop also visits `stamped(rt)` routes of the app, runs the same |
| +bundle loads, and puts one whose bound bundle is unreadable or missing in |
| +`Unresolved`; a stamped route whose bundles load is listed nowhere, |
| +because the root pass delivers and restarts it within seconds. The |
| +classification of who delivers is separate from whether resolution can be |
| +known to fail, and the second is knowable at the door. |
| |
| The same environment and account narrowing applies as for `Hosts`: a |
| named-bundle write reaches the routes bound to it, a default-bundle write |
| -reaches every route of the app. The shadowing test does not apply, since |
| -the point is that no file exists to be unchanged. |
| - |
| -**(decision)** On the wire, `apiConfigRestart` (`internal/console/ |
| -apiconfig.go:495`) gains `undelivered []string, omitempty`. Additive: an |
| -older CLI ignores it and prints what it printed before; an older console |
| -sends no field and the newer CLI prints nothing for it. `Empty()` |
| -(`syncexec.go:655`) counts it. |
| - |
| -**(decision)** The CLI prints one line per host under the same `note:` |
| -prefix, then the existing scope caveat once. Copy direction, to be fixed |
| -by copy-review before the branch lands, not here: |
| +reaches every route of the app. The shadowing test does not apply to |
| +`Undelivered`. |
| + |
| +**(decision)** On the wire, both `apiConfigRestart` types gain |
| +`undelivered []string, omitempty`: the console's (`internal/console/ |
| +apiconfig.go:495`) and the CLI's (`apimode.go:2196`), and |
| +`restartFromAPI` (`appconfig_cli.go:43`) copies it (codex R1-10). |
| +Additive: an older CLI ignores it; an older console sends no field and the |
| +newer CLI prints nothing for it. `Empty()` (`syncexec.go:655`) counts it. |
| + |
| +**(decision)** `RestartHost.Unit`, `apiConfigRestartHost.Unit`, the |
| +`restartCopyUnit` line and `TestAwaitingRestartCarriesUnit` are KEPT |
| +(codex R1-10; the first draft removed them). A new console never sets |
| +`unit`, because a stamped route is not in `Hosts`; an older console still |
| +does, and the remedy it names, `carlos restart`, is still true against |
| +that console's box. The test moves from `AwaitingRestart` to the renderer. |
| + |
| +**(decision)** The CLI prints one line per undelivered host under the |
| +same `note:` prefix, then the existing scope caveat once. Copy direction, |
| +to be fixed by copy-review before the branch lands, not here: |
| |
| ``` |
| -note: woodstar-relay.bdm.oncarlos.com has no file to read. The value is stored, not delivered. Name the unit that runs it (carlos route --unit, on the box) or opt the app into file delivery (carlos env exec-delivery --app woodstar-relay file). |
| +note: this write did not reach woodstar-relay.bdm.oncarlos.com. No pass delivers config to it. If a systemd unit runs its process, name it on the box (carlos route --unit); if the app's own supervisor reads a delivered file, opt the bundle in (carlos env exec-delivery --app woodstar-relay file). |
| (this machine's routes only) |
| ``` |
| |
| -**(decision)** With §3.1, `AwaitingRestart` can no longer return a host |
| -with a `Unit`, because a stamped route is not an `execAddrCandidate`. The |
| -`Unit` field on `RestartHost` and `apiConfigRestartHost`, the |
| -`restartCopyUnit` line (`appconfig_cli.go:153`) and |
| -`TestAwaitingRestartCarriesUnit` are removed. Dropping an `omitempty` |
| -field from a response is additive in the sense that matters: an older CLI |
| -that still reads `unit` sees it absent and prints the plain line, which is |
| -now the true one. A stamped route needs no remedy line because the root |
| -pass restarts it within seconds of the write, like a `carlos-app@` tenant. |
| +Both remedies are named as what they are: the stamp is a box-side verb |
| +today (§5), and the opt-in writes a file that the app's supervisor must |
| +already read. |
| |
| `carlos env sync`'s output already says files land in seconds and |
| anything carlos does not own keeps its old values until restarted; it is |
| @@ -279,27 +370,29 @@ still true and is not changed. |
| **(decision)** The unit row of `ConfigRouteFingerprints` |
| (`routefingerprint.go:67`) appends `rt.Unit`. A re-stamp to a different |
| unit changes which drop-in the route belongs in without changing the env |
| -file's bytes, and the fast loop's registry half (`agent.go:834-841`) is |
| +file's bytes, and the fast loop's registry half (`agent.go:829-841`) is |
| what turns a registry change into a root pass request within a tick |
| rather than an hour. The exec row does not carry it: a stamped route is |
| not in that set. A stamp being added or removed already moves the route |
| between the two sets and so changes both digests. |
| |
| -`genPollPass` (`agent.go:829`) is otherwise untouched: `UnitConfigApps` |
| -now lists apps with stamped routes, so their generation object is polled |
| -and a write requests a root pass through `configSyncMarker` |
| -(`agent.go:888`) exactly as for `carlos-app@` tenants. |
| +`genPollPass` is otherwise untouched: `UnitConfigApps` now lists apps |
| +with stamped routes, so their generation object is polled and a write |
| +requests a root pass through `configSyncMarker` (`agent.go:888`) exactly |
| +as for `carlos-app@` tenants. |
| |
| ### 3.6 Documentation |
| |
| -`docs/site/cli.md`: the `carlos route --unit` paragraph (line 910) says |
| -that naming the unit also makes the root pass deliver the route's config |
| -to it, through a drop-in the pass manages, and restart it on change; the |
| -`carlos add --unit` paragraph (line 884) says the same; the `carlos env` |
| -and `carlos secrets` sections gain one sentence on the "no file to read" |
| -note and its two remedies. `internal/docsite` checks headings and |
| -examples, not this prose, so the gate does not force these, but a change |
| -that alters what a flag does is not done until the page says so. |
| +`docs/site/cli.md`: the `carlos route` section (the paragraph at line |
| +908-911 that introduces `--backing` and `--unit`) says that naming the |
| +unit also makes the root pass deliver the route's config to it, through a |
| +drop-in the pass manages under `/etc/systemd/system/<unit>.d/`, and |
| +restart it on change; the `carlos add --unit` sentence (line 884) says |
| +the same; the `carlos env` and `carlos secrets` sections gain one sentence |
| +on the "did not reach" note and its two remedies. `internal/docsite` |
| +checks headings and examples, not this prose, so the gate does not force |
| +these, but a change that alters what a flag does is not done until the |
| +page says so. |
| |
| AGENTS.md's "Deploy reality" bullet on #110 (line 282) gains one clause: |
| a stamped route also receives its config from the root pass. |
| @@ -307,110 +400,137 @@ a stamped route also receives its config from the root pass. |
| ## 4. Consequences to be explicit about |
| |
| **One restart per stamped, configured route on the first pass after the |
| -roll.** Every such route gets a drop-in it did not have, and §3.2 restarts |
| -on a new drop-in. On today's fleet that is Woodstar's stamped units on |
| -their box and any console route stamped under #110 whose app has config. |
| -This is the same trade the `CARLOS_PURGE` roll accepted (`sync.go:401- |
| -420`): the restart is the feature landing, and a route that stays on the |
| -old file would be exactly the bug being fixed. A route whose app has no |
| -config is not restarted. |
| +roll.** Every such route gets a drop-in it did not have, and §3.2 marks |
| +and restarts on a new drop-in. On today's fleet that is Woodstar's |
| +stamped units on their box and any console route stamped under #110 whose |
| +app has config. This is the same trade the `CARLOS_PURGE` roll accepted |
| +(`sync.go:401-420`): the restart is the feature landing, and a route that |
| +stays on the old file would be exactly the bug being fixed. A route whose |
| +app has no manifest object at all (`sync.go:114`, `:153`) gets no file, no |
| +drop-in, no marker and no restart. |
| |
| **A stamped console cycles itself on its own config change.** After the |
| roll, `carlos env set` on the console's app bounces `console.service` |
| within seconds, as it would any tenant. That is what stamping means, and |
| -it is the behaviour #110 asked for on adoption; config joins it. An |
| -operator who wants a console's config delivered but not applied until a |
| -chosen moment has no verb for that today and did not before either. |
| +it is the behaviour #110 asked for on adoption; config joins it. |
| + |
| +**`restart`, not `try-restart`, when a marker is present.** A unit an |
| +operator stopped while carlos owed it a restart comes back on the next |
| +pass. The marker exists only after carlos changed that unit's config, and |
| +the journal line names the unit and the reason; this is the price of a |
| +failed config restart being recoverable without a box. |
| |
| **The drop-in is a root write into a directory carlos does not own.** It |
| -is additive to the unit, named so that it reads as carlos's, carries no |
| -secret, is removed when the stamp goes, and is written only for a unit an |
| -operator named on the route. A hand-written drop-in with the same file |
| -name would be overwritten; the name is chosen to make that collision |
| -unlikely and the marker line makes it visible. |
| - |
| -**The stale `/data/<host>.config.env` disappears.** For a stamped route |
| -that was opted into file delivery, the agent's next sweep removes the |
| -file (§3.3). Nothing reads it, by the premise of #114, so nothing |
| -changes for the process; the point is that a decrypted secret is not left |
| -in a file no pass refreshes. |
| - |
| -**Two units, one process, is still one arm.** `makeRestartRoute` tries |
| -`Unit` and returns; it never falls through to `carlos-app@<host>` or the |
| -activator for a stamped route. Unchanged, and relied on. |
| +is additive to the unit, marker-owned, carries no secret, is removed when |
| +the stamp goes, and is written only for a unit an operator named on the |
| +route and only after the rail in §3.2. A hand-written file at the same |
| +path without the marker blocks delivery and is reported, never replaced. |
| + |
| +**Two root passes can overlap.** `carlos-adopt.service` and |
| +`carlos-config-sync.service` are not serialised against each other today |
| +and both run `Sync`; the env-file writes already tolerate this (atomic |
| +rename, bytes-equal short circuit). The drop-in and marker writes are the |
| +same shape. Serialising root passes is a change to the existing units and |
| +is not part of this spec (codex R1-12, second half). |
| + |
| +**The stale `/data/<host>.config.env` stays.** For a stamped route that |
| +was opted into file delivery, the agent's old file is left as it is |
| +(§3.3). Nothing carlos owns reads it; it is reaped with the route row. |
| |
| **Multi-box fleets.** The console reads its own box's registry for the |
| notice, as today (#145's blind spot); the caveat line stays. Delivery |
| itself runs on every box that serves the route, each from its own |
| -registry, so a stamped route on two boxes is delivered on both. |
| +registry. |
| |
| ## 5. What is not built |
| |
| - **Ask 3, delivered-generation readback.** Deferred **(ruling 2)**. |
| #11, #13 and #16 want the same absent field; it needs an app-side |
| - signal and deserves one decision across all four threads. |
| -- **A remote door for `--unit`.** Stamping is `carlos route --unit` against |
| - the registry on the box (`cli.go:470-483`). AGENTS.md's rule makes a |
| - box-only step a product gap, and this one remains; it is not needed for |
| - Woodstar, whose route is already stamped, and provisioning a stamp from |
| - an instance record is a design of its own (the record cannot express |
| - `Backing` today either, `internal/provision/reconcile.go`). |
| + signal and deserves one decision across all four threads. The pass |
| + also does not read back a unit's effective environment after restart |
| + (codex R1-5's second half); that is the same absent readback. |
| +- **A remote door for `--unit`.** Stamping is `carlos route --unit` |
| + against the registry on the box (`cli.go:470-483`). AGENTS.md's rule |
| + makes a box-only step a product gap, and this one remains; it is not |
| + needed for Woodstar, whose route is already stamped, and provisioning |
| + a stamp from an instance record is a design of its own (the record |
| + cannot express `Backing` today either, `internal/provision/ |
| + reconcile.go`). |
| - **`UNIT` and `BACKING` columns in `carlos routes`.** The API already |
| - carries both (`internal/console/apiroutes.go:61-62`); printing them is |
| - a small change that belongs with ask 3's column, not here. |
| + carries both (`internal/console/apiroutes.go:61-62`); printing them |
| + belongs with ask 3's column. |
| +- **Widening `carlos restart` to stamped `kind: service` routes.** |
| + `restartApps` is `Adoptable`; a stamped service route is delivered to |
| + and restarted by the config pass but not by the restart verb. Its own |
| + thread if wanted. |
| +- **Serialising the two root passes** (§4). |
| - **A post-delivery hook** for the un-owned addr shape (#16's second |
| - ask). The stamp is the answer for a process a unit runs; the hook would |
| - be for processes nothing on the box supervises. |
| + ask). |
| |
| ## 6. Testing |
| |
| `internal/appconfig/sync_test.go`, beside `TestSyncSkipsExecBackedRoutes`, |
| -with the same registry-and-tempdir harness and a fake `restart` recorder; |
| -the drop-in root and the daemon-reload runner are parameters of the |
| -function under test so that no test touches `/etc` or runs `systemctl`: |
| - |
| -- a stamped route renders `<host>.env`, writes the drop-in with the |
| - expected bytes, calls daemon-reload once, restarts the host; |
| -- a second pass with nothing changed writes nothing, reloads nothing, |
| - restarts nothing; |
| -- an env-file change alone restarts; a drop-in change alone (a re-stamp |
| - to a new unit) restarts and the old unit's drop-in is reaped; |
| -- two stamped routes naming one unit produce one drop-in with two sorted |
| - lines and one reload; |
| -- an app with no manifest object writes no file and no drop-in; |
| +with the same registry-and-tempdir harness. The drop-in root, the |
| +daemon-reload runner and the unit restarter are parameters of the |
| +function under test, so no test touches `/etc` or runs `systemctl`: |
| + |
| +- a stamped `kind: instance` route and a stamped `kind: service` route |
| + each render `<host>.env`, write the drop-in with the expected bytes and |
| + marker, write the marker, reload once, restart the unit, clear the |
| + marker; |
| +- a second pass with nothing changed writes, reloads and restarts |
| + nothing; |
| +- an env-file change alone marks and restarts; a drop-in change alone |
| + (a re-stamp to a new unit) marks and restarts the new unit and reaps |
| + the old unit's drop-in and marker; |
| +- a reload failure leaves the marker and skips the restart; the next |
| + pass with identical bytes reloads and restarts, then clears it; |
| +- a restart failure leaves the marker; the next pass restarts again; |
| +- two stamped routes naming one unit deliver to neither and log both; |
| +- a template name, a prefix name, an instance name, a host with a |
| + newline, and a symlinked drop-in directory are each refused before any |
| + write; |
| +- a file at the drop-in path without the marker is neither replaced nor |
| + removed, and delivery to that host is logged as blocked; |
| +- an app with no manifest object writes no file, no drop-in, no marker; |
| - the key-loss guard still refuses a bound route with no default bundle |
| when the file on disk holds keys the render drops; |
| -- a drop-in for a unit named by no route is removed and reload follows; |
| - an empty route list removes nothing; |
| -- daemon-reload failure logs and skips that unit's restart; |
| -- `UnitConfigApps` and the unit fingerprint include a stamped route; |
| - the fingerprint changes on re-stamp. |
| +- a marker-bearing drop-in for a unit named by no route is removed with |
| + its marker and reload follows; an empty route list removes nothing; |
| +- `UnitConfigApps` and the unit fingerprint include stamped instance and |
| + service routes; the fingerprint changes on re-stamp. |
| |
| `internal/appconfig/syncexec_test.go` and `awaitingrestart_test.go`: |
| |
| - `execRouteBase` rejects a stamped route; `ExecConfigApps` and the exec |
| - fingerprint exclude it; |
| -- `SyncExec` removes an existing `<host>.config.env` for a stamped route |
| - and does not bounce; |
| + fingerprint exclude it; `SyncExec` neither writes nor removes its |
| + `<host>.config.env`; |
| - `AwaitingRestart` lists an un-opted addr route in `Undelivered`, keeps |
| - an unreadable bundle in `Unresolved`, and never lists a stamped route |
| - anywhere; a named-bundle write lists only routes bound to it. |
| + an unreadable bundle in `Unresolved`, puts a stamped route with a |
| + missing bound bundle in `Unresolved` and a healthy stamped route |
| + nowhere, and never lists a stamped route in `Hosts` or `Undelivered`; |
| + a named-bundle write lists only routes bound to it. |
| |
| -`internal/console` and the CLI: the wire field round-trips; the CLI |
| -prints the note for `Undelivered`, prints nothing for a console that |
| -sends no field, and the copy tests are updated for the removed unit line. |
| +`internal/console`, `apimode.go` and the CLI: `undelivered` round-trips |
| +through both wire types and `restartFromAPI`; a response with |
| +`hosts[].unit` from an older console still renders the unit remedy; a |
| +console that sends no field prints nothing. |
| |
| `internal/host`: the comment change at `agent.go:2899` has no test; the |
| restart arm's existing tests (`restart_test.go`) already cover a stamped |
| -route. |
| +route for the release pass. |
| |
| Everything runs under `go test ./... -count=1`; socket-binding tests in |
| this repo need `TMPDIR=/tmp/g`. |
| |
| ## 7. Rollout |
| |
| -1. Land the branch on the forge; CI green on the amadan remote; push |
| - `main` to the GitHub mirror. |
| +1. Push the branch to the forge (`origin` is amadan.net for this |
| + checkout; CI runs there), set the branch description, wait for |
| + `amadan ci status` to pass, land it with `amadan branch merge`, then |
| + fast-forward the GitHub mirror's `main` from forge `main`. AGENTS.md's |
| + Build & test section still describes the earlier arrangement in which |
| + GitHub was `origin`; the forge memory note is current. |
| 2. Roll the platform binary. The first root pass on each box writes the |
| drop-ins and cycles each stamped, configured route once (§4). |
| 3. Verify on Woodstar with no box access: `carlos secrets set -app |
| @@ -427,11 +547,33 @@ this repo need `TMPDIR=/tmp/g`. |
| ## 8. Rulings (Paul, 2026-09-07) |
| |
| 1. **The root pass owns a named-unit route fully.** Render the env file, |
| - write the `EnvironmentFile=` drop-in, daemon-reload, try-restart on |
| + write the `EnvironmentFile=` drop-in, daemon-reload, restart on |
| change. Stamping the unit is the consent. No SSM step, including for |
| `console.service`, which cycles on its own config changes like any |
| `carlos-app@` tenant. **(ruling 1)**, lands in §3.2 and §4. |
| 2. **Ask 3 is deferred to its own thread**, to be decided once across |
| #11, #13, #16 and #114. **(ruling 2)**, lands in §5. |
| 3. The design as presented in chat (§3.1 to §3.6, §4, §6) was approved |
| - without change. |
| + without change; §9 records what the codex read changed afterwards. |
| + |
| +## 9. Review round 1 (codex, 2026-09-07) |
| + |
| +Findings are in `CODEX-FINDINGS-UNIT-CONFIG.md`, unedited. Verdict was |
| +request changes. Disposition: |
| + |
| +| # | Sev | Accepted | Where | |
| +|---|-----|----------|-------| |
| +| R1-1 | P1 | yes: template, prefix and instance names refused at the root write | §3.2 rail | |
| +| R1-2 | P1 | yes: host and unit re-validated at the write; newline-safe host rail; symlink check on the directory | §3.2 rail | |
| +| R1-3 | P1 | yes: marker is the ownership proof for replace and delete; unmarked file blocks and is reported | §3.2 | |
| +| R1-4 | P2 | yes: `carlos-env.pending` marker; `restart` not `try-restart` for a marked unit; retry every pass | §3.2, §4 | |
| +| R1-5 | P2 | partly: fixed `/etc/systemd/system/<unit>.d/` and `zz-` ordering accepted; reading back the effective environment declined as ask 3's readback | §3.2, §5 | |
| +| R1-6 | P2 | yes: `stamped()` admits service and platform rows independently of `Adoptable` | §3.1 | |
| +| R1-7 | P2 | yes: one delivered route per unit; two refuse loudly | §3.2 | |
| +| R1-8 | P2 | yes: stamped routes stay in `Unresolved` when resolution is knowably failing | §3.4 | |
| +| R1-9 | P2 | yes: `Undelivered` means "this write did not reach", remedies stated as what they are | §3.4 | |
| +| R1-10 | P2 | yes: both wire types and `restartFromAPI` updated; `unit` decoding and remedy kept for older consoles | §3.4 | |
| +| R1-11 | P2 | superseded: the file is no longer removed at all, per the existing ruling at `syncexec.go:428` | §3.3 | |
| +| R1-12 | P2 | partly: restart bound to the prepared unit; serialising the two root passes declined as out of scope and recorded | §3.2, §4, §5 | |
| +| R1-13 | P2 | declined: AGENTS.md's Build & test text predates the move of `origin` to the forge; step 1 now spells the actual sequence out | §7 | |
| +| R1-14 | P3 | yes: citations point at `unitRail`, `backingRail`, `provision.go:438`, and the right cli.md paragraph | §2, §3.2, §3.6 | |