merge: opt-in exec config delivery — the app-supervised shape (issue #18, platform half)
The third backing shape gets config: a TCP-addr exec route whose app sets exec_delivery=file in its default bundle has its merged config materialized to /data/<host>.config.env, with a header saying carlos wrote it and never bounces it — the app's own supervisor owns restarts, structurally (the bounce closure is nil for this shape, not flag-checked). Opt-in lives in the default bundle only; LoadMerged copies only Vars, so a named environment is structurally incapable of carrying it. Opt-out leaves the file — deleting it would change a live app's next respawn env from a flag flip carlos doesn't control — but names it every pass, because it holds decrypted secrets and nothing will ever update it again. A transient bundle failure logs and leaves everything alone, and can never be mistaken for opt-out: the two notices are mutually exclusive by construction and pinned by mutation-verified tests. Two Opus review rounds; the fix round's own two gaps (an unpinned guard and unpinned header idempotence) closed with mutation-caught assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
7 files changed,
+1363
−20
appconfig_cli.go+72 −4e2e_test.go+105 −0internal/appconfig/appconfig.go+39 −0internal/appconfig/store.go+49 −0internal/appconfig/store_test.go+89 −0internal/appconfig/syncexec.go+245 −15internal/appconfig/syncexec_test.go+764 −1
diff --git a/appconfig_cli.go b/appconfig_cli.go| index a00157af2..1a4528ae1 100644 |
| --- a/appconfig_cli.go |
| +++ b/appconfig_cli.go |
| @@ -21,15 +21,25 @@ import ( |
| const envUsage = `usage: carlos env <command> |
| commands: |
| - set -app <a> [-environment <name>] KEY=value [KEY2=value2 ...] set one or more plain config vars |
| - unset -app <a> [-environment <name>] KEY [KEY2 ...] remove config vars |
| - list -app <a> [-environment <name>] list an app's config vars |
| - environments -app <a> list an app's named environment bundles |
| + set -app <a> [-environment <name>] KEY=value [KEY2=value2 ...] set one or more plain config vars |
| + unset -app <a> [-environment <name>] KEY [KEY2 ...] remove config vars |
| + list -app <a> [-environment <name>] list an app's config vars |
| + environments -app <a> list an app's named environment bundles |
| + exec-delivery -app <a> file|off opt an app's TCP-addr exec routes into config delivery (issue #18) |
| -environment names a bundle at apps/<acct>/<app>/environments/<name>.json, |
| layered on top of the default bundle (config.json) at materialization time — |
| see LoadMerged. Omit -environment for the default bundle itself. The bundle |
| is created implicitly on its first "set", same as config.json today. |
| + |
| +exec-delivery is APP-LEVEL ONLY (never -environment — see runEnvExecDelivery): |
| +it sets/clears "exec_delivery" in the default bundle alone. "file" makes |
| +SyncExec write /data/<host>.config.env for the app's TCP-addr exec routes |
| +(routes carlos proxies to but does not spawn — e.g. titogo's router fleet) |
| +exactly as it already does for socket-exec routes, but those routes are |
| +NEVER bounced: carlos does not own the process, so the app's own supervisor |
| +owns the restart. "off" clears the field; a plain "env list" mentions it |
| +when set. |
| ` |
| // RunEnv is the `carlos env` verb. |
| @@ -47,6 +57,8 @@ func RunEnv(args []string) { |
| runEnvList(args[1:]) |
| case "environments": |
| runEnvEnvironments(args[1:]) |
| + case "exec-delivery": |
| + runEnvExecDelivery(args[1:]) |
| default: |
| fmt.Fprint(os.Stderr, envUsage) |
| os.Exit(1) |
| @@ -213,6 +225,14 @@ func runEnvList(args []string) { |
| log.Fatal(err) |
| } |
| fmt.Printf("%s (%s):\n", *app, bundleLabel(*environment)) |
| + // exec_delivery is app-level only (read from the default bundle |
| + // alone — see appconfig.Manifest.ExecDelivery) and never printed for |
| + // a named environment's listing, matching where it can ever actually |
| + // be SET (runEnvExecDelivery refuses -environment before it reaches |
| + // appconfig at all). |
| + if *environment == "" && m.ExecDelivery != "" { |
| + fmt.Printf("exec-delivery: %s\n", m.ExecDelivery) |
| + } |
| var keys []string |
| for k, v := range m.Vars { |
| if v.Kind == "env" { |
| @@ -228,6 +248,54 @@ func runEnvList(args []string) { |
| w.Flush() |
| } |
| +// runEnvExecDelivery is `carlos env exec-delivery` — issue #18's opt-in |
| +// verb, deliberately NOT `env set -app <a> EXEC_DELIVERY=...`: the field |
| +// is not a config var (it's never rendered into an EnvironmentFile, never |
| +// listed alongside KEY/VALUE pairs), it's a manifest-level flag with its |
| +// own validation and its own restriction to the default bundle, so it |
| +// gets its own verb surface instead of overloading env set's KEY=value |
| +// parsing with a special-cased key name. |
| +func runEnvExecDelivery(args []string) { |
| + fs := flag.NewFlagSet("env exec-delivery", flag.ExitOnError) |
| + app := fs.String("app", "", "app name") |
| + // -environment is accepted (matching every other env subcommand's |
| + // flag set) purely so a mistaken attempt gets a clear, specific |
| + // refusal here rather than a generic "unknown flag" — see the check |
| + // below. There is no environment-scoped variant to route it to: |
| + // appconfig.SetExecDelivery has no environment parameter at all. |
| + environment := fs.String("environment", "", "refused: exec_delivery is app-level only (default bundle), never a named environment") |
| + acctOf := accountFlags(fs) |
| + fs.Parse(args) |
| + if *app == "" || fs.NArg() != 1 { |
| + log.Fatal("usage: carlos env exec-delivery -app <a> file|off") |
| + } |
| + if *environment != "" { |
| + log.Fatal(`carlos env exec-delivery: -environment is refused — exec_delivery is APP-LEVEL ONLY, read from the ` + |
| + "default bundle (config.json) alone. LoadMerged never carries this field from a named environment's " + |
| + "manifest into anything the sync path reads — it builds its merged result from Vars alone — so it is not " + |
| + "merely ignored there, it is structurally impossible for it to take effect there at all, per the issue #18 " + |
| + "design comment's \"config-that-lies\" rationale: the reader is the app's own supervisor, one program per " + |
| + "app, not per-environment. Omit -environment.") |
| + } |
| + var value string |
| + switch fs.Arg(0) { |
| + case appconfig.ExecDeliveryFile: |
| + value = appconfig.ExecDeliveryFile |
| + case "off": |
| + value = "" |
| + default: |
| + log.Fatalf("carlos env exec-delivery: %q is not a valid value — want %q or \"off\"", fs.Arg(0), appconfig.ExecDeliveryFile) |
| + } |
| + if err := appconfig.SetExecDelivery(context.Background(), openStore(), acctOf(), *app, value); err != nil { |
| + log.Fatal(err) |
| + } |
| + if value == "" { |
| + fmt.Printf("exec-delivery off for %s\n", *app) |
| + } else { |
| + fmt.Printf("exec-delivery %s for %s\n", value, *app) |
| + } |
| +} |
| + |
| func runEnvEnvironments(args []string) { |
| fs := flag.NewFlagSet("env environments", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
diff --git a/e2e_test.go b/e2e_test.go| index c49e0670f..ec7c817f9 100644 |
| --- a/e2e_test.go |
| +++ b/e2e_test.go |
| @@ -1606,6 +1606,111 @@ func TestEnvironmentBundlesCLI(t *testing.T) { |
| } |
| } |
| +// TestExecDeliveryCLI exercises `carlos env exec-delivery` (issue #18): |
| +// the app-level opt-in that lets SyncExec materialize |
| +// /data/<host>.config.env for TCP-addr exec routes — round-trip, unknown |
| +// value refusal, and named-environment placement refusal, following |
| +// TestEnvCLI's build-a-real-binary pattern. |
| +func TestExecDeliveryCLI(t *testing.T) { |
| + goBin, err := exec.LookPath("go") |
| + if err != nil { |
| + t.Skip("go toolchain not found") |
| + } |
| + tmp := t.TempDir() |
| + carlosBin := filepath.Join(tmp, "carlos") |
| + build := exec.Command(goBin, "build", "-o", carlosBin, ".") |
| + build.Env = os.Environ() |
| + if out, err := build.CombinedOutput(); err != nil { |
| + t.Fatalf("build carlos: %v\n%s", err, out) |
| + } |
| + storeDir := filepath.Join(tmp, "store") |
| + env := append(os.Environ(), "CARLOS_DEPLOYMENT_DIR="+storeDir) |
| + run := func(args ...string) string { |
| + t.Helper() |
| + cmd := exec.Command(carlosBin, args...) |
| + cmd.Env = env |
| + out, err := cmd.CombinedOutput() |
| + if err != nil { |
| + t.Fatalf("carlos %s: %v\n%s", strings.Join(args, " "), err, out) |
| + } |
| + return string(out) |
| + } |
| + runFails := func(args ...string) string { |
| + t.Helper() |
| + cmd := exec.Command(carlosBin, args...) |
| + cmd.Env = env |
| + out, err := cmd.CombinedOutput() |
| + if err == nil { |
| + t.Fatalf("carlos %s: succeeded, want a refusal\n%s", strings.Join(args, " "), out) |
| + } |
| + return string(out) |
| + } |
| + |
| + // Round trip: off by default (not mentioned in a plain list), "file" |
| + // turns it on and a plain `env list` mentions it, "off" clears it |
| + // again and the mention disappears. |
| + run("env", "set", "-account", "bab", "-app", "titogo", "K=1") |
| + out := run("env", "list", "-account", "bab", "-app", "titogo") |
| + if strings.Contains(out, "exec-delivery") { |
| + t.Fatalf("env list before opt-in = %q, must not mention exec-delivery", out) |
| + } |
| + |
| + out = run("env", "exec-delivery", "-account", "bab", "-app", "titogo", "file") |
| + if !strings.Contains(out, "titogo") || !strings.Contains(out, "file") { |
| + t.Fatalf("env exec-delivery file = %q, want a confirmation naming the app and the value", out) |
| + } |
| + out = run("env", "list", "-account", "bab", "-app", "titogo") |
| + if !regexp.MustCompile(`(?m)^exec-delivery:\s*file$`).MatchString(out) { |
| + t.Fatalf("env list after opt-in = %q, want an exec-delivery: file line", out) |
| + } |
| + // The var set earlier survives — exec-delivery is a field write, not a |
| + // bundle replace. |
| + if !regexp.MustCompile(`(?m)^K\s+1$`).MatchString(out) { |
| + t.Fatalf("env list after opt-in = %q, lost K=1 (exec-delivery must not clobber existing vars)", out) |
| + } |
| + |
| + out = run("env", "exec-delivery", "-account", "bab", "-app", "titogo", "off") |
| + if !strings.Contains(out, "off") { |
| + t.Fatalf("env exec-delivery off = %q, want a confirmation", out) |
| + } |
| + out = run("env", "list", "-account", "bab", "-app", "titogo") |
| + if strings.Contains(out, "exec-delivery") { |
| + t.Fatalf("env list after off = %q, must no longer mention exec-delivery", out) |
| + } |
| + |
| + // Unknown values are refused at set time, naming the bad value. |
| + msg := runFails("env", "exec-delivery", "-account", "bab", "-app", "titogo", "grpc") |
| + if !strings.Contains(msg, "grpc") { |
| + t.Fatalf("env exec-delivery grpc = %q, want a refusal naming the bad value", msg) |
| + } |
| + msg = runFails("env", "exec-delivery", "-account", "bab", "-app", "titogo", "on") // not the spelling |
| + if !strings.Contains(msg, "on") { |
| + t.Fatalf("env exec-delivery on = %q, want a refusal naming the bad value", msg) |
| + } |
| + |
| + // Named-environment placement is refused before it ever reaches the |
| + // store: -environment is app-level only for this verb. |
| + msg = runFails("env", "exec-delivery", "-account", "bab", "-app", "titogo", "-environment", "demo", "file") |
| + if !strings.Contains(strings.ToLower(msg), "app-level") { |
| + t.Fatalf("env exec-delivery -environment demo file = %q, want a refusal explaining it's app-level only", msg) |
| + } |
| + store := release.NewDirStore(storeDir) |
| + if _, _, err := store.Get(context.Background(), "apps/bab/titogo/environments/demo.json"); err == nil { |
| + t.Fatal("env exec-delivery -environment demo wrote a named environment bundle despite the refusal") |
| + } |
| + |
| + // Turning off against an app that was never configured at all is |
| + // refused, not a silent create — same idiom as `env unset` on a |
| + // nonexistent bundle. |
| + msg = runFails("env", "exec-delivery", "-account", "bab", "-app", "titogo-never-configured", "off") |
| + if !strings.Contains(msg, "nothing to turn off") { |
| + t.Fatalf("env exec-delivery off on a never-configured app = %q, want a refusal", msg) |
| + } |
| + if _, _, err := store.Get(context.Background(), "apps/bab/titogo-never-configured/config.json"); err == nil { |
| + t.Fatal("env exec-delivery off on a never-configured app created its config.json") |
| + } |
| +} |
| + |
| // TestEnvironmentMergeVisibleFromCLI is the merge-visible check the plan |
| // asks for: set the default bundle plus a named override through the CLI |
| // exactly as an operator would, then verify — via the library's LoadMerged, |
diff --git a/internal/appconfig/appconfig.go b/internal/appconfig/appconfig.go| index ca4b4e729..846396278 100644 |
| --- a/internal/appconfig/appconfig.go |
| +++ b/internal/appconfig/appconfig.go |
| @@ -22,6 +22,45 @@ type Manifest struct { |
| Schema int `json:"schema"` // 1 |
| App string `json:"app"` |
| Vars map[string]Var `json:"vars"` |
| + // ExecDelivery is the app's opt-in for issue #18's third SyncExec |
| + // eligibility case: a TCP-addr exec route (Backing == "", Socket == "", |
| + // Addr != "") whose process carlos does not spawn or own — an |
| + // app-supervised instance family (titogo's router fleet is the |
| + // motivating case). "" (the zero value, omitted from JSON) means not |
| + // opted in; ExecDeliveryFile ("file") means SyncExec writes this app's |
| + // routes' merged config to <dataDir>/<host>.config.env exactly as it |
| + // does for socket-exec routes — but NEVER bounces them, since carlos |
| + // does not own the process (see syncexec.go). |
| + // |
| + // Additive to schema 1 (schema is not bumped for this field), so any |
| + // other value is refused at SET time (SetExecDelivery, ValidExecDelivery) |
| + // and, since an older binary must still be able to parse a manifest a |
| + // newer one wrote, ignored — loudly, once per pass — at SYNC time |
| + // (see SyncExec's checkOptedIn) rather than causing a hard failure. |
| + // |
| + // APP-LEVEL ONLY: read from the default bundle (config.json) alone. |
| + // SetExecDelivery has no environment-scoped variant, unlike apply's |
| + // applyAt generalization — a named environment's manifest is never a |
| + // valid place for this field, and LoadMerged's merged result never |
| + // carries it forward. The reader is the app's own supervisor, one |
| + // program per app, not per-route or per-environment: per-route or |
| + // per-environment opt-in would let half a fleet read config the other |
| + // half silently ignores, the "config-that-lies" failure mode (issue |
| + // #18 design comment, 2026-08-03). |
| + ExecDelivery string `json:"exec_delivery,omitempty"` |
| +} |
| + |
| +// ExecDeliveryFile is the only non-empty value ValidExecDelivery accepts |
| +// today for Manifest.ExecDelivery — see its doc comment. |
| +const ExecDeliveryFile = "file" |
| + |
| +// ValidExecDelivery reports whether v is a value this binary understands |
| +// for Manifest.ExecDelivery: "" (not opted in) or ExecDeliveryFile. Shared |
| +// by the set-time refusal (SetExecDelivery, appconfig_cli.go's |
| +// runEnvExecDelivery) and SyncExec's sync-time ignore-with-a-log, so the |
| +// two can never define "valid" differently. |
| +func ValidExecDelivery(v string) bool { |
| + return v == "" || v == ExecDeliveryFile |
| } |
| // Var is one config var or secret. Kind selects which of Value/Envelope is |
diff --git a/internal/appconfig/store.go b/internal/appconfig/store.go| index 33883ec37..19d23a570 100644 |
| --- a/internal/appconfig/store.go |
| +++ b/internal/appconfig/store.go |
| @@ -214,3 +214,52 @@ func SetSecret(ctx context.Context, s release.Store, acct, app string, sealed ma |
| func Unset(ctx context.Context, s release.Store, acct, app string, keys []string) (int, error) { |
| return apply(ctx, s, acct, app, DefaultBundle, nil, keys) |
| } |
| + |
| +// SetExecDelivery writes (`file`) or clears (`""`) the default bundle's |
| +// exec_delivery field — issue #18's TCP-addr exec route opt-in (see |
| +// Manifest.ExecDelivery) — via the same CAS read-modify-write path |
| +// SetEnv/SetSecret/Unset use. value must be ValidExecDelivery; anything |
| +// else is refused here, before it ever reaches the bucket, mirroring |
| +// applyAt's var-name/var-value validation. |
| +// |
| +// APP-LEVEL ONLY, structurally: unlike apply's applyAt generalization, |
| +// there is no environment-scoped variant of this function — it always |
| +// writes Key(acct, app), never EnvironmentKey(acct, app, name). A named |
| +// environment's manifest is never a valid place for this field (Manifest. |
| +// ExecDelivery's doc comment has the "config-that-lies" rationale); the |
| +// CLI's -environment flag is refused before it can reach here at all |
| +// (appconfig_cli.go's runEnvExecDelivery). |
| +// |
| +// Turning the field off (value == "") against an app with no default |
| +// bundle at all is refused, not silently turned into a create — the same |
| +// "unset-only against a nonexistent object" refusal applyAt documents, for |
| +// the same reason: minting an empty bundle just to hold an absent flag |
| +// would satisfy `carlos route -environment`'s existence rail and Sync's |
| +// "configured, then fully unset" state for nothing the operator asked for. |
| +func SetExecDelivery(ctx context.Context, s release.Store, acct, app, value string) error { |
| + if !release.ValidAccount(acct) { |
| + return fmt.Errorf("config: invalid account %q", acct) |
| + } |
| + if !release.ValidApp(app) { |
| + return fmt.Errorf("config: invalid app %q", app) |
| + } |
| + if !ValidExecDelivery(value) { |
| + return fmt.Errorf("config: invalid exec_delivery %q — want %q or \"\" (off)", value, ExecDeliveryFile) |
| + } |
| + key := Key(acct, app) |
| + m, etag, found, err := getForWriteAt(ctx, s, key, app) |
| + if err != nil { |
| + return err |
| + } |
| + if !found && value == "" { |
| + return fmt.Errorf("config: %s has no default bundle — nothing to turn off", app) |
| + } |
| + m.ExecDelivery = value |
| + m.Schema, m.App = 1, app |
| + raw, err := json.MarshalIndent(m, "", " ") |
| + if err != nil { |
| + return err |
| + } |
| + _, err = s.Put(ctx, key, raw, etag) |
| + return err |
| +} |
diff --git a/internal/appconfig/store_test.go b/internal/appconfig/store_test.go| index 186af2492..629911dc1 100644 |
| --- a/internal/appconfig/store_test.go |
| +++ b/internal/appconfig/store_test.go |
| @@ -195,6 +195,92 @@ func TestSetEnvRejectsUnrepresentableValue(t *testing.T) { |
| } |
| } |
| +func TestSetExecDeliveryRoundTrips(t *testing.T) { |
| + s := release.NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"K": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDelivery(file): %v", err) |
| + } |
| + m, err := Get(ctx, s, "bab", "titogo") |
| + if err != nil { |
| + t.Fatalf("Get: %v", err) |
| + } |
| + if m.ExecDelivery != ExecDeliveryFile { |
| + t.Fatalf("ExecDelivery = %q, want %q", m.ExecDelivery, ExecDeliveryFile) |
| + } |
| + // The var set earlier survives — SetExecDelivery is a field write, not |
| + // a bundle replace. |
| + if m.Vars["K"].Value != "1" { |
| + t.Fatalf("Vars[K] = %+v, want value 1 (SetExecDelivery must not clobber existing vars)", m.Vars["K"]) |
| + } |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ""); err != nil { |
| + t.Fatalf("SetExecDelivery(off): %v", err) |
| + } |
| + m, err = Get(ctx, s, "bab", "titogo") |
| + if err != nil { |
| + t.Fatalf("Get: %v", err) |
| + } |
| + if m.ExecDelivery != "" { |
| + t.Fatalf("ExecDelivery after off = %q, want empty", m.ExecDelivery) |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryRejectsUnknownValue(t *testing.T) { |
| + s := release.NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"K": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + for _, bad := range []string{"grpc", "FILE", "true", "yes"} { |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", bad); err == nil { |
| + t.Fatalf("SetExecDelivery(%q) succeeded, want a refusal", bad) |
| + } |
| + } |
| + // The refusal must not have touched the bucket object at all. |
| + m, err := Get(ctx, s, "bab", "titogo") |
| + if err != nil { |
| + t.Fatalf("Get: %v", err) |
| + } |
| + if m.ExecDelivery != "" { |
| + t.Fatalf("ExecDelivery = %q after a refused set, want untouched (empty)", m.ExecDelivery) |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryOffOnNeverConfiguredAppIsRefused(t *testing.T) { |
| + // Mirrors TestUnsetOnNeverConfiguredAppErrorsAndWritesNothing: turning |
| + // the field off must not mint an empty default bundle just to hold an |
| + // absent flag. |
| + s := release.NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ""); err == nil { |
| + t.Fatal("SetExecDelivery(off) on a never-configured app succeeded, want a refusal") |
| + } |
| + if _, found, gerr := getIfExists(ctx, s, "bab", "titogo"); gerr != nil || found { |
| + t.Fatalf("SetExecDelivery(off) created a manifest object (found = %v, err = %v)", found, gerr) |
| + } |
| +} |
| + |
| +func TestSetExecDeliveryOnNeverConfiguredAppCreatesTheBundle(t *testing.T) { |
| + // The "file" direction DOES create a bundle on first use — same as |
| + // SetEnv/SetSecret already do — since "file" is genuine content to |
| + // store, not an unset-only call against nothing. |
| + s := release.NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDelivery(file): %v", err) |
| + } |
| + m, err := Get(ctx, s, "bab", "titogo") |
| + if err != nil { |
| + t.Fatalf("Get: %v", err) |
| + } |
| + if m.ExecDelivery != ExecDeliveryFile { |
| + t.Fatalf("ExecDelivery = %q, want %q", m.ExecDelivery, ExecDeliveryFile) |
| + } |
| +} |
| + |
| func TestNewerSchemaIsRefusedNotDowngraded(t *testing.T) { |
| // json.Unmarshal drops fields this binary doesn't know; acting on a |
| // newer manifest would silently discard them — and a write-back would |
| @@ -211,6 +297,9 @@ func TestNewerSchemaIsRefusedNotDowngraded(t *testing.T) { |
| if err := SetEnv(ctx, s, "bab", "keymail", map[string]string{"A": "1"}); err == nil { |
| t.Fatal("SetEnv wrote over a schema-2 manifest, want a refusal") |
| } |
| + if err := SetExecDelivery(ctx, s, "bab", "keymail", ExecDeliveryFile); err == nil { |
| + t.Fatal("SetExecDelivery wrote over a schema-2 manifest, want a refusal") |
| + } |
| got, _, err := s.Get(ctx, Key("bab", "keymail")) |
| if err != nil { |
| t.Fatalf("Get raw: %v", err) |
diff --git a/internal/appconfig/syncexec.go b/internal/appconfig/syncexec.go| index b6040135c..fe81811db 100644 |
| --- a/internal/appconfig/syncexec.go |
| +++ b/internal/appconfig/syncexec.go |
| @@ -8,6 +8,18 @@ |
| // hand-staged-file migration hazard by construction. See spec |
| // 2026-08-02-exec-config-delivery-design.md. |
| // |
| +// A third eligibility case (issue #18): a TCP-addr exec route (Backing == |
| +// "", Socket == "", Addr != "") whose app has opted in via its default |
| +// bundle's additive "exec_delivery": "file" manifest field |
| +// (Manifest.ExecDelivery) gets the identical file, materialized with the |
| +// same layering guarantees, environments-awareness, reap discipline, and |
| +// mass-removal/path-traversal guards as a socket-exec route — but is |
| +// NEVER bounced (see syncExecHost and execAddrCandidate): carlos routes |
| +// to these processes without spawning or owning them, so the restart half |
| +// belongs to the app's own supervisor. An un-opted TCP-addr route is |
| +// skipped exactly as before this case existed — see execAddrCandidate and |
| +// SyncExec's checkOptedIn closure. |
| +// |
| // Resolution mirrors sync.go's exactly — the group is (Account, App, |
| // Environment) and a route's bound environment is an overlay on the app's |
| // default bundle (environments.go, pipelines design §4a). Only the file |
| @@ -89,8 +101,87 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv |
| defaults[ak] = d |
| return d |
| } |
| + // optedInRes is checkOptedIn's cached verdict. known is false ONLY for |
| + // a fetch error: v is a placeholder in that case, never a |
| + // determination — see checkOptedIn's def.err branch for why that |
| + // distinction matters to the stale-file notice below. |
| + type optedInRes struct { |
| + v bool |
| + known bool |
| + } |
| + // optedIn caches the app-level exec_delivery decision so a box with |
| + // many TCP-addr routes on one app doesn't cost more than one manifest |
| + // read for it (loadDefault's own cache already dedupes the fetch; this |
| + // dedupes the value-interpretation, including the two logs below, |
| + // which must fire at most once per (acct, app) per pass, not once per |
| + // route). |
| + optedIn := map[appKey]optedInRes{} |
| + checkOptedIn := func(acct, app string) optedInRes { |
| + ak := appKey{acct, app} |
| + if c, done := optedIn[ak]; done { |
| + return c |
| + } |
| + def := loadDefault(acct, app) |
| + var c optedInRes |
| + switch { |
| + case def.err != nil: |
| + // NOT a determination: this app may well be opted in and this |
| + // box just failed to see it this pass — indistinguishable here |
| + // from any other route's ordinary fetch-failure blip. Silence |
| + // would mean an opted-in app's delivery can stall with zero |
| + // trace in the log, on the exact path this feature exists to |
| + // light up, so say so loudly — the same "unresolved, left |
| + // as-is" contract the resolution loop below already gives a |
| + // bound-group fetch failure. |
| + // |
| + // It is safe to leave any existing file exactly as it is |
| + // regardless of how long this persists: NOT because a fetch |
| + // failure and "never opted in" are indistinguishable (they |
| + // aren't, that's the whole point of this branch existing |
| + // separately from the ones below) but because reapConfigEnv |
| + // keys on ROUTE EXISTENCE, never on scope (its live map is |
| + // built from every route's Host, full stop — see its own doc |
| + // comment). A route that drops out of scope for one pass, for |
| + // ANY reason, is simply not visited by the resolution loop, |
| + // the materialization loop, or reap; its file is untouched by |
| + // construction, not because this function decided to spare |
| + // it. Next pass retries from scratch. |
| + fmt.Printf(" %s/%s: exec_delivery unresolved (config fetch failed), leaving this app's TCP-addr route file(s) as-is: %v\n", acct, app, def.err) |
| + c = optedInRes{v: false, known: false} |
| + case !def.found: |
| + c = optedInRes{v: false, known: true} |
| + case def.m.ExecDelivery == ExecDeliveryFile: |
| + c = optedInRes{v: true, known: true} |
| + case def.m.ExecDelivery == "": |
| + c = optedInRes{v: false, known: true} |
| + default: |
| + // A manifest a NEWER carlos wrote with a value THIS binary |
| + // doesn't understand must not brick this box's sync (additive |
| + // discipline, CLAUDE.md) — ignore it, loudly, once, and treat |
| + // the app as not opted in rather than refusing the whole pass. |
| + fmt.Printf(" %s/%s: config: exec_delivery %q in the default bundle is not a value this binary understands — ignoring it (not opted in); upgrade carlos if this is expected\n", acct, app, def.m.ExecDelivery) |
| + c = optedInRes{v: false, known: true} |
| + } |
| + optedIn[ak] = c |
| + return c |
| + } |
| + // inScope is the single door both loops below use to decide whether a |
| + // route is in scope this pass, and — for the addr shape — that it must |
| + // never be bounced. One function so the two loops structurally cannot |
| + // diverge on either question (see the "if they ever diverge" comment |
| + // on the second loop's map lookup, which depends on the same key |
| + // having been computed the same way here). |
| + inScope := func(rt registry.Route) (ok, addrShape bool) { |
| + if execSyncable(rt) { |
| + return true, false |
| + } |
| + if execAddrCandidate(rt) && checkOptedIn(release.RouteAccount(rt), rt.App).v { |
| + return true, true |
| + } |
| + return false, false |
| + } |
| for _, rt := range routes { |
| - if !execSyncable(rt) { |
| + if in, _ := inScope(rt); !in { |
| continue |
| } |
| k := groupKey{release.RouteAccount(rt), rt.App, rt.Environment} |
| @@ -174,15 +265,16 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv |
| } |
| for _, rt := range routes { |
| - if !execSyncable(rt) { |
| + in, addrShape := inScope(rt) |
| + if !in { |
| continue |
| } |
| k := groupKey{release.RouteAccount(rt), rt.App, rt.Environment} |
| // Insist the group was really resolved rather than taking the map's |
| // zero value: a zero resolution reads as "absent", which on this |
| - // path REMOVES the file. The two loops filter on the same |
| - // execSyncable today, so this cannot fire — but if they ever |
| - // diverge, this turns a silent config deletion into a skip. |
| + // path REMOVES the file. The two loops filter through the same |
| + // inScope today, so this cannot fire — but if they ever diverge, |
| + // this turns a silent config deletion into a skip. |
| r, ok := resolved[k] |
| if !ok { |
| continue |
| @@ -208,7 +300,68 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv |
| // resolves through LoadMerged, which errors on a missing bundle |
| // rather than returning nothing). A bound group's key would be |
| // dead in every branch that reads this. |
| - syncExecHost(rt, r.m, Key(k.acct, k.app), priv, dataDir, bounce) |
| + // |
| + // addrShape routes get nil in bounce's place, not the real |
| + // closure with a runtime check skipping the call: carlos does not |
| + // own an app-supervised process, so there is structurally nothing |
| + // to hand syncExecHost that COULD invoke it for this route (issue |
| + // #18 design comment) — bounceLogged's existing nil-is-a-no-op |
| + // contract does the rest. Same shape for the header: addrShape |
| + // routes get execAddrConfigHeader, socket routes get "" (no |
| + // header at all) — see execAddrConfigHeader's doc comment for why |
| + // this must never apply to the socket shape. |
| + hostBounce := bounce |
| + header := "" |
| + if addrShape { |
| + hostBounce = nil |
| + header = execAddrConfigHeader |
| + } |
| + syncExecHost(rt, r.m, Key(k.acct, k.app), priv, dataDir, hostBounce, header) |
| + } |
| + // Stale-secrets notice (issue #18 design ruling, ratified 2026-08-03 |
| + // review): an addr-shaped route whose app is DEFINITIVELY not opted in |
| + // this pass (off, never configured, or an unrecognized exec_delivery |
| + // value — checkOptedIn's known == true, as opposed to a transient |
| + // fetch error) but still has a route row keeps any earlier |
| + // <host>.config.env EXACTLY as it is: it was never added to |
| + // inScope/resolved above, so neither the resolution loop nor the |
| + // materialization loop touched it, and reap (below) never removes a |
| + // file whose route still has a row. |
| + // |
| + // RULING: do not auto-delete it. Removing the file would change what |
| + // the app's OWN supervisor layers in on its next respawn — a live |
| + // environment change caused by a flag flip, taking effect at a moment |
| + // carlos does not control and cannot announce, with no bounce (there |
| + // never is one for this shape) to make it observable. That is worse |
| + // than a stale file. It is also consistent with every other |
| + // ineligibility transition in this file: a route losing |
| + // Status: "ready" or gaining Platform: true doesn't get its file |
| + // deleted either, only stops being written to. |
| + // |
| + // BUT staying silent about it is wrong, more than for those other |
| + // transitions, because this file holds DECRYPTED secrets, and once an |
| + // app opts out nothing will EVER update it again: rotate a secret in |
| + // the bucket afterward and this file keeps serving yesterday's |
| + // plaintext, indefinitely, with carlos reporting nothing. "Off" reads |
| + // to an operator as "stop delivering config"; what it actually does |
| + // is "freeze whatever was last delivered, forever". Name the file and |
| + // the manual remedy, once per pass, so an operator doesn't mistake |
| + // the one for the other — the remedy has to be manual (delete it once |
| + // 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) |
| + 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) |
| } |
| // An empty route list is never authority to delete anything — |
| // registry.Open silently creates an empty database on a wrong path or |
| @@ -220,18 +373,78 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv |
| return nil |
| } |
| -// execSyncable: lifecycle-managed instances the ACTIVATOR spawns — and |
| -// therefore whose environment this sweep can actually deliver. Mirrors |
| -// the unit path's syncable() exclusions, then narrows to exec routes |
| -// with a socket upstream: addr-upstream routes are proxy-only (the |
| -// process behind them is externally managed) and unit routes belong to |
| -// the root adopt path. |
| -func execSyncable(rt registry.Route) bool { |
| +// execRouteBase is the lifecycle-managed-instance shape both exec route |
| +// kinds share: the unit path's syncable() exclusions, narrowed to |
| +// non-unit backing (unit routes belong to the root adopt path). Neither |
| +// SyncExec eligibility case on its own — see execSyncable and |
| +// execAddrCandidate, which each narrow it by upstream shape. |
| +func execRouteBase(rt registry.Route) bool { |
| return rt.Kind == "instance" && rt.App != "" && !rt.Platform && |
| (rt.Status == "" || rt.Status == "ready") && |
| - rt.Backing != registry.BackingUnit && rt.Socket != "" |
| + rt.Backing != registry.BackingUnit |
| +} |
| + |
| +// execSyncable: lifecycle-managed instances the ACTIVATOR spawns — and |
| +// therefore whose environment this sweep can actually deliver and whose |
| +// process it can bounce. Narrows execRouteBase to a socket upstream; |
| +// unconditionally in scope, opt-in or not (this is the original, PR #16 |
| +// shape). |
| +func execSyncable(rt registry.Route) bool { |
| + return execRouteBase(rt) && rt.Socket != "" |
| } |
| +// execAddrCandidate: the TCP-addr exec shape (issue #18) — an |
| +// app-supervised instance family carlos routes to but neither spawns nor |
| +// owns the process of (titogo's router fleet is the motivating case: |
| +// titogo-router + controld own the instance processes, carlos just |
| +// proxies to 127.0.0.1:<port>). Narrows execRouteBase to an addr upstream |
| +// with no socket. |
| +// |
| +// This predicate is the ROUTE-SHAPE half only — it says nothing about |
| +// whether the route is actually in scope this pass. Unlike execSyncable, |
| +// that also requires the app's default bundle to carry "exec_delivery": |
| +// "file" (Manifest.ExecDelivery): the reader of the file this shape |
| +// produces is the app's OWN supervisor, not carlos, so writing it for an |
| +// app that never reads it would be dead weight at best and, if the app's |
| +// supervisor treats the file's mere presence or mtime as a restart |
| +// signal, an unwanted bounce at worst. See SyncExec's checkOptedIn/ |
| +// inScope closures, which gate this predicate on that opt-in. |
| +func execAddrCandidate(rt registry.Route) bool { |
| + return execRouteBase(rt) && rt.Socket == "" && rt.Addr != "" |
| +} |
| + |
| +// execAddrConfigHeader is prepended to a TCP-addr (issue #18) route's |
| +// <host>.config.env — and ONLY that shape. RenderEnv itself stays |
| +// header-free, deliberately: it is shared by sync.go's unit path AND this |
| +// file's socket-exec path, and either one growing a header would change |
| +// the byte content of every EXISTING file of that shape on upgrade — |
| +// sync.go's own comment on lostKeys already weighs a "# managed by |
| +// carlos" marker for the unit path and rejects it for exactly this |
| +// churn. The socket-exec shape has the same installed-base problem: every |
| +// currently-running instance's file would look "changed" on the pass |
| +// after this ships, and — unlike the addr shape — a socket route DOES |
| +// bounce on a content change, so that churn would restart every |
| +// socket-exec instance on the fleet once, for a comment. The addr shape |
| +// has no installed base at all (this is its first pass ever) and never |
| +// bounces, so the header costs it nothing. |
| +// |
| +// Format: "#"-prefixed and blank lines are the universal EnvironmentFile |
| +// comment convention — systemd's own EnvironmentFile= parser skips them, |
| +// and this repo's own guard for the unit path, lostKeys (sync.go), |
| +// already relies on exactly that tolerance when parsing a hand-staged |
| +// file. The issue #18 design comment specifies titogo's controld layers |
| +// this file the same way systemd's EnvironmentFile= does (default bundle |
| +// under the app-supervisor's own env), so a plain KEY=value-plus-#-comments |
| +// reader is the assumed parse contract on that side too; this repo has no |
| +// titogo checkout to verify it against directly, so treat this as an |
| +// assumption to confirm against titogo's actual parser before or during |
| +// the app-half PR, not a verified fact. |
| +const execAddrConfigHeader = `# Written by carlos (SyncExec, issue #18). Carlos may overwrite or remove |
| +# this file on any future sync pass, but NEVER restarts this process because |
| +# of it — carlos does not own it. This app's own supervisor owns restarts |
| +# and must notice and act on changes here on its own schedule. |
| +` |
| + |
| // syncExecHost converges one route's config file. m == nil means the |
| // manifest object is absent from the bucket: remove the file (sync owns |
| // the namespace; a file with no manifest is a leftover, and leaving it |
| @@ -241,7 +454,21 @@ func execSyncable(rt registry.Route) bool { |
| // key came back absent) and effect (which file got removed) land |
| // together on one line, instead of requiring a reader to cross-reference |
| // the fetch-side log. |
| -func syncExecHost(rt registry.Route, m *Manifest, key string, priv *ecdh.PrivateKey, dataDir string, bounce func(host string) error) { |
| +// |
| +// bounce may be nil: bounceLogged treats that as "nothing to call", which |
| +// is how a TCP-addr (issue #18) route's file gets converged or removed |
| +// exactly like a socket-exec route's — same write/rename/remove path, |
| +// same logging — while never restarting anything, because carlos does |
| +// not own that process. The caller (SyncExec) passes nil for those routes |
| +// structurally, not a flag this function branches on. |
| +// |
| +// header is prepended to the rendered content when non-empty |
| +// (execAddrConfigHeader for the addr shape, "" for socket — see that |
| +// constant's doc comment for why the two must never be swapped or |
| +// unified) — folded into the SAME diff/write/rename path as the vars |
| +// themselves, so a header-only change (e.g. its text edited in a future |
| +// release) converges exactly like a var change would, once. |
| +func syncExecHost(rt registry.Route, m *Manifest, key string, priv *ecdh.PrivateKey, dataDir string, bounce func(host string) error, header string) { |
| if rt.Host == "" || rt.Host == "." || rt.Host == ".." || rt.Host != filepath.Base(rt.Host) { |
| fmt.Printf(" %q: not a bare hostname, refusing to derive a config file path from it\n", rt.Host) |
| return |
| @@ -279,6 +506,9 @@ func syncExecHost(rt registry.Route, m *Manifest, key string, priv *ecdh.Private |
| fmt.Printf(" %s: render config: %v\n", rt.Host, err) |
| return |
| } |
| + if header != "" { |
| + rendered = append([]byte(header), rendered...) |
| + } |
| if existed && bytes.Equal(cur, rendered) { |
| return // up to date — stay quiet on the periodic tick |
| } |
diff --git a/internal/appconfig/syncexec_test.go b/internal/appconfig/syncexec_test.go| index 1c863c076..a5f12f875 100644 |
| --- a/internal/appconfig/syncexec_test.go |
| +++ b/internal/appconfig/syncexec_test.go |
| @@ -1,11 +1,16 @@ |
| package appconfig |
| import ( |
| + "bytes" |
| "context" |
| + "errors" |
| + "io" |
| "os" |
| "path/filepath" |
| "slices" |
| + "strings" |
| "testing" |
| + "time" |
| "github.com/carlosframework/platform/internal/registry" |
| "github.com/carlosframework/platform/internal/release" |
| @@ -26,6 +31,52 @@ func execKit(t *testing.T) (ctx context.Context, s release.Store, reg *registry. |
| return |
| } |
| +// captureStdout runs fn with os.Stdout redirected to a pipe and returns |
| +// everything fn printed. SyncExec logs via fmt.Printf directly to |
| +// os.Stdout, matching every other pass in this package — this is the only |
| +// way to pin a log line's presence without restructuring every call site |
| +// to take a Writer, so it's used sparingly: only where a test's whole |
| +// point IS that a specific line got printed (a `fmt.Printf` deleted by a |
| +// future edit would otherwise go unnoticed by every other test, which |
| +// only assert on the filesystem). |
| +func captureStdout(t *testing.T, fn func()) string { |
| + t.Helper() |
| + r, w, err := os.Pipe() |
| + if err != nil { |
| + t.Fatalf("os.Pipe: %v", err) |
| + } |
| + old := os.Stdout |
| + os.Stdout = w |
| + defer func() { os.Stdout = old }() |
| + fn() |
| + if err := w.Close(); err != nil { |
| + t.Fatalf("close pipe writer: %v", err) |
| + } |
| + var buf bytes.Buffer |
| + if _, err := io.Copy(&buf, r); err != nil { |
| + t.Fatalf("read captured stdout: %v", err) |
| + } |
| + return buf.String() |
| +} |
| + |
| +// failingKeyStore wraps a release.Store and makes Get fail with a fixed |
| +// error for exactly one key — simulating a transient bucket blip against a |
| +// single app's default bundle while every other key (other apps' bundles, |
| +// including in the SAME pass) resolves normally, the shape a real S3 |
| +// SlowDown/503 against one object takes. |
| +type failingKeyStore struct { |
| + release.Store |
| + failKey string |
| + err error |
| +} |
| + |
| +func (f *failingKeyStore) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + if key == f.failKey { |
| + return nil, "", f.err |
| + } |
| + return f.Store.Get(ctx, key) |
| +} |
| + |
| func TestSyncExecWritesConfigFileAndBouncesOnChange(t *testing.T) { |
| ctx, s, reg, dd, priv, _ := execKit(t) |
| pk, err := ParsePrivateKey(priv, "test") |
| @@ -479,6 +530,61 @@ func TestSyncExecReapsOnlyItsOwnNamespace(t *testing.T) { |
| } |
| } |
| +// TestSyncExecReapsOnlyItsOwnNamespaceWithAddrShapeInTheMix is the reap |
| +// test's issue #18 twin: an opted-in addr route's own file survives reap |
| +// (it has a route row, same as any in-scope route), and a stray |
| +// *.config.env with NO route row at all is reaped regardless of which |
| +// exec shape produced it in some earlier pass — reap keys purely on |
| +// (namespace suffix, route existence), never on shape or opt-in. |
| +func TestSyncExecReapsOnlyItsOwnNamespaceWithAddrShapeInTheMix(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, _ := ParsePrivateKey(priv, "test") |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"X": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDelivery: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "live.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:8444", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + files := map[string]bool{ // name -> should survive |
| + "live.go.tito.io.config.env": true, // opted-in addr route, has a route row |
| + "gone.go.tito.io.config.env": false, // no route row at all: reaped, whatever shape wrote it |
| + } |
| + for name := range files { |
| + if err := os.WriteFile(filepath.Join(dd, name), []byte("X=1\n"), 0o600); err != nil { |
| + t.Fatalf("stage %s: %v", name, err) |
| + } |
| + } |
| + var bounced []string |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(h string) error { |
| + bounced = append(bounced, h) |
| + return nil |
| + }); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + for name, keep := range files { |
| + _, err := os.Stat(filepath.Join(dd, name)) |
| + if keep && err != nil { |
| + t.Fatalf("%s: want kept, got %v", name, err) |
| + } |
| + if !keep && !os.IsNotExist(err) { |
| + t.Fatalf("%s: want reaped, stat err = %v", name, err) |
| + } |
| + } |
| + // The reap pass itself must never bounce — reap's log line says "no |
| + // route on this box", never "bounce"; and the addr route's own |
| + // convergence (a fresh, unchanged-content write above) must not |
| + // bounce either. |
| + if len(bounced) != 0 { |
| + t.Fatalf("bounced = %v, want none", bounced) |
| + } |
| +} |
| + |
| func TestSyncExecEmptyRegistryRemovesNothing(t *testing.T) { |
| ctx, s, reg, dd, priv, _ := execKit(t) |
| pk, _ := ParsePrivateKey(priv, "test") |
| @@ -499,7 +605,7 @@ func TestSyncExecRefusesNonBareHostname(t *testing.T) { |
| pk, _ := ParsePrivateKey(priv, "test") |
| m := &Manifest{Schema: 1, App: "eleven", Vars: map[string]Var{"A": {Kind: "env", Value: "1"}}} |
| for _, host := range []string{"../escape", "a/b", "", ".", ".."} { |
| - syncExecHost(registry.Route{Host: host}, m, "apps/bab/eleven/config.json", pk, dd, func(string) error { return nil }) |
| + syncExecHost(registry.Route{Host: host}, m, "apps/bab/eleven/config.json", pk, dd, func(string) error { return nil }, "") |
| } |
| ents, err := os.ReadDir(dd) |
| if err != nil { |
| @@ -510,6 +616,663 @@ func TestSyncExecRefusesNonBareHostname(t *testing.T) { |
| } |
| } |
| +// TestSyncExecOptedInAddrRouteMaterializesFile is issue #18's core case: |
| +// a TCP-addr route (Socket == "", Addr != "") whose app opted in via |
| +// ExecDeliveryFile in its default bundle gets the identical |
| +// <host>.config.env a socket-exec route would — same rendering, same |
| +// layering guarantees — even though carlos neither spawned nor owns the |
| +// process behind it. |
| +func TestSyncExecOptedInAddrRouteMaterializesFile(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"TITOGO_DEMO_BANNER": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDelivery: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "demo.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:8444", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + got, err := os.ReadFile(filepath.Join(dd, "demo.go.tito.io.config.env")) |
| + if err != nil { |
| + t.Fatalf("opted-in TCP-addr route got no config file: %v", err) |
| + } |
| + want := execAddrConfigHeader + "TITOGO_DEMO_BANNER=1\n" |
| + if string(got) != want { |
| + t.Fatalf("config env = %q, want %q (issue #18's required header, then the vars)", got, want) |
| + } |
| + |
| + // Idempotence must survive the header (2026-08-03 re-review, IMPORTANT |
| + // 7): the up-to-date comparison has to include the header bytes, or an |
| + // unchanged pass rewrites this secrets-bearing root-owned file every |
| + // tick forever. The socket shape has this pin; before this assertion, |
| + // moving the header prepend after the comparison left the whole suite |
| + // green. mtime is the observable: a second pass over unchanged config |
| + // must not touch the file. |
| + path := filepath.Join(dd, "demo.go.tito.io.config.env") |
| + before, err := os.Stat(path) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + time.Sleep(10 * time.Millisecond) // ensure a rewrite would move mtime |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec (second pass): %v", err) |
| + } |
| + after, err := os.Stat(path) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if !after.ModTime().Equal(before.ModTime()) { |
| + t.Fatalf("an unchanged second pass rewrote the config file (mtime %v -> %v)", before.ModTime(), after.ModTime()) |
| + } |
| +} |
| + |
| +// TestSyncExecUnoptedAddrRouteIsSkipped pins the un-opted case: the exact |
| +// same TCP-addr route shape, but its app never set exec_delivery, is |
| +// skipped exactly as before issue #18's third case existed — no file, no |
| +// error, nothing to converge or remove. |
| +func TestSyncExecUnoptedAddrRouteIsSkipped(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"TITOGO_DEMO_BANNER": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + // Deliberately no SetExecDelivery call. |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "demo.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:8444", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + if _, err := os.Stat(filepath.Join(dd, "demo.go.tito.io.config.env")); !os.IsNotExist(err) { |
| + t.Fatalf("un-opted TCP-addr route got a config file, stat err = %v", err) |
| + } |
| +} |
| + |
| +// TestSyncExecUnrecognizedExecDeliveryValueIgnoredNotFatal covers additive |
| +// discipline (CLAUDE.md): a manifest a NEWER carlos wrote, carrying an |
| +// exec_delivery value this binary doesn't understand, must not brick this |
| +// box's sync — the whole pass must still succeed, the unknown value must |
| +// be treated as "not opted in" (not a fatal error, not silently treated |
| +// as "file"), and OTHER apps' routes in the same pass must converge |
| +// normally. |
| +func TestSyncExecUnrecognizedExecDeliveryValueIgnoredNotFatal(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + // Written directly, bypassing SetExecDelivery's set-time refusal — the |
| + // shape of a manifest a newer binary produced. |
| + raw := []byte(`{"schema":1,"app":"titogo","vars":{"K":{"kind":"env","value":"1"}},"exec_delivery":"grpc"}`) |
| + if _, err := s.Put(ctx, Key("bab", "titogo"), raw, ""); err != nil { |
| + t.Fatalf("Put: %v", err) |
| + } |
| + // A second, ordinary socket-exec app in the same pass, to prove the |
| + // unknown value doesn't abort or otherwise disturb the rest of the |
| + // sweep. |
| + if err := SetEnv(ctx, s, "bab", "eleven", map[string]string{"LOG_LEVEL": "debug"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + routes := []registry.Route{ |
| + {Host: "demo.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", Status: "ready", Addr: "127.0.0.1:8444"}, |
| + {Host: "inst.example.com", Kind: "instance", App: "eleven", Account: "bab", Status: "ready", Socket: "/run/carlos/inst.sock"}, |
| + } |
| + for _, rt := range routes { |
| + if err := reg.Upsert(rt); err != nil { |
| + t.Fatalf("Upsert %s: %v", rt.Host, err) |
| + } |
| + } |
| + var syncErr error |
| + out := captureStdout(t, func() { |
| + syncErr = SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }) |
| + }) |
| + if syncErr != nil { |
| + t.Fatalf("SyncExec: %v", syncErr) |
| + } |
| + if _, err := os.Stat(filepath.Join(dd, "demo.go.tito.io.config.env")); !os.IsNotExist(err) { |
| + t.Fatalf("unrecognized exec_delivery value was treated as opted-in, stat err = %v", err) |
| + } |
| + if _, err := os.Stat(filepath.Join(dd, "inst.example.com.config.env")); err != nil { |
| + t.Fatalf("eleven's ordinary socket route did not converge despite titogo's unrecognized value: %v", err) |
| + } |
| + // MINOR 5 (2026-08-03 review): the whole safety story for this case is |
| + // "ignored LOUDLY" — pin the loudness itself, not just the tolerance, |
| + // so a future edit that quietly dropped the fmt.Printf would fail a |
| + // test instead of only being noticed by someone reading a log by hand. |
| + if !strings.Contains(out, "titogo") || !strings.Contains(out, "grpc") { |
| + t.Fatalf("stdout = %q, want a line naming the app (titogo) and the unrecognized value (grpc)", out) |
| + } |
| +} |
| + |
| +// TestSyncExecOptedInAddrRouteFetchFailureLogsAndLeavesFileAsIs is |
| +// IMPORTANT 1 from the 2026-08-03 review: a transient default-bundle read |
| +// failure against an ALREADY-opted-in app must not go silent. Before this |
| +// fix, checkOptedIn folded def.err != nil into the same "not opted in, |
| +// say nothing" branch as a genuinely never-configured app — so an opted-in |
| +// app whose bucket blips became indistinguishable, in the log, from an |
| +// app that was never opted in at all, on the exact path this feature |
| +// exists to make observable. The safety property (the file survives |
| +// byte-identical) was already correct; only the silence was the bug. |
| +func TestSyncExecOptedInAddrRouteFetchFailureLogsAndLeavesFileAsIs(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"TITOGO_DEMO_BANNER": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDelivery: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "demo.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:8444", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + // First pass materializes normally — this is the file a transient |
| + // blip must leave exactly as it is. |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec (initial): %v", err) |
| + } |
| + path := filepath.Join(dd, "demo.go.tito.io.config.env") |
| + original, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("read initial config env: %v", err) |
| + } |
| + |
| + blipErr := errors.New("s3: transient 503 SlowDown") |
| + failing := &failingKeyStore{Store: s, failKey: Key("bab", "titogo"), err: blipErr} |
| + var bounced []string |
| + bounce := func(h string) error { bounced = append(bounced, h); return nil } |
| + var syncErr error |
| + out := captureStdout(t, func() { |
| + syncErr = SyncExec(ctx, reg, failing, pk, dd, bounce) |
| + }) |
| + if syncErr != nil { |
| + t.Fatalf("SyncExec (blip): %v", syncErr) |
| + } |
| + got, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("read config env after the blip: %v", err) |
| + } |
| + if string(got) != string(original) { |
| + t.Fatalf("config env changed across a transient fetch failure: got %q, want the original %q untouched", got, original) |
| + } |
| + if len(bounced) != 0 { |
| + t.Fatalf("bounced = %v, want none — a fetch failure is not a change", bounced) |
| + } |
| + // The fix: this must no longer be silent. Name the app and the |
| + // underlying error, the same "unresolved ... left as-is" contract the |
| + // socket path's resolution loop already gives an ordinary fetch |
| + // failure. |
| + if !strings.Contains(out, "titogo") { |
| + t.Fatalf("stdout = %q, want a line naming the app (titogo)", out) |
| + } |
| + if !strings.Contains(out, "SlowDown") { |
| + t.Fatalf("stdout = %q, want the underlying fetch error surfaced", out) |
| + } |
| + // The mutual exclusion the ruling depends on (2026-08-03 re-review, |
| + // IMPORTANT 6): a transient blip must print ONLY the unresolved line — |
| + // never the stale-secrets notice, whose remedy is "delete this file by |
| + // hand". Dropping !c.known from that notice's guard would turn every |
| + // bucket blip into an instruction to rm a live production file |
| + // mid-incident, and before this assertion the whole suite stayed green |
| + // under exactly that mutation. |
| + if strings.Contains(out, "delete it by hand") { |
| + t.Fatalf("stdout = %q — a transient fetch failure produced the stale-file removal advice", out) |
| + } |
| +} |
| + |
| +// TestSyncExecOptedInAddrRouteIsNeverBounced is the structural pin the |
| +// issue's design comment demands: carlos does not own an app-supervised |
| +// process, so an opted-in TCP-addr route's bounce callback must not even |
| +// be invoked — not "invoked and then skipped by a flag", genuinely never |
| +// called. The recorder below would append on any call at all; it must |
| +// stay empty across a pass that DOES change the route's file content |
| +// (proving the file write path ran) and a second pass where content is |
| +// unchanged (proving the still-quiet path also never bounces). |
| +func TestSyncExecOptedInAddrRouteIsNeverBounced(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"TITOGO_DEMO_BANNER": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDelivery: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "demo.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:8444", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + var bounced []string |
| + bounce := func(h string) error { |
| + bounced = append(bounced, h) |
| + return nil |
| + } |
| + if err := SyncExec(ctx, reg, s, pk, dd, bounce); err != nil { |
| + t.Fatalf("SyncExec (initial write): %v", err) |
| + } |
| + if len(bounced) != 0 { |
| + t.Fatalf("bounced = %v, want none — carlos does not own this process", bounced) |
| + } |
| + // Change the config so the file's content changes on the next pass — |
| + // the socket-exec equivalent of this WOULD bounce. |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"TITOGO_DEMO_BANNER": "2"}); err != nil { |
| + t.Fatalf("SetEnv (change): %v", err) |
| + } |
| + if err := SyncExec(ctx, reg, s, pk, dd, bounce); err != nil { |
| + t.Fatalf("SyncExec (changed content): %v", err) |
| + } |
| + if len(bounced) != 0 { |
| + t.Fatalf("bounced = %v after a content change, want none", bounced) |
| + } |
| + got, err := os.ReadFile(filepath.Join(dd, "demo.go.tito.io.config.env")) |
| + if err != nil { |
| + t.Fatalf("read config env: %v", err) |
| + } |
| + want := execAddrConfigHeader + "TITOGO_DEMO_BANNER=2\n" |
| + if string(got) != want { |
| + t.Fatalf("config env = %q, want %q — the changed content, header included — the write path must still run", got, want) |
| + } |
| +} |
| + |
| +// TestSyncExecSocketRouteStillBouncesAlongsideOptedInAddrRoute proves the |
| +// addr shape's no-bounce rule is scoped to addr routes ONLY: a socket-exec |
| +// route for a DIFFERENT app in the same pass must still bounce on change, |
| +// unaffected by the addr route sharing the sweep. |
| +func TestSyncExecSocketRouteStillBouncesAlongsideOptedInAddrRoute(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"A": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDelivery: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "eleven", map[string]string{"B": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + routes := []registry.Route{ |
| + {Host: "demo.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", Status: "ready", Addr: "127.0.0.1:8444"}, |
| + {Host: "inst.example.com", Kind: "instance", App: "eleven", Account: "bab", Status: "ready", Socket: "/run/carlos/inst.sock"}, |
| + } |
| + for _, rt := range routes { |
| + if err := reg.Upsert(rt); err != nil { |
| + t.Fatalf("Upsert %s: %v", rt.Host, err) |
| + } |
| + } |
| + var bounced []string |
| + bounce := func(h string) error { |
| + bounced = append(bounced, h) |
| + return nil |
| + } |
| + if err := SyncExec(ctx, reg, s, pk, dd, bounce); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + if !slices.Contains(bounced, "inst.example.com") { |
| + t.Fatalf("bounced = %v, want the socket-exec route bounced (unaffected by the addr route sharing the pass)", bounced) |
| + } |
| + if slices.Contains(bounced, "demo.go.tito.io") { |
| + t.Fatalf("bounced = %v, the opted-in addr route must never be bounced", bounced) |
| + } |
| + if len(bounced) != 1 { |
| + t.Fatalf("bounced = %v, want exactly one bounce", bounced) |
| + } |
| +} |
| + |
| +// TestSyncExecOptedInAddrRouteHonorsBoundEnvironment: environments- |
| +// awareness applies identically to the addr shape — a route bound to a |
| +// named environment resolves via LoadMerged (default overlaid by the |
| +// environment, environment winning per key), exactly like a socket-exec |
| +// route bound the same way. |
| +func TestSyncExecOptedInAddrRouteHonorsBoundEnvironment(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{ |
| + "LOG_LEVEL": "info", |
| + "BASE_ONLY": "1", |
| + }); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDelivery: %v", err) |
| + } |
| + if err := SaveEnvironment(ctx, s, "bab", "titogo", "demo", map[string]Var{ |
| + "LOG_LEVEL": {Kind: "env", Value: "debug"}, |
| + "DEMO_ONLY": {Kind: "env", Value: "yes"}, |
| + }, nil); err != nil { |
| + t.Fatalf("SaveEnvironment: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "demo.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:8444", Environment: "demo", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + got, err := os.ReadFile(filepath.Join(dd, "demo.go.tito.io.config.env")) |
| + if err != nil { |
| + t.Fatalf("read config env: %v", err) |
| + } |
| + want := execAddrConfigHeader + "BASE_ONLY=1\nDEMO_ONLY=yes\nLOG_LEVEL=debug\n" |
| + if string(got) != want { |
| + t.Fatalf("config env = %q, want %q", got, want) |
| + } |
| +} |
| + |
| +// TestSyncExecOptedInAddrAppWithNoVarsConvergesToEmptyFile: opting in |
| +// (SetExecDelivery alone, no SetEnv) creates the default bundle with an |
| +// empty Vars map — found=true, so this is the "configured, then nothing |
| +// set" state, not "absent". The route must get an EMPTY file, not have |
| +// none written at all and not have the mass-removal guard's absent |
| +// bucket confused by it. This is also the reason |
| +// "opted-in AND the group resolves absent" is unreachable by |
| +// construction: checkOptedIn requires the default bundle to exist |
| +// (found=true) before an addr route is ever added to resolved, and a |
| +// present-but-empty bundle is exactly resolution{m: <empty manifest>}, |
| +// never resolution{} (nil, absent). |
| +func TestSyncExecOptedInAddrAppWithNoVarsConvergesToEmptyFile(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDelivery: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "demo.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:8444", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + got, err := os.ReadFile(filepath.Join(dd, "demo.go.tito.io.config.env")) |
| + if err != nil { |
| + t.Fatalf("opted-in-but-unconfigured app's route got no file at all (want the header alone, not none): %v", err) |
| + } |
| + // No vars, but never a truly empty file: the header (issue #18) is |
| + // unconditional for this shape, whether or not the app has any vars |
| + // set yet. |
| + if string(got) != execAddrConfigHeader { |
| + t.Fatalf("config env = %q, want just the header (%q)", got, execAddrConfigHeader) |
| + } |
| +} |
| + |
| +// TestSyncExecMassRemovalGuardUnaffectedByOptedInAddrAppWithContent |
| +// exercises the mass-removal guard sharing its EXACT code path with the |
| +// addr shape (no addr-specific carve-out): an opted-in addr app WITH |
| +// content sits in the pass alongside a socket app that is genuinely |
| +// never configured. Only the socket app's group is absent — the guard |
| +// must not fire (only one of two eligible groups is absent, same as |
| +// TestSyncExecMassRemovalGuardDoesNotBlockNormalConvergence pins for two |
| +// socket apps), so the socket app's stale file is removed normally while |
| +// the addr app's file converges with its content, unaffected by sharing |
| +// the pass. |
| +func TestSyncExecMassRemovalGuardUnaffectedByOptedInAddrAppWithContent(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"A": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDelivery: %v", err) |
| + } |
| + // eleven is never configured at all: genuinely absent. |
| + routes := []registry.Route{ |
| + {Host: "demo.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", Status: "ready", Addr: "127.0.0.1:8444"}, |
| + {Host: "b.example.com", Kind: "instance", App: "eleven", Account: "bab", Status: "ready", Socket: "/run/carlos/b.sock"}, |
| + } |
| + for _, rt := range routes { |
| + if err := reg.Upsert(rt); err != nil { |
| + t.Fatalf("Upsert %s: %v", rt.Host, err) |
| + } |
| + } |
| + pathEleven := filepath.Join(dd, "b.example.com.config.env") |
| + if err := os.WriteFile(pathEleven, []byte("OLD=2\n"), 0o600); err != nil { |
| + t.Fatalf("stage eleven: %v", err) |
| + } |
| + var bounced []string |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(h string) error { |
| + bounced = append(bounced, h) |
| + return nil |
| + }); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + got, err := os.ReadFile(filepath.Join(dd, "demo.go.tito.io.config.env")) |
| + if err != nil { |
| + t.Fatalf("titogo's config file wasn't written: %v", err) |
| + } |
| + want := execAddrConfigHeader + "A=1\n" |
| + if string(got) != want { |
| + t.Fatalf("titogo config env = %q, want %q", got, want) |
| + } |
| + if _, err := os.Stat(pathEleven); !os.IsNotExist(err) { |
| + t.Fatalf("eleven's config file survived its manifest's genuine absence, stat err = %v (guard incorrectly disarmed convergence, or incorrectly held it back)", err) |
| + } |
| + // eleven's file is a removal (bounceLogged fires); titogo's is a |
| + // fresh write, socket-shape rules would bounce it too, but titogo is |
| + // the addr shape — it must never appear here. |
| + if !slices.Contains(bounced, "b.example.com") { |
| + t.Fatalf("bounced = %v, want eleven's removal bounce", bounced) |
| + } |
| + if slices.Contains(bounced, "demo.go.tito.io") { |
| + t.Fatalf("bounced = %v, the opted-in addr route must never be bounced", bounced) |
| + } |
| + if len(bounced) != 1 { |
| + t.Fatalf("bounced = %v, want exactly one bounce", bounced) |
| + } |
| +} |
| + |
| +// TestSyncExecOptOutLeavesStaleFileButWarnsEveryPass is IMPORTANT 2 from |
| +// the 2026-08-03 review's ruling: turning exec_delivery off does NOT |
| +// delete an already-materialized <host>.config.env (deleting it would |
| +// change what the app's own supervisor layers in on its next respawn, at |
| +// a moment carlos does not control and cannot announce — worse than a |
| +// stale file, and consistent with every other ineligibility transition in |
| +// this file). But the file holds DECRYPTED secrets and nothing will ever |
| +// update it again once opted out, so the silence itself was the bug: |
| +// every pass must name the stale file and the manual remedy. |
| +func TestSyncExecOptOutLeavesStaleFileButWarnsEveryPass(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"TITOGO_DEMO_BANNER": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDelivery: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "demo.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:8444", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec (opted in): %v", err) |
| + } |
| + path := filepath.Join(dd, "demo.go.tito.io.config.env") |
| + staged, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("read staged config env: %v", err) |
| + } |
| + |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ""); err != nil { |
| + t.Fatalf("SetExecDelivery (off): %v", err) |
| + } |
| + var bounced []string |
| + bounce := func(h string) error { bounced = append(bounced, h); return nil } |
| + var syncErr error |
| + out := captureStdout(t, func() { |
| + syncErr = SyncExec(ctx, reg, s, pk, dd, bounce) |
| + }) |
| + if syncErr != nil { |
| + t.Fatalf("SyncExec (opted out): %v", syncErr) |
| + } |
| + got, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("stale config env was deleted on opt-out, want it left in place: %v", err) |
| + } |
| + if string(got) != string(staged) { |
| + t.Fatalf("stale config env changed on opt-out: got %q, want the untouched original %q", got, staged) |
| + } |
| + if len(bounced) != 0 { |
| + t.Fatalf("bounced = %v, want none", bounced) |
| + } |
| + if !strings.Contains(out, "demo.go.tito.io") || !strings.Contains(out, path) { |
| + t.Fatalf("stdout = %q, want a line naming the host and the stale file's path", out) |
| + } |
| + if !strings.Contains(strings.ToLower(out), "secret") { |
| + t.Fatalf("stdout = %q, want the warning to say the file holds secrets", out) |
| + } |
| + |
| + // The warning is PER PASS, not one-shot: a second pass with nothing |
| + // changed must repeat it, since nothing about the risk (stale |
| + // plaintext secrets, forever) has gone away. |
| + out2 := captureStdout(t, func() { |
| + syncErr = SyncExec(ctx, reg, s, pk, dd, bounce) |
| + }) |
| + if syncErr != nil { |
| + t.Fatalf("SyncExec (opted out, second pass): %v", syncErr) |
| + } |
| + if !strings.Contains(out2, "demo.go.tito.io") { |
| + t.Fatalf("second pass stdout = %q, want the warning repeated", out2) |
| + } |
| + if len(bounced) != 0 { |
| + t.Fatalf("bounced = %v after the second pass, want none", bounced) |
| + } |
| +} |
| + |
| +// TestSyncExecNoOptOutWarningWhenNeverOptedIn: the stale-file warning |
| +// fires only when a file actually exists — an app that was simply NEVER |
| +// opted in must stay exactly as quiet as it was before issue #18 existed, |
| +// not gain a new per-pass line for a file it never had. |
| +func TestSyncExecNoOptOutWarningWhenNeverOptedIn(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"K": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + // No SetExecDelivery call at all. |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "demo.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", |
| + Status: "ready", Addr: "127.0.0.1:8444", |
| + }); err != nil { |
| + t.Fatalf("Upsert: %v", err) |
| + } |
| + var syncErr error |
| + out := captureStdout(t, func() { |
| + syncErr = SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }) |
| + }) |
| + if syncErr != nil { |
| + t.Fatalf("SyncExec: %v", syncErr) |
| + } |
| + if strings.Contains(out, "demo.go.tito.io") { |
| + t.Fatalf("stdout = %q, want no mention of a route that never had a file", out) |
| + } |
| +} |
| + |
| +// TestSyncExecAddrFileCarriesHeaderSocketFileDoesNot is IMPORTANT 3 from |
| +// the 2026-08-03 review: the issue #18 design comment's contract line — |
| +// "the file's header comment says so" — pinned directly. The addr shape's |
| +// file must carry it; the socket shape's file (same function, same pass) |
| +// must NOT, so an existing fleet's socket-exec files never churn or bounce |
| +// on upgrade (see execAddrConfigHeader's doc comment for the full reason). |
| +func TestSyncExecAddrFileCarriesHeaderSocketFileDoesNot(t *testing.T) { |
| + ctx, s, reg, dd, priv, _ := execKit(t) |
| + pk, err := ParsePrivateKey(priv, "test") |
| + if err != nil { |
| + t.Fatalf("ParsePrivateKey: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"A": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + if err := SetExecDelivery(ctx, s, "bab", "titogo", ExecDeliveryFile); err != nil { |
| + t.Fatalf("SetExecDelivery: %v", err) |
| + } |
| + if err := SetEnv(ctx, s, "bab", "eleven", map[string]string{"B": "1"}); err != nil { |
| + t.Fatalf("SetEnv: %v", err) |
| + } |
| + routes := []registry.Route{ |
| + {Host: "demo.go.tito.io", Kind: "instance", App: "titogo", Account: "bab", Status: "ready", Addr: "127.0.0.1:8444"}, |
| + {Host: "inst.example.com", Kind: "instance", App: "eleven", Account: "bab", Status: "ready", Socket: "/run/carlos/inst.sock"}, |
| + } |
| + for _, rt := range routes { |
| + if err := reg.Upsert(rt); err != nil { |
| + t.Fatalf("Upsert %s: %v", rt.Host, err) |
| + } |
| + } |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec: %v", err) |
| + } |
| + addrGot, err := os.ReadFile(filepath.Join(dd, "demo.go.tito.io.config.env")) |
| + if err != nil { |
| + t.Fatalf("read addr config env: %v", err) |
| + } |
| + if !strings.HasPrefix(string(addrGot), "#") { |
| + t.Fatalf("addr config env = %q, want it to start with a header comment", addrGot) |
| + } |
| + if !strings.Contains(string(addrGot), "carlos") { |
| + t.Fatalf("addr config env = %q, want the header to say carlos wrote it", addrGot) |
| + } |
| + if !strings.Contains(strings.ToLower(string(addrGot)), "never") || !strings.Contains(strings.ToLower(string(addrGot)), "restart") { |
| + t.Fatalf("addr config env = %q, want the header to say carlos never restarts this process", addrGot) |
| + } |
| + if !strings.Contains(strings.ToLower(string(addrGot)), "supervisor") { |
| + t.Fatalf("addr config env = %q, want the header to say the app's supervisor owns restarts", addrGot) |
| + } |
| + socketGot, err := os.ReadFile(filepath.Join(dd, "inst.example.com.config.env")) |
| + if err != nil { |
| + t.Fatalf("read socket config env: %v", err) |
| + } |
| + if want := "B=1\n"; string(socketGot) != want { |
| + t.Fatalf("socket config env = %q, want exactly %q — no header on this shape, ever", socketGot, want) |
| + } |
| +} |
| + |
| // TestSyncExecConvergesToEmptyFileWhenAllVarsUnset covers the lifecycle |
| // F8 flagged as missing: a manifest object that still EXISTS but has had |
| // every var unset must converge its route to an EMPTY file, not have the |