carlos / platform Public

isolated exec: nested schedule token path is tested; comment copy follows the no-dash rule (plan 3, task 6 fix round 1)

Fix round 1 on 66b33fce:

1. Added TestScheduleTokenUsesTheNestedPathForANestedRoute in
   internal/host/schedule_wiring_test.go: a route classified LayoutNested
   (DBPath = provision.NestedDBPath, Socket = provision.NestedSocket, Kind
   instance, Hibernate true) gets its token read and minted at
   provision.InstanceDir(dd, host)/<host>.env, asserted by reading that
   file's bytes and by confirming the flat <dataDir>/<host>.env is never
   created; a flat route added to the same registry still resolves to and
   mints at the flat path, asserted the same way.

2. Rewrote the 17 added lines (this task's own diff against 66b33fce, not
   pre-existing lines) that used an em dash, across
   internal/appconfig/syncexec.go, internal/host/agent.go,
   internal/host/agent_isolated_test.go, internal/host/movespass.go,
   internal/host/movespass_test.go, internal/host/schedule_wiring.go,
   internal/ops/litestream.go, internal/ops/ops_test.go and
   internal/provision/provision.go, each split into two sentences or
   rejoined with a comma. `git diff 66b33fce -U0 -- internal/appconfig
   internal/host internal/ops internal/provision internal/schedule | grep
   '^+' | grep '—'` now returns nothing.

internal/activator untouched, per the coordinator's instruction.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 1 month ago bb4e4e916b37a0cf0d6ecc2a68c9668297eb7bb4 parent c8d540d

10 files changed, +99 −20

internal/appconfig/syncexec.go +7 −7
internal/host/agent.go +3 −3
internal/host/agent_isolated_test.go +1 −1
internal/host/movespass.go +1 −1
internal/host/movespass_test.go +3 −3
internal/host/schedule_wiring.go +1 −1
internal/host/schedule_wiring_test.go +79 −0
internal/ops/litestream.go +1 −1
internal/ops/ops_test.go +2 −2
internal/provision/provision.go +1 −1

CI passed — run details

diff --git a/internal/appconfig/syncexec.go b/internal/appconfig/syncexec.go

index 4d38d8bfc..9b286db07 100644

--- a/internal/appconfig/syncexec.go

+++ b/internal/appconfig/syncexec.go

@@ -55,9 +55,9 @@ import (

const configEnvSuffix = ".config.env"

// nestedRoute reports whether rt's DB and socket are exactly the nested

-// layout's defaults (isolated-exec design §8.1) — the same two-column test

-// activator.Classify makes, spelled out here rather than imported so this

-// package need not take a dependency on internal/activator for one

+// layout's defaults (isolated-exec design §8.1). It is the same two-column

+// test activator.Classify makes, spelled out here rather than imported so

+// this package need not take a dependency on internal/activator for one

// comparison.

func nestedRoute(dataDir string, rt registry.Route) bool {

return rt.DBPath == provision.NestedDBPath(dataDir, rt.Host) && rt.Socket == provision.NestedSocket(rt.Host)

@@ -940,8 +940,8 @@ func syncExecHost(rt registry.Route, m *Manifest, key string, priv *ecdh.Private

if existed && bytes.Equal(cur, rendered) {

return // up to date — stay quiet on the periodic tick

}

- // A nested row's config file lives inside its own instance directory —

- // tenant-owned ground (isolated-exec design §3.2) — so it is written

+ // A nested row's config file lives inside its own instance directory,

+ // tenant-owned ground (isolated-exec design §3.2). So it is written

// through instancefs.WriteAtomic's O_EXCL temp name and openat2

// resolution rather than a predictable path.tmp: the same reasoning

// backend_exec.go's own writes into that directory follow. The flat

@@ -1065,8 +1065,8 @@ func reapConfigEnv(routes []registry.Route, dataDir string) {

// directory, one level under <dataDir>/instances, so a stale file there is

// found by walking that tree rather than dataDir's own listing. The flat

// directory's <host>.config / <host>.config.env name collision (reapConfigEnv's

-// own ambiguity guard, above) cannot happen inside an instance directory —

-// nothing in there is named after some OTHER host — so no such guard is

+// own ambiguity guard, above) cannot happen inside an instance directory.

+// Nothing in there is named after some OTHER host, so no such guard is

// needed here.

func reapNestedConfigEnv(live map[string]bool, dataDir string) {

root := filepath.Join(dataDir, "instances")

diff --git a/internal/host/agent.go b/internal/host/agent.go

index 1dbc325f7..8f4b5e6c6 100644

--- a/internal/host/agent.go

+++ b/internal/host/agent.go

@@ -324,7 +324,7 @@ func RunAgent(args []string) {

Unit: &activator.UnitBackend{OptDir: release.OptDir()},

}, reg) // the registry satisfies activator.DueStore directly

// doWake's own re-read of the committed row before it spawns

- // (design §6.8) — the registry satisfies RouteLookup with its own

+ // (design §6.8). The registry satisfies RouteLookup with its own

// Get, exactly as DueStore above.

svc.Routes = reg

if fleetStore != nil {

@@ -349,8 +349,8 @@ func RunAgent(args []string) {

}

armMovesPass(*cfg, dd, reg, svc)

// The startup sweep (design §6.7): a leftover instance unit from a

- // previous agent process — one this box's registry no longer holds

- // a live wake for — is stopped before this process starts spawning

+ // previous agent process, one this box's registry no longer holds

+ // a live wake for, is stopped before this process starts spawning

// its own. Best-effort, like every other startup duty here: a

// channel that never answers logs and moves on, and the box is no

// worse off than before this existed (a leftover unit that keeps

diff --git a/internal/host/agent_isolated_test.go b/internal/host/agent_isolated_test.go

index f54d110c7..c81836eb6 100644

--- a/internal/host/agent_isolated_test.go

+++ b/internal/host/agent_isolated_test.go

@@ -44,7 +44,7 @@ func TestNestedGateRefusesOnAMountCheckError(t *testing.T) {

// TestNestedGatePassesMountsThenChecksReady pins the second half: once

// every mount checks out, the gate's answer is exactly whatever /ready

-// says — refused when the control service is not ready, allowed when it is.

+// says, refused when the control service is not ready, allowed when it is.

func TestNestedGatePassesMountsThenChecksReady(t *testing.T) {

mounts := func(string) (bool, error) { return true, nil }

diff --git a/internal/host/movespass.go b/internal/host/movespass.go

index b4fda635f..865479cbf 100644

--- a/internal/host/movespass.go

+++ b/internal/host/movespass.go

@@ -142,7 +142,7 @@ type moveCfg struct {

OwnsRoute func(host string) bool

// Route reads the row itself, so a cutting move can checkpoint the

// row's OWN DBPath (isolated-exec design §9) instead of guessing the

- // legacy <DataDir>/<host>.db — the guess is wrong for a nested row,

+ // legacy <DataDir>/<host>.db. The guess is wrong for a nested row,

// whose DB lives under provision.InstanceDir. Nil (a test with no

// registry in hand) keeps the guess, unchanged.

Route func(host string) (*registry.Route, error)

diff --git a/internal/host/movespass_test.go b/internal/host/movespass_test.go

index 2426c692e..17b0f1d7a 100644

--- a/internal/host/movespass_test.go

+++ b/internal/host/movespass_test.go

@@ -246,8 +246,8 @@ func TestMovesPassSkipsWithoutTargetCoords(t *testing.T) {

// A cutting move for a host whose layout migration is in flight (the

// journal at provision.LayoutJournalPath) does nothing this round: its DB

// may be mid-copy between the flat and nested paths, so neither checkpoint

-// nor the warm-cache purge is safe until the journal clears — the same fence

-// Classify applies to a wake, applied here to a cut.

+// nor the warm-cache purge is safe until the journal clears. It is the same

+// fence Classify applies to a wake, applied here to a cut.

func TestMovesPassCutFencedByLayoutJournal(t *testing.T) {

h := newMoveHarness(t)

host := "a.go.example.com"

@@ -305,7 +305,7 @@ func TestMovesPassCutUsesRouteDBPath(t *testing.T) {

t.Fatalf("checkpoint did not use the route's own DBPath: %v", h.f.ops)

}

if fileExistsForTest(nested) {

- t.Fatal("the route's own DB survived the cut — the warm-cache trap did not follow DBPath")

+ t.Fatal("the route's own DB survived the cut. The warm-cache trap did not follow DBPath")

}

if got := h.read(t, host); got.Region != "uk" || got.Move != nil {

t.Fatalf("after cut: region=%q move=%+v", got.Region, got.Move)

diff --git a/internal/host/schedule_wiring.go b/internal/host/schedule_wiring.go

index da721303e..d3a951d36 100644

--- a/internal/host/schedule_wiring.go

+++ b/internal/host/schedule_wiring.go

@@ -183,7 +183,7 @@ func scheduleToken(dd string, svc *activator.Service, reg *registry.Registry) fu

// envPathFor is scheduleToken's route to an instance's own env file: flat by

// default, and the nested layout's own <host>.env (design §7) when this

-// box's registry classifies the host as a nested row — the SAME

+// box's registry classifies the host as a nested row. It is the SAME

// classification the exec backend used to decide where it spawned the

// instance in the first place (activator.Classify), so the runner and the

// token-ensure step ask the file the backend actually wrote rather than

diff --git a/internal/host/schedule_wiring_test.go b/internal/host/schedule_wiring_test.go

index 0b2c134ef..aa221a854 100644

--- a/internal/host/schedule_wiring_test.go

+++ b/internal/host/schedule_wiring_test.go

@@ -10,6 +10,7 @@ import (

"testing"

"github.com/carlosframework/platform/internal/activator"

+ "github.com/carlosframework/platform/internal/provision"

"github.com/carlosframework/platform/internal/registry"

)

@@ -145,6 +146,84 @@ func TestScheduleTokenMintsForASleepingInstance(t *testing.T) {

}

+// TestScheduleTokenUsesTheNestedPathForANestedRoute pins envPathFor's nested

+// branch (schedule_wiring.go): a route this box's registry classifies as

+// LayoutNested (its DBPath and Socket are the nested layout's own defaults)

+// gets its token read and minted at the nested layout's own <host>.env,

+// inside provision.InstanceDir, never at the flat <dataDir>/<host>.env path.

+// A flat route, added to the very same registry, still resolves to the

+// flat path exactly as before this branch existed.

+func TestScheduleTokenUsesTheNestedPathForANestedRoute(t *testing.T) {

+ dd := t.TempDir()

+ reg, err := registry.Open(filepath.Join(dd, "r.db"))

+ if err != nil {

+ t.Fatal(err)

+ }

+ defer reg.Close()

+ svc := &activator.Service{}

+ const nestedHost = "nested.bab.oncarlos.com"

+ if err := os.MkdirAll(provision.InstanceDir(dd, nestedHost), 0o755); err != nil {

+ t.Fatal(err)

+ }

+ if err := reg.Upsert(registry.Route{

+ Host: nestedHost, Kind: "instance", Account: "bab", App: "app", Status: "ready",

+ Hibernate: true,

+ DBPath: provision.NestedDBPath(dd, nestedHost),

+ Socket: provision.NestedSocket(nestedHost),

+ }); err != nil {

+ t.Fatal(err)

+ }

+ tok, err := scheduleToken(dd, svc, reg)(nestedHost)

+ if err != nil {

+ t.Fatalf("Token on a nested route: %v", err)

+ }

+ if len(tok) != 64 {

+ t.Fatalf("minted token = %q, want 32 random bytes as hex", tok)

+ }

+ nestedPath := filepath.Join(provision.InstanceDir(dd, nestedHost), nestedHost+".env")

+ nestedRaw, err := os.ReadFile(nestedPath)

+ if err != nil {

+ t.Fatalf("token was not minted at the nested path %s: %v", nestedPath, err)

+ }

+ if !strings.Contains(string(nestedRaw), "CARLOS_ADMIN_TOKEN="+tok) {

+ t.Fatalf("nested env file = %q, want it to carry the minted token", nestedRaw)

+ }

+ flatPathForNestedHost := filepath.Join(dd, nestedHost+".env")

+ if _, err := os.Stat(flatPathForNestedHost); !os.IsNotExist(err) {

+ t.Fatalf("a nested route's token also landed at the flat path %s, stat err = %v", flatPathForNestedHost, err)

+ }

+ // The same registry, asked for a route whose DBPath and Socket are the

+ // FLAT defaults: envPathFor must not have gone flat-blind because a

+ // nested row now shares the box.

+ const flatHost = "flat.bab.oncarlos.com"

+ if err := reg.Upsert(registry.Route{

+ Host: flatHost, Kind: "instance", Account: "bab", App: "app", Status: "ready",

+ Hibernate: true,

+ DBPath: provision.DefaultDBPath(dd, flatHost),

+ Socket: filepath.Join(provision.SocketDir, flatHost+".sock"),

+ }); err != nil {

+ t.Fatal(err)

+ }

+ flatTok, err := scheduleToken(dd, svc, reg)(flatHost)

+ if err != nil {

+ t.Fatalf("Token on a flat route: %v", err)

+ }

+ flatPath := filepath.Join(dd, flatHost+".env")

+ flatRaw, err := os.ReadFile(flatPath)

+ if err != nil {

+ t.Fatalf("token was not minted at the flat path %s: %v", flatPath, err)

+ }

+ if !strings.Contains(string(flatRaw), "CARLOS_ADMIN_TOKEN="+flatTok) {

+ t.Fatalf("flat env file = %q, want it to carry the minted token", flatRaw)

+ }

+ if _, err := os.Stat(filepath.Join(provision.InstanceDir(dd, flatHost), flatHost+".env")); !os.IsNotExist(err) {

+ t.Fatalf("a flat route's token landed under its instance directory, stat err = %v", err)

+ }

// A Token error becomes the row's last_status and from there the bucket

// status object and `carlos schedule ls`: it must not carry the box's

// filesystem layout.

diff --git a/internal/ops/litestream.go b/internal/ops/litestream.go

index 6b14b482f..21396ef01 100644

--- a/internal/ops/litestream.go

+++ b/internal/ops/litestream.go

@@ -359,7 +359,7 @@ func FleetDBsWithSkipped(reg *registry.Registry, dataDir string) (dbs []string,

// <dataDir>/instances/<id>/<host>.db (isolated-exec design §7): the flat

// glob above never walks into that directory, so its matches are folded

// in here before the claimed-files gate below, which already knows how

- // to accept or skip each one — a nested row's DBPath was added to

+ // to accept or skip each one. A nested row's DBPath was added to

// claimed by the very same loop that adds a flat row's.

nested, err := filepath.Glob(filepath.Join(dataDir, "instances", "*", "*.db"))

if err != nil {

diff --git a/internal/ops/ops_test.go b/internal/ops/ops_test.go

index 9bd1f4780..edfcb71ae 100644

--- a/internal/ops/ops_test.go

+++ b/internal/ops/ops_test.go

@@ -87,8 +87,8 @@ func TestConfigGenSkipsHibernatingRoutes(t *testing.T) {

// TestFleetDBsDiscoversNestedDatabases pins the isolated-exec companion to

// the flat glob above (design §7): a route's DBPath being "claimed" only

-// FILTERS what the glob already found — for a non-unit route it never adds

-// a path by itself — so a nested row's database, under

+// FILTERS what the glob already found. For a non-unit route it never adds

+// a path by itself, so a nested row's database, under

// <dataDir>/instances/<id>/<host>.db, would stay invisible to replication

// forever without the nested glob to find it in the first place.

func TestFleetDBsDiscoversNestedDatabases(t *testing.T) {

diff --git a/internal/provision/provision.go b/internal/provision/provision.go

index 9f5117c8f..7c68ed826 100644

--- a/internal/provision/provision.go

+++ b/internal/provision/provision.go

@@ -557,7 +557,7 @@ func ValidPool(pool string) bool { return regionPoolRe.MatchString(pool) }

// so the reverse import would be a cycle. A record's Environment column

// and a bundle's environment name are the SAME vocabulary; if the shape

// in appconfig/environments.go ever changes, this copy must change with

-// it — it exists only because of the import direction, not because the

+// it. It exists only because of the import direction, not because the

// rule is provision's to define.

var envNameRe = regexp.MustCompile(`^[a-z0-9-]{1,32}$`)