amadan

carlos / platform Public

registry: args/idle/max_run must land on a re-Upsert

Paul Campbell pushed by paul@keymail.dev 92d782e9656734ccb02e7112e4f010316543fe3d parent d802764
2 files changed, +78 −7
  • internal/registry/registry.go +22 −7
  • internal/registry/registry_test.go +56 −0
diff --git a/internal/registry/registry.go b/internal/registry/registry.go
index b68b7089e..775efbad1 100644
--- a/internal/registry/registry.go
+++ b/internal/registry/registry.go
@@ -656,12 +656,13 @@ func (r *Registry) Delete(host string) error {
return err
}
-// Upsert inserts or updates a route by host. Seven columns are deliberately
+// Upsert inserts or updates a route by host. Ten columns are deliberately
// not plain overwrites — `environment`, `caps`, `tunnel`, `provisioned`,
-// `seed`, `unit` and `fleet_box` all keep the existing row's value when the
-// incoming Route carries the zero one.
+// `seed`, `unit`, `fleet_box`, `args`, `idle_seconds` and `max_run_seconds`
+// all keep the existing row's value when the incoming Route carries the
+// zero one.
//
-// The reason is the same for all seven, and it is structural: a full-row
+// The reason is the same for all ten, and it is structural: a full-row
// Upsert caller (`carlos add`, the domains reconciler's two mint paths, the
// provision pass's mint) builds its Route from what IT knows, and none of
// them knows about the columns another feature owns. `environment` was the
@@ -686,12 +687,23 @@ func (r *Registry) Delete(host string) error {
// the next promote would stamp X-Carlos-Version onto a console still serving
// the old build, which is issue #110 returning silently.
//
+// `args`, `idle_seconds` and `max_run_seconds` are the same shape as `seed`
+// and for the same reason (2026-08-17 fix round 1): they are materialized
+// from a sidecar's manifest by the reconcile pass, and a full-row Upsert
+// from a caller that knows nothing about sidecars (`carlos add`) must not
+// blank a live argv or reset a worker's ceilings to the platform default.
+// Omitting them from this clause entirely — the original shape of this
+// change — was tried and rejected: omission is not preserve-if-empty, it
+// silently ignores even a genuine NEW value, so a sidecar whose manifest
+// argv changed would keep stale argv on the row until a narrow SetArgs
+// happened to run.
+//
// An explicit non-zero value still wins in every case; clearing keeps
// exactly one door per column, none of which goes through Upsert:
// SetEnvironment (`carlos route -environment -`), SetCaps (`carlos route
// -revoke`), SetTunnel (`carlos route -tunnel -`), SetUnit (`carlos route
-// -unit -`), SetSeed, SetFleetBox, and for `provisioned` the row's own
-// deletion.
+// -unit -`), SetSeed, SetFleetBox, SetArgs, SetWorkerLimits, and for
+// `provisioned` the row's own deletion.
//
// `addrs` and `health` (multi-upstream routing, issue #152) are, by
// contrast, PLAIN overwrites — see their ON CONFLICT clause below — same as
@@ -751,7 +763,10 @@ ON CONFLICT(host) DO UPDATE SET
seed=CASE WHEN excluded.seed = '' THEN routes.seed ELSE excluded.seed END,
unit=CASE WHEN excluded.unit = '' THEN routes.unit ELSE excluded.unit END,
fleet_box=CASE WHEN excluded.fleet_box = '' THEN routes.fleet_box ELSE excluded.fleet_box END,
- addrs=excluded.addrs, health=excluded.health`,
+ addrs=excluded.addrs, health=excluded.health,
+ args=CASE WHEN excluded.args = '' THEN routes.args ELSE excluded.args END,
+ idle_seconds=CASE WHEN excluded.idle_seconds = 0 THEN routes.idle_seconds ELSE excluded.idle_seconds END,
+ max_run_seconds=CASE WHEN excluded.max_run_seconds = 0 THEN routes.max_run_seconds ELSE excluded.max_run_seconds END`,
t.Host, t.Slug, t.Kind, t.Socket, t.Addr, t.Status, t.LastError,
t.DBPath, t.Version, t.PrevVersion, t.App, t.Pool, t.Region, t.Channel,
b2i(t.Hibernate), b2i(t.Canary), t.Replication, t.NextDue, t.Backing,
diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go
index 0bdba818d..189291ead 100644
--- a/internal/registry/registry_test.go
+++ b/internal/registry/registry_test.go
@@ -2467,3 +2467,59 @@ func TestRouteWrittenBeforeArgsStillParses(t *testing.T) {
t.Fatalf("row = %+v, want zero values for a pre-existing row", got)
}
}
+
+// TestUpsertAppliesNewArgsAndLimits (fix round 1, 2026-08-17): a re-Upsert
+// carrying a NEW non-empty args/idle/max_run over an existing row must land
+// the new values — the sidecar reconcile pass re-Upserts on every pass when
+// the app's manifest changes, and omission from the ON CONFLICT clause
+// would silently ignore even a genuine new value, not merely preserve an
+// old one.
+func TestUpsertAppliesNewArgsAndLimits(t *testing.T) {
+ r := openTestRegistry(t)
+ if err := r.Upsert(Route{
+ Host: "bot.y.internal", Kind: "sidecar",
+ Args: `["sidecar","run"]`, IdleSeconds: 300, MaxRunSeconds: 3600,
+ }); err != nil {
+ t.Fatalf("first upsert: %v", err)
+ }
+ if err := r.Upsert(Route{
+ Host: "bot.y.internal", Kind: "sidecar",
+ Args: `["sidecar","run","-v"]`, IdleSeconds: 60, MaxRunSeconds: 120,
+ }); err != nil {
+ t.Fatalf("second upsert: %v", err)
+ }
+ got, err := r.Get("bot.y.internal")
+ if err != nil {
+ t.Fatalf("get: %v", err)
+ }
+ if got.Args != `["sidecar","run","-v"]` || got.IdleSeconds != 60 || got.MaxRunSeconds != 120 {
+ t.Fatalf("row = %+v, want the second Upsert's new args/limits to have landed", got)
+ }
+}
+
+// TestUpsertPreservesArgsAndLimitsWhenCallerKnowsNothingOfThem (fix round 1,
+// 2026-08-17): a re-Upsert carrying EMPTY args and ZERO ceilings — the
+// `carlos add` shape, which knows nothing about sidecars — over a row that
+// already has them must PRESERVE the existing values, not blank them. Same
+// preserve-if-empty precedent as `seed` in the same ON CONFLICT clause.
+func TestUpsertPreservesArgsAndLimitsWhenCallerKnowsNothingOfThem(t *testing.T) {
+ r := openTestRegistry(t)
+ if err := r.Upsert(Route{
+ Host: "bot.z.internal", Kind: "sidecar",
+ Args: `["sidecar","run"]`, IdleSeconds: 300, MaxRunSeconds: 3600,
+ }); err != nil {
+ t.Fatalf("first upsert: %v", err)
+ }
+ // The carlos-add shape: a full-row Upsert that names none of these
+ // three columns at all.
+ if err := r.Upsert(Route{Host: "bot.z.internal", Kind: "sidecar"}); err != nil {
+ t.Fatalf("second upsert: %v", err)
+ }
+ got, err := r.Get("bot.z.internal")
+ if err != nil {
+ t.Fatalf("get: %v", err)
+ }
+ if got.Args != `["sidecar","run"]` || got.IdleSeconds != 300 || got.MaxRunSeconds != 3600 {
+ t.Fatalf("row = %+v, want the original args/limits preserved, not blanked", got)
+ }
+}