amadan

carlos / platform Public

final-fix wave: validate the row Upsert stores, secrets notice for a stamped route, lock wording, dead code, deferred minors

Final-review fix wave for the named-unit-config-delivery branch (brief:
.superpowers/sdd/2026-09-07-named-unit-config-delivery/final-fix-brief.md).

A. `carlos add` now validates the ROW `registry.Upsert` will actually
   store, not the raw flags on this invocation: it opens the registry and
   reads the existing row before the default-socket and flag-combination
   guards, computes effectiveUnit (the flag, or the existing row's unit
   when the flag is blank), and gates the socket default, the
   socket/hibernate refusal, and the final StampShape check on it. A bare
   re-add of an already-stamped route (no --unit on the command line) can
   no longer slip past the guard that keeps a stamped row in StampShape.

B. SyncExec's frozen-secrets notice now also fires for a route that was
   stamped AFTER its <host>.config.env was staged: execAddrCandidate
   excludes a stamped route, so the exec opt-out branch never saw it
   again and stayed silent forever, even though the file still holds
   decrypted secrets nothing will ever touch again.

C. The config-lock message and docs no longer claim contention returns an
   error: lockFileEx's LOCK_EX blocks, so the two passes queue rather than
   interleave, and only a lock FILE that cannot be opened makes the pass
   print and return untouched. Updated the print in sync.go,
   lockConfigSync's doc comment, and spec §3.2's decision paragraph.

D. Deleted the dead hostOutcome/hostFailed/hostUnchanged/hostChanged type
   (never referenced) and the dead `if !existed { cur = nil }` in
   prepareHost (os.ReadFile already returns a nil slice on error). Moved
   a short version of the three-state note onto unitWork.ok's doc comment.

E. Deferred minors: UnitConfigApps' doc now names unitDelivered's actual
   test; SyncUnits guards a stamped route planned no unit; applyStamped
   checks the post-reload probe's LoadState before comparing env files;
   listMarkedDropIns returns a third `unreadable map[string]error` for
   drop-ins that errored on read, and reapDropIns prints one line per
   unreadable unit instead of silently skipping it; RunAdopt's doc names
   appconfig.SyncUnits and the drop-in/daemon-reload writes it also does;
   HostRailReason centralizes the not-a-bare-hostname refusal text shared
   by planStamped and BlockedStamps.

Tests: TestAddOnAStampedRouteStaysStamped (new, cli_test.go) covers item
A's two subprocess scenarios; TestSyncExecLeavesAStampedRoutesOldFileAlone
extended with captureStdout for item B; TestListMarkedDropInsReportsAnUnreadableFile
(new) and the two existing listMarkedDropIns tests updated for the third
return value.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjtuveugrRzjMsc4NJDKAb
Paul Campbell pushed by paul@keymail.dev 171f1faab94f98d16bedc95377a677c22c026970 parent 2e5b50f
12 files changed, +251 −67
  • cli.go +25 −13
  • cli_test.go +78 −0
  • docs/superpowers/specs/2026-09-07-named-unit-config-delivery-design.md +6 −5
  • internal/appconfig/dropin.go +16 −3
  • internal/appconfig/dropin_test.go +28 −2
  • internal/appconfig/stamped.go +10 −0
  • internal/appconfig/sync.go +11 −22
  • internal/appconfig/syncexec.go +28 −12
  • internal/appconfig/syncexec_test.go +12 −3
  • internal/appconfig/unitsync.go +23 −3
  • internal/host/agent.go +9 −4
  • internal/host/unitops.go +5 −0
diff --git a/cli.go b/cli.go
index c5e130aae..accd3fbf2 100644
--- a/cli.go
+++ b/cli.go
@@ -112,6 +112,24 @@ func runAdd(args []string) {
log.Fatalf("add %s: --backing unit with --unit %s: a unit-backed route's process is the carlos-app@%s unit the platform derives and already restarts, "+
"so a second unit name here would be two answers to one question — drop one of the two flags", host, *unit, host)
}
+ // Opened here, before the default-socket and flag-combination guards
+ // below, so both validate the ROW THAT WOULD RESULT rather than the raw
+ // flags on this invocation: a bare re-add of an existing stamped route
+ // (e.g. `carlos add -app relay -channel edge relay.example.com`, no
+ // --unit) still names a unit via the existing row, and must not slip
+ // past the guards that exist to keep a stamped row in StampShape.
+ r, err := registry.Open(*reg)
+ if err != nil {
+ log.Fatal(err)
+ }
+ existing, err := r.Get(host)
+ if err != nil {
+ log.Fatal(err)
+ }
+ effectiveUnit := *unit
+ if effectiveUnit == "" && existing != nil {
+ effectiveUnit = existing.Unit
+ }
// The root pass delivers config to a stamped route only in one shape
// (appconfig.StampShape); this door must not mint a row it would refuse
// every pass. Only the unambiguous flag-level combinations are refused
@@ -121,11 +139,11 @@ func runAdd(args []string) {
// addr-backed route has no --addr on this invocation at all, and the
// existing row's Addr is merged in below — so that check waits until
// the row that would actually result is built.
- if *unit != "" && (*socket != "" || *hibernate) {
- log.Fatalf("add %s: --unit %s needs --addr and neither --socket nor --hibernate: the root pass delivers config to a stamped route only when a unit owns an addr-backed, always-on process", host, *unit)
+ if effectiveUnit != "" && (*socket != "" || *hibernate) {
+ log.Fatalf("add %s: unit %s needs --addr and neither --socket nor --hibernate: the root pass delivers config to a stamped route only when a unit owns an addr-backed, always-on process — clear the stamp first with `carlos route --host %s --unit -`", host, effectiveUnit, host)
}
// A stamped route never gets a default socket; it needs an addr.
- if *socket == "" && *addr == "" && *kind == "instance" && *unit == "" {
+ if *socket == "" && *addr == "" && *kind == "instance" && effectiveUnit == "" {
if *backing == registry.BackingUnit {
// The carlos-app@<host>.socket unit's path — enable that unit
// alongside this route (or use --addr for a TCP-only app). Not a
@@ -189,10 +207,6 @@ func runAdd(args []string) {
log.Fatalf("add %s: %s (%s)", host, daemonHibernateRefusal, note)
}
}
- r, err := registry.Open(*reg)
- if err != nil {
- log.Fatal(err)
- }
// The account is stamped on the row: it is what adopt and the edge build
// this app's bucket keys from. Reads still fall back to the legacy
// unqualified keys, so an unmigrated app added here keeps working.
@@ -201,10 +215,6 @@ func runAdd(args []string) {
// verb the operator reads back must say which one happened (issue #69):
// "added" printed over a rewrite is how vitogo's operator was left
// believing a fresh row existed while a running edge kept the old one.
- existing, err := r.Get(host)
- if err != nil {
- log.Fatal(err)
- }
route := registry.Route{
Host: host, Slug: *slug, Kind: *kind, Socket: *socket, Addr: *addr,
App: *app, Account: account,
@@ -256,8 +266,10 @@ func runAdd(args []string) {
// must not be minted here either. Its "no addr upstream" reason is
// accurate for a fresh add with no --addr and no existing row to fall
// back to.
- if route.Unit != "" {
- if err := appconfig.StampShape(route); err != nil {
+ if effectiveUnit != "" {
+ effective := route
+ effective.Unit = effectiveUnit
+ if err := appconfig.StampShape(effective); err != nil {
log.Fatalf("add %s: %v — the root pass delivers config to a stamped route only when a unit owns an addr-backed, always-on process", host, err)
}
}
diff --git a/cli_test.go b/cli_test.go
index 609b5daac..906c1aa65 100644
--- a/cli_test.go
+++ b/cli_test.go
@@ -922,6 +922,84 @@ func TestAddUnitStamp(t *testing.T) {
}
}
+// TestAddOnAStampedRouteStaysStamped covers final-review item A: `runAdd`
+// must validate the row `registry.Upsert` will actually store, not the raw
+// flags on this invocation — a routine re-add of an already-stamped route
+// with no --unit on the command line still has a unit once Upsert's
+// CASE-preserved `unit` column is accounted for, and must not slip past the
+// guards that keep a stamped row in appconfig.StampShape.
+func TestAddOnAStampedRouteStaysStamped(t *testing.T) {
+ bin := buildCarlos(t)
+ tmp := t.TempDir()
+ scratch := t.TempDir()
+ regPath := filepath.Join(tmp, "registry.db")
+
+ run := func(args ...string) (string, error) {
+ t.Helper()
+ cmd := exec.Command(bin, args...)
+ cmd.Env = carlosTestEnv(t)
+ cmd.Dir = scratch
+ out, err := cmd.CombinedOutput()
+ return string(out), err
+ }
+
+ const host = "relay.example.com"
+ if out, err := run("add", "-registry", regPath, "-app", "relay",
+ "-unit", "relay.service", "-addr", "127.0.0.1:8451", host); err != nil {
+ t.Fatalf("add stamped route: %v\n%s", err, out)
+ }
+
+ // (a) A bare re-add naming only -channel: no --unit, no --socket, no
+ // --hibernate on this invocation. The row that results must still be
+ // stamped (Upsert preserves the existing unit column), and the default
+ // socket / hibernate guards must have seen that, not an empty flag.
+ out, err := run("add", "-registry", regPath, "-app", "relay",
+ "-channel", "edge", host)
+ if err != nil {
+ t.Fatalf("bare re-add of a stamped route: want exit 0, got %v\n%s", err, out)
+ }
+ reg, err := registry.Open(regPath)
+ if err != nil {
+ t.Fatalf("open registry: %v", err)
+ }
+ defer reg.Close()
+ rt, err := reg.Get(host)
+ if err != nil || rt == nil {
+ t.Fatalf("get %s: %+v, %v", host, rt, err)
+ }
+ if rt.Unit != "relay.service" {
+ t.Errorf("Unit = %q, want relay.service preserved across a bare re-add", rt.Unit)
+ }
+ if rt.Addr != "127.0.0.1:8451" {
+ t.Errorf("Addr = %q, want the existing addr preserved", rt.Addr)
+ }
+ if rt.Socket != "" {
+ t.Errorf("Socket = %q, want none: a stamped route never gets the default socket", rt.Socket)
+ }
+ if rt.Hibernate {
+ t.Error("Hibernate = true, want false")
+ }
+ if rt.Channel != "edge" {
+ t.Errorf("Channel = %q, want edge", rt.Channel)
+ }
+
+ // (b) Re-adding that same stamped route with -hibernate (still no
+ // --unit on the command line) must be refused: the row that would
+ // result is stamped AND hibernating, which appconfig.StampShape
+ // forbids. The refusal must name the unit and the way to unstamp.
+ out, err = run("add", "-registry", regPath, "-app", "relay",
+ "-hibernate", host)
+ if err == nil {
+ t.Fatalf("re-add of a stamped route with -hibernate: want non-zero exit, got 0\n%s", out)
+ }
+ if !strings.Contains(out, "relay.service") {
+ t.Errorf("refusal = %q, want it to name the unit relay.service", out)
+ }
+ if !strings.Contains(out, "--unit -") {
+ t.Errorf("refusal = %q, want it to name `--unit -` as the way to unstamp", out)
+ }
+}
+
// --- release versioning: the CLI surface -------------------------------
//
// (release versioning spec, 2026-08-08.) Driven through the real binary in
diff --git 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
index 1ec7a5b65..35902b8df 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
@@ -307,11 +307,12 @@ drop-in carries a path, not a secret.
**(decision)** One config pass at a time (codex R2-3). `Sync` takes an
exclusive flock on `<envDir>/.sync.lock` for its whole run, the shape
-`restartPass` already has, and returns without touching anything when it
-cannot; the hourly `carlos-adopt.service` and the triggered
-`carlos-config-sync.service` therefore never interleave inside a config
-transaction, and the next trigger or timer runs the pass that stepped
-aside. No unit file changes.
+`restartPass` already has, and blocks until the other pass releases it, so
+the two passes queue rather than interleave; only a lock file that cannot
+be opened makes the pass print and return without touching anything; the
+hourly `carlos-adopt.service` and the triggered `carlos-config-sync.service`
+therefore never interleave inside a config transaction, and the next
+trigger or timer runs the pass that stepped aside. No unit file changes.
**(decision)** Application is tracked separately from file content, and
authorised only by successful preparation (codex R1-4, R2-2, R2-4).
diff --git a/internal/appconfig/dropin.go b/internal/appconfig/dropin.go
index f8af494fb..1a03a9fc3 100644
--- a/internal/appconfig/dropin.go
+++ b/internal/appconfig/dropin.go
@@ -147,14 +147,27 @@ func unitDirs(root string) (units, skipped []string) {
// listMarkedDropIns: units whose plain <unit>.d holds a marker-bearing
// drop-in. An unmarked file at the path is not carlos's and is not listed.
-func listMarkedDropIns(root string) (units, skipped []string) {
+// unreadable carries the units whose drop-in file exists but could not be
+// read (permission denied, a directory where the file should be, ...),
+// keyed to the error that readDropIn returned: those units are neither
+// listed nor silently dropped — the caller decides what "unreadable" means
+// for its own pass (reapDropIns: not reaped).
+func listMarkedDropIns(root string) (units, skipped []string, unreadable map[string]error) {
dirs, skipped := unitDirs(root)
for _, u := range dirs {
- if _, exists, marked, err := readDropIn(root, u); err == nil && exists && marked {
+ _, exists, marked, err := readDropIn(root, u)
+ if err != nil {
+ if unreadable == nil {
+ unreadable = map[string]error{}
+ }
+ unreadable[u] = err
+ continue
+ }
+ if exists && marked {
units = append(units, u)
}
}
- return units, skipped
+ return units, skipped, unreadable
}
// listPending: units whose plain <unit>.d holds a pending marker.
diff --git a/internal/appconfig/dropin_test.go b/internal/appconfig/dropin_test.go
index 297c529f4..eb0f6060b 100644
--- a/internal/appconfig/dropin_test.go
+++ b/internal/appconfig/dropin_test.go
@@ -68,10 +68,13 @@ func TestReadDropInSeesAnUnmarkedFile(t *testing.T) {
if err != nil || !exists || marked {
t.Fatalf("exists=%v marked=%v err=%v, want an existing unmarked file", exists, marked, err)
}
- units, _ := listMarkedDropIns(root)
+ units, _, unreadable := listMarkedDropIns(root)
if len(units) != 0 {
t.Fatalf("listMarkedDropIns = %v, want none for an unmarked file", units)
}
+ if len(unreadable) != 0 {
+ t.Fatalf("unreadable = %v, want none for a plain unmarked file", unreadable)
+ }
}
func TestEnsureDropInDirRefusesSymlinkAndFile(t *testing.T) {
@@ -101,18 +104,41 @@ func TestListMarkedDropInsSkipsSymlinkedDirs(t *testing.T) {
os.Symlink(elsewhere, filepath.Join(root, "retired.service.d"))
os.MkdirAll(filepath.Join(root, "real.service.d"), 0o755)
os.WriteFile(filepath.Join(root, "real.service.d", DropInFile), renderDropIn("/e", "h"), 0o644)
- units, skipped := listMarkedDropIns(root)
+ units, skipped, unreadable := listMarkedDropIns(root)
if len(units) != 1 || units[0] != "real.service" {
t.Fatalf("units = %v, want [real.service]", units)
}
if len(skipped) != 1 || skipped[0] != "retired.service" {
t.Fatalf("skipped = %v, want [retired.service]", skipped)
}
+ if len(unreadable) != 0 {
+ t.Fatalf("unreadable = %v, want none", unreadable)
+ }
if _, err := os.Stat(filepath.Join(elsewhere, DropInFile)); err != nil {
t.Fatal("the file behind the symlink must be untouched")
}
}
+func TestListMarkedDropInsReportsAnUnreadableFile(t *testing.T) {
+ root := t.TempDir()
+ dir := filepath.Join(root, "relay.service.d")
+ os.MkdirAll(dir, 0o755)
+ // A directory where the drop-in file should be: os.ReadFile fails with
+ // something other than IsNotExist, which is the shape this covers —
+ // permission-denied on a real box reads the same way to the caller.
+ os.MkdirAll(filepath.Join(dir, DropInFile), 0o755)
+ units, skipped, unreadable := listMarkedDropIns(root)
+ if len(units) != 0 {
+ t.Fatalf("units = %v, want none for an unreadable drop-in", units)
+ }
+ if len(skipped) != 0 {
+ t.Fatalf("skipped = %v, want none: the directory itself is plain", skipped)
+ }
+ if err, ok := unreadable["relay.service"]; !ok || err == nil {
+ t.Fatalf("unreadable = %v, want relay.service with a non-nil error", unreadable)
+ }
+}
+
func TestRemoveDropIn(t *testing.T) {
root := t.TempDir()
ensureDropInDir(root, "relay.service")
diff --git a/internal/appconfig/stamped.go b/internal/appconfig/stamped.go
index 39e9acda1..337d7bb54 100644
--- a/internal/appconfig/stamped.go
+++ b/internal/appconfig/stamped.go
@@ -88,3 +88,13 @@ func BareHostname(host string) bool {
}
return true
}
+
+// HostRailReason is the refusal text for a route whose host fails
+// BareHostname, shared so planStamped (the pass) and BlockedStamps (the
+// notice) report the SAME reason for the same fact — the drift this whole
+// file exists to end. It stays a string rather than an error: planStamped
+// keys a map on it and BlockedStamps carries it as BlockedHost.Reason,
+// neither of which wants an error to unwrap.
+func HostRailReason(host string) string {
+ return fmt.Sprintf("%q is not a bare hostname; refusing to write it into a systemd drop-in", host)
+}
diff --git a/internal/appconfig/sync.go b/internal/appconfig/sync.go
index 27de091e9..283ec8923 100644
--- a/internal/appconfig/sync.go
+++ b/internal/appconfig/sync.go
@@ -54,7 +54,7 @@ func SyncUnits(ctx context.Context, reg *registry.Registry, s release.Store, pri
if ops != nil && ops.Lock != nil {
unlock, err := ops.Lock()
if err != nil {
- fmt.Printf(" config sync: another pass holds the lock (%v); nothing touched this pass\n", err)
+ fmt.Printf(" config sync: cannot take the config lock (%v); nothing touched this pass\n", err)
return nil
}
defer unlock()
@@ -185,6 +185,10 @@ func SyncUnits(ctx context.Context, reg *registry.Registry, s release.Store, pri
continue
}
unit := plan.unitOf[rt.Host]
+ if unit == "" {
+ fmt.Printf(" %s: stamped but planned no unit; skipped\n", rt.Host)
+ continue
+ }
work[unit] = prepareStamped(rt, unit, m, r.defaultMissing, priv, envDir, ops)
continue
}
@@ -325,11 +329,12 @@ func syncable(rt registry.Route) bool {
}
// UnitConfigApps lists the distinct (account, app) pairs having at least
-// one unit-backed syncable route on this box — the apps whose config the
-// ROOT adopt pass delivers (Sync + try-restart), which the agent itself
-// cannot. The fast loop uses it to decide when to request a root pass
-// (see configGenLoop): same dedupe-and-sort shape as ExecConfigApps, and
-// the two may overlap — an app can have both route shapes at once.
+// one unit-delivered route on this box — a syncable carlos-app@ backing or a
+// unit stamp (see unitDelivered) — the apps whose config the ROOT adopt pass
+// delivers (Sync + try-restart), which the agent itself cannot. The fast
+// loop uses it to decide when to request a root pass (see configGenLoop):
+// same dedupe-and-sort shape as ExecConfigApps, and the two may overlap —
+// an app can have both route shapes at once.
//
// Exported from THIS file, beside the predicate Sync itself gates on, for
// the reason ExecConfigApps gives for living beside its own: an app the
@@ -387,19 +392,6 @@ func syncHost(rt registry.Route, m *Manifest, defaultMissing bool, priv *ecdh.Pr
fmt.Printf(" %s: config updated, restarted\n", rt.Host)
}
-// hostOutcome is what one route's env file did this pass. Three states,
-// not two (spec §3.2, codex R2-2): a refusal, a render error or a write
-// error are all hostFailed, and a caller that treats "nothing changed" and
-// "could not write" alike would restart a unit against a stale or absent
-// file.
-type hostOutcome int
-
-const (
- hostFailed hostOutcome = iota
- hostUnchanged
- hostChanged
-)
-
// prepareHost is the read-and-render half of syncHost: everything up to
// the decision to write, with every refusal syncHost makes. ok == false
// means a refusal or error was printed and nothing may be written for this
@@ -420,9 +412,6 @@ func prepareHost(rt registry.Route, m *Manifest, defaultMissing bool, priv *ecdh
fmt.Printf(" %s: read %s: %v\n", rt.Host, path, err)
return "", nil, nil, false, false
}
- if !existed {
- cur = nil
- }
// The same protection an UNBOUND route gets from Sync's "no manifest
// object at all ⇒ leave the file alone" skip, applied to the DEFAULT half
// of a bound route's merge (2026-08-02 review, CRITICAL 1). Binding an
diff --git a/internal/appconfig/syncexec.go b/internal/appconfig/syncexec.go
index 7e5c32fa4..aba6e0640 100644
--- a/internal/appconfig/syncexec.go
+++ b/internal/appconfig/syncexec.go
@@ -447,18 +447,34 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
// the app's own supervisor has stopped reading it) because carlos has
// no way to know when that is.
for _, rt := range routes {
- if !execAddrCandidate(rt) {
- continue
- }
- c := checkOptedIn(release.RouteAccount(rt), rt.App, rt.Environment)
- if c.v || !c.known {
- continue
- }
- path := filepath.Join(dataDir, rt.Host+configEnvSuffix)
- if _, err := os.Stat(path); err != nil {
- continue // nothing was ever staged for this route: nothing to warn about
+ switch {
+ case execAddrCandidate(rt):
+ c := checkOptedIn(release.RouteAccount(rt), rt.App, rt.Environment)
+ if c.v || !c.known {
+ continue
+ }
+ path := filepath.Join(dataDir, rt.Host+configEnvSuffix)
+ if _, err := os.Stat(path); err != nil {
+ continue // nothing was ever staged for this route: nothing to warn about
+ }
+ fmt.Printf(" %s: exec_delivery is off (or was never set) but %s still exists from an earlier opt-in — it holds DECRYPTED secrets and carlos will never update or remove it while this app stays un-opted-in; delete it by hand once the app's own supervisor has stopped reading it\n", rt.Host, path)
+ case Stamped(rt):
+ // A route can gain a unit stamp AFTER exec delivery already
+ // staged this file (the test above this comment's home,
+ // TestSyncExecLeavesAStampedRoutesOldFileAlone, does exactly
+ // that): execAddrCandidate excludes it the moment it is
+ // stamped, so the opt-out branch above never sees it again and
+ // stays silent forever. The same "frozen, not updated" fact is
+ // true here for a different reason — the unit's drop-in now
+ // owns EnvironmentFile, so nothing even reads this file — and
+ // it deserves the same one-line-per-pass notice rather than
+ // silence.
+ path := filepath.Join(dataDir, rt.Host+configEnvSuffix)
+ if _, err := os.Stat(path); err != nil {
+ continue // nothing was ever staged for this route: nothing to warn about
+ }
+ fmt.Printf(" %s: %s is from before this route named its unit; nothing reads it now (the unit's drop-in resets its EnvironmentFile list) and carlos will never update or remove it — delete it once you are satisfied\n", rt.Host, path)
}
- fmt.Printf(" %s: exec_delivery is off (or was never set) but %s still exists from an earlier opt-in — it holds DECRYPTED secrets and carlos will never update or remove it while this app stays un-opted-in; delete it by hand once the app's own supervisor has stopped reading it\n", rt.Host, path)
}
// An empty route list is never authority to delete anything —
// registry.Open silently creates an empty database on a wrong path or
@@ -700,7 +716,7 @@ func BlockedStamps(routes []registry.Route, acct, app, envName string) []Blocked
case DeliverableUnitName(rt.Unit) != nil:
out = append(out, BlockedHost{rt.Host, DeliverableUnitName(rt.Unit).Error()})
case !BareHostname(rt.Host):
- out = append(out, BlockedHost{rt.Host, fmt.Sprintf("%q is not a bare hostname", rt.Host)})
+ out = append(out, BlockedHost{rt.Host, HostRailReason(rt.Host)})
case byUnit[rt.Unit] > 1:
out = append(out, BlockedHost{rt.Host, fmt.Sprintf("two or more routes on this box name %s; one unit takes one route's config", rt.Unit)})
}
diff --git a/internal/appconfig/syncexec_test.go b/internal/appconfig/syncexec_test.go
index 8cb0860b5..8deeb536d 100644
--- a/internal/appconfig/syncexec_test.go
+++ b/internal/appconfig/syncexec_test.go
@@ -1976,9 +1976,11 @@ func TestSyncExecLeavesAStampedRoutesOldFileAlone(t *testing.T) {
if err := SetEnv(ctx, s, "bab", "relay", map[string]string{"TOKEN": "new"}); err != nil {
t.Fatal(err)
}
- if err := SyncExec(ctx, reg, s, privKey, dataDir, nil, ""); err != nil {
- t.Fatal(err)
- }
+ out := captureStdout(t, func() {
+ if err := SyncExec(ctx, reg, s, privKey, dataDir, nil, ""); err != nil {
+ t.Fatal(err)
+ }
+ })
after, err := os.ReadFile(path)
if err != nil {
t.Fatalf("file removed for a stamped route: %v", err)
@@ -1986,4 +1988,11 @@ func TestSyncExecLeavesAStampedRoutesOldFileAlone(t *testing.T) {
if string(after) != string(before) {
t.Fatalf("file refreshed for a stamped route: %q -> %q", before, after)
}
+ // The frozen-secrets warning must still fire once the route is
+ // stamped, not just for the exec opt-out shape it was written for
+ // (final review item B): this file still holds decrypted secrets and
+ // carlos still won't ever touch it again.
+ if !strings.Contains(out, "relay.example.com") || !strings.Contains(out, "never update or remove") {
+ t.Fatalf("no stamped-route frozen-secrets notice for relay.example.com: %q", out)
+ }
}
diff --git a/internal/appconfig/unitsync.go b/internal/appconfig/unitsync.go
index 9e819e8f8..3f1c04712 100644
--- a/internal/appconfig/unitsync.go
+++ b/internal/appconfig/unitsync.go
@@ -80,7 +80,7 @@ func planStamped(routes []registry.Route, ops *UnitOps) stampedPlan {
continue
}
if !BareHostname(rt.Host) {
- p.refused[rt.Host] = fmt.Sprintf("%q is not a bare hostname; refusing to write it into a systemd drop-in", rt.Host)
+ p.refused[rt.Host] = HostRailReason(rt.Host)
continue
}
byUnit[rt.Unit] = append(byUnit[rt.Unit], rt.Host)
@@ -129,7 +129,11 @@ type unitWork struct {
host string
// ok: both files converged this pass (changed or unchanged). False
// means a refusal or write failure was printed and this unit may be
- // reloaded but must not be restarted this pass.
+ // reloaded but must not be restarted this pass. Three states, not two
+ // (spec §3.2, codex R2-2): a refusal, a render error and a write error
+ // all leave ok false; "nothing changed" and "changed" both leave it
+ // true — it is the marker, not this flag, that says whether a restart
+ // is owed.
ok bool
}
@@ -201,10 +205,22 @@ func reapDropIns(routes []registry.Route, ops *UnitOps) {
named[rt.Unit] = true
}
}
- units, skipped := listMarkedDropIns(ops.DropInRoot)
+ units, skipped, unreadable := listMarkedDropIns(ops.DropInRoot)
for _, unit := range skipped {
fmt.Printf(" %s: %s is not a plain directory; not entered\n", unit, dropInDir(ops.DropInRoot, unit))
}
+ // Unreadable, not skipped: the directory is fine but its drop-in file
+ // could not be read (permission denied, a directory where the file
+ // should be, ...), so whether it is ours and whether it is marked is
+ // unknown — printed and left exactly as it is, never reaped on a guess.
+ unreadableUnits := make([]string, 0, len(unreadable))
+ for unit := range unreadable {
+ unreadableUnits = append(unreadableUnits, unit)
+ }
+ sort.Strings(unreadableUnits)
+ for _, unit := range unreadableUnits {
+ fmt.Printf(" %s: drop-in unreadable (%v); not reaped\n", unit, unreadable[unit])
+ }
for _, unit := range units {
if named[unit] {
continue
@@ -276,6 +292,10 @@ func applyStamped(envDir string, ops *UnitOps, plan stampedPlan, work map[string
fmt.Printf(" %s: post-reload probe failed (%v); restart still owed\n", unit, err)
continue
}
+ if st, ok := states[unit]; !ok || st.LoadState != "loaded" {
+ fmt.Printf(" %s: %s is not loaded after reload (%s); restart still owed\n", w.host, unit, st.LoadState)
+ continue
+ }
want := filepath.Join(envDir, w.host+".env")
if !envFilesMatch(states[unit].EnvFiles, want) {
fmt.Printf(" %s: delivery blocked: %s reads %v after reload, want exactly [%s]; a later drop-in owns the EnvironmentFile list\n",
diff --git a/internal/host/agent.go b/internal/host/agent.go
index 611bf39f7..a3c411e90 100644
--- a/internal/host/agent.go
+++ b/internal/host/agent.go
@@ -2836,11 +2836,16 @@ func openStatusStore(ctx context.Context, getenv func(string) string) (release.S
// RunAdopt is the `carlos adopt` verb: one immediate adoption pass — the
// carlos-adopt.timer unit's ExecStart, and an operator's "converge now".
// Runs as root (see carlos-adopt.service): this is the ONLY place the
-// UNIT-BACKED half of config sync (appconfig.Sync) may run, because it
-// writes /etc/carlos/apps and restarts units — the same filesystem-write
-// shape release staging needs root for, and exactly what the sandboxed
+// UNIT-BACKED half of config sync (appconfig.Sync, which is
+// appconfig.SyncUnits underneath) may run, because it writes
+// /etc/carlos/apps and restarts units — the same filesystem-write shape
+// release staging needs root for, and exactly what the sandboxed
// carlos-edge.service's in-process adopt loop (ProtectSystem=strict,
-// ReadWritePaths=/data /run/carlos only) cannot do. The EXEC-BACKED half
+// ReadWritePaths=/data /run/carlos only) cannot do. For a route that names
+// its own unit (spec 2026-09-07-named-unit-config-delivery-design.md
+// §3.2), this same pass also writes that unit's drop-in under
+// /etc/systemd/system/<unit>.d/ and runs `daemon-reload` before it
+// restarts anything — root-only for the same reason. The EXEC-BACKED half
// (appconfig.SyncExec) runs elsewhere, in the agent's in-process
// configSyncLoop: it needs neither root nor /etc access, because it only
// ever writes under /data and bounces its own children.
diff --git a/internal/host/unitops.go b/internal/host/unitops.go
index 4413de7c8..b8dafa26a 100644
--- a/internal/host/unitops.go
+++ b/internal/host/unitops.go
@@ -33,6 +33,11 @@ func productionUnitOps(envDir string) *appconfig.UnitOps {
// exclusive flock on <path>.lock held for the whole of one SyncUnits, so
// carlos-adopt.service and carlos-config-sync.service queue rather than
// interleave inside a config transaction (spec §3.2, codex R2-3).
+// lockFileEx's LOCK_EX blocks until the other pass releases the lock, so
+// contention never surfaces as an error here — the two passes simply queue.
+// The only failure this returns is the lock FILE not opening (a missing
+// envDir, permissions), and that is what SyncUnits' caller prints and
+// returns without touching anything for.
func lockConfigSync(path string) (unlock func(), err error) {
f, err := os.OpenFile(path+".lock", os.O_CREATE|os.O_RDWR, 0o644)
if err != nil {