carlos / platform Public

Plan 4: fixes from the fleet-outage review

The every-pass ack task is gone, per the spec change: it could never
succeed on a fleet-door box, and it made the ack's timestamp stop
describing the etag beside it. Acks stay change-only, which is enough,
because writing an instance's credential always changes that file.

Also: the four test callers of the two re-signed functions, an
unresolved listing said once per app rather than once per route, and a
roll note that leads with the ListBucket grant a freeze would depend on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul Campbell pushed by claude agent, operated by paul@keymail.dev 55b2566282c6ddb12f2facbd67dad46e07b619ee parent 07e1afb
1 file changed, +76 −387
  • docs/superpowers/plans/2026-09-15-tenant-email-4-instance-config.md +76 −387
diff --git a/docs/superpowers/plans/2026-09-15-tenant-email-4-instance-config.md b/docs/superpowers/plans/2026-09-15-tenant-email-4-instance-config.md
index 9e0953e1e..7c79adbef 100644
--- a/docs/superpowers/plans/2026-09-15-tenant-email-4-instance-config.md
+++ b/docs/superpowers/plans/2026-09-15-tenant-email-4-instance-config.md
@@ -4,7 +4,7 @@
**Goal:** Give an app a third config bundle, `apps/<acct>/<app>/instances-config/<host>.json`, resolved after the named environment, so one instance can be handed a value none of its siblings ever see, and make the agent say out loud which version of that layer it rendered: `ConfigAck` gains `Schema` and `InstanceConfigETag`, and the agent rewrites the ack on every pass rather than only when a file changes, so a console can tell "this box has not got the layer yet" apart from "this box has nothing to render". Nothing in this plan writes an overlay for anyone: there is no CLI verb, no console door, and no email change. Step 5 is the layer's first caller.
-**Architecture:** `internal/appconfig` gains `instances.go`: the key, the host rule, the CAS read/write/delete, `LoadMergedHost`, and the two functions the sweeps reach the same answer through (`instanceOverlays`, one prefix listing per app, and `applyInstanceOverlay`, the pure merge). `Sync` and `SyncExec` keep their group caches exactly as they are — the group is still (Account, App, Environment) and the default bundle is still read once per (account, app) — and each route's own merge happens in the materialization loop, so a pass costs one `List` per app plus one `Get` per route that actually has an overlay. Four guards written against a two-layer merge are revisited one per task, each with the case that would have been wrong: `lostKeys`/`defaultMissing` (`sync.go:260`, `:388`), the mass-removal guard (`syncexec.go:341-380`), `AwaitingRestart` (`syncexec.go:771`) and `allShadowed` (`syncexec.go:872`). `ConfigAck` grows three fields and a second writer, `RefreshConfigAck`, which carries the previous ack's delivery facts forward so an every-pass rewrite cannot un-report an adoption `carlos relay status` already showed.
+**Architecture:** `internal/appconfig` gains `instances.go`: the key, the host rule, the CAS read/write/delete, `LoadMergedHost`, and the two functions the sweeps reach the same answer through (`instanceOverlays`, one prefix listing per app, and `applyInstanceOverlay`, the pure merge). `Sync` and `SyncExec` keep their group caches exactly as they are — the group is still (Account, App, Environment) and the default bundle is still read once per (account, app) — and each route's own merge happens in the materialization loop, so a pass costs one `List` per app plus one `Get` per route that actually has an overlay. Four guards written against a two-layer merge are revisited one per task, each with the case that would have been wrong: `lostKeys`/`defaultMissing` (`sync.go:260`, `:388`), the mass-removal guard (`syncexec.go:341-380`), `AwaitingRestart` (`syncexec.go:771`) and `allShadowed` (`syncexec.go:872`). `ConfigAck` grows two fields, `Schema` and `InstanceConfigETag`, and keeps its single change-only writer. An earlier draft had the agent rewrite the ack on every pass so that absence became decidable; the spec now rules that out (§6.5), because it is a Get and a Put per converged route per pass on every box, and on a fleet-door box `control/config-ack/` is outside both the read and the write scope (`internal/edge/storedoor.go:389`, `:489`), so every one of those passes would fail, log, and still leave the console with nothing. Delivery stays decidable where it matters: writing an instance's credential always changes that instance's file, so it always produces an ack.
**Tech Stack:** Go 1.26, `internal/appconfig` (bundles, merge, the two sync sweeps, the ack), `internal/host` (the agent's two config loops and `configAckFunc`), `internal/release` (`Store`, `NewDirStore`, `AppPrefix`, `ErrCASConflict`), `internal/registry` (route rows), `internal/console` (reads acks; unchanged by this plan).
@@ -37,9 +37,9 @@
| `internal/appconfig/store.go` (modify) | `getIfExistsAtETag` (the etag-returning read `getIfExistsAt` delegates to); `applyAt` returns the new etag |
| `internal/appconfig/sync.go` (modify) | the per-app overlay index, the per-route merge, `defaultMissing` extended to the overlay-alone case |
| `internal/appconfig/syncexec.go` (modify) | the same index and merge; absent base with an overlay is not a removal; the mass-removal guard; `AwaitingRestart` and `allShadowed` |
-| `internal/appconfig/configack.go` (modify) | `Schema`, `InstanceConfigETag`, `Refresh`, `ConfigAckSchema`, `RefreshConfigAck`, `LatestConfigAck`, `InstanceConfigDelivery` |
+| `internal/appconfig/configack.go` (modify) | `Schema`, `InstanceConfigETag`, `ConfigAckSchema`, `ConfigAckPrefix`, `LatestConfigAck`, `InstanceConfigDelivery` |
| `internal/appconfig/routefingerprint.go` (modify) | doc only: the layer is bucket-side, so the generation bump is its signal |
-| `internal/host/agent.go` (modify) | `configAckFunc` routes a refresh ack to `RefreshConfigAck` |
+| `internal/host/agent.go` (modify) | `configAckFunc` stamps `ConfigAckSchema` on every ack it writes |
---
@@ -251,6 +251,15 @@ func ValidInstanceHost(host string) bool {
if host == "" || len(host) > 253 || strings.Contains(host, "..") {
return false
}
+ // Per-label bound as well as the whole-name one, matching DNS and
+ // launchctx/controlname's label rule. Without it a single 200-character
+ // label passes here and is refused everywhere else that handles the same
+ // host, which is a disagreement between two gates rather than one rule.
+ for _, label := range strings.Split(host, ".") {
+ if len(label) > 63 {
+ return false
+ }
+ }
return instanceHostRe.MatchString(host)
}
@@ -1228,6 +1237,9 @@ func instanceOverlayLoader(ctx context.Context, s release.Store) func(acct, app,
// per host, and it costs one listing per app plus a Get for each
// instance that actually has an overlay.
overlayFor := instanceOverlayLoader(ctx, s)
+ // One line per app when its listing fails, not one per route: every
+ // route of that app fails identically and for the same reason.
+ said := map[groupKey]bool{}
```
Then in the materialization loop (`:143-176`), name the group key once and apply the overlay between the `m == nil` skip and the backing check:
@@ -1257,7 +1269,15 @@ Then in the materialization loop (`:143-176`), name the group key once and apply
// group whose bundle could not be read.
inst, _, err := overlayFor(k.acct, k.app, rt.Host)
if err != nil {
- fmt.Printf(" %s: instance config unresolved, env file left as-is: %v\n", rt.Host, err)
+ // Said ONCE per (account, app), not once per route. The listing
+ // this failed on is per app, so every route of that app fails
+ // identically and for the same reason; a line each would bury the
+ // cause under its own repetitions on a box with many routes. The
+ // loader already holds the error, so said is only about output.
+ if !said[k] {
+ said[k] = true
+ fmt.Printf(" %s/%s: instance config unresolved, env files left as-is: %v\n", k.acct, k.app, err)
+ }
continue
}
m = applyInstanceOverlay(k.app, m, inst)
@@ -1405,7 +1425,10 @@ Expected: `TestSyncRefusesToDropHandStagedKeysForAnOverlayAlone` fails — the f
// group whose bundle could not be read.
inst, _, err := overlayFor(k.acct, k.app, rt.Host)
if err != nil {
- fmt.Printf(" %s: instance config unresolved, env file left as-is: %v\n", rt.Host, err)
+ if !said[k] {
+ said[k] = true
+ fmt.Printf(" %s/%s: instance config unresolved, env files left as-is: %v\n", k.acct, k.app, err)
+ }
continue
}
m := r.m
@@ -1423,6 +1446,12 @@ Expected: `TestSyncRefusesToDropHandStagedKeysForAnOverlayAlone` fails — the f
m = applyInstanceOverlay(k.app, m, inst)
```
+The `syncHost` call further down the same loop takes the local rather than the resolution's field, or this compiles to "declared and not used" and the guard never sees the new case:
+
+```go
+ syncHost(rt, m, defaultMissing, priv, envDir, restart, instETag, ack)
+```
+
`internal/appconfig/sync.go`, `syncHost`'s guard (`:388-407`): the condition is unchanged, the message names which overlay it is.
```go
@@ -1632,6 +1661,9 @@ Expected: `TestSyncExecRendersTheInstanceOverlayForItsOwnRouteOnly` fails with `
// pass's: one listing per app, a Get only for a host it named, and an
// unresolved overlay leaves the file exactly as it is.
overlayFor := instanceOverlayLoader(ctx, s)
+ // One line per app when its listing fails, not one per route: every
+ // route of that app fails identically and for the same reason.
+ said := map[groupKey]bool{}
```
Then, inside the materialization loop, fetch it immediately after the `r.err` branch (`:397-407`) — BEFORE any branch that can remove a file — and apply it at the call:
@@ -1643,7 +1675,10 @@ Then, inside the materialization loop, fetch it immediately after the `r.err` br
// delivered secrets on a listing blip.
inst, _, instErr := overlayFor(k.acct, k.app, rt.Host)
if instErr != nil {
- fmt.Printf(" %s: instance config unresolved, config file left as-is: %v\n", rt.Host, instErr)
+ if !said[k] {
+ said[k] = true
+ fmt.Printf(" %s/%s: instance config unresolved, config files left as-is: %v\n", k.acct, k.app, instErr)
+ }
continue
}
if r.m == nil && skipRemovals {
@@ -1888,6 +1923,8 @@ git commit -m "appconfig: an instance overlay is configuration, so it is never a
```
`AwaitingRestart` keeps its signature exactly: `(ctx, s, routes, acct, app, envName, keys) RestartPending`.
+**On exported surface that arrives before its caller.** This plan lands three readers whose only callers are step 5's: `LoadMergedHost` (task 3), and `LatestConfigAck` with `InstanceConfigDelivery` (task 10). That is deliberate — they are the decidable half of a question step 5 has to answer at a door, and proving an etag comparison means what it says belongs with the layer, not with the door. The ruling below is narrower than it reads: it is about inventing a NEW question nothing has asked yet, not about landing the answer to one step 5 is already known to ask.
+
**Reporting an INSTANCE-layer write is step 5's, and that is a ruling.** `AwaitingRestart` answers "a write to app's `envName` bundle landed a file nothing will read": its whole vocabulary is (app, environment), and the CLI and console callers (`appconfig_cli.go:83`, `internal/console/apiconfig.go:544`) pass exactly what a member just wrote. Nothing in this plan writes an instance overlay, so there is no call site that would need the instance-layer question answered, and inventing one now would mean an exported function with no caller and no test but its own. What this task owes is the other direction, which is live the moment task 6 shipped: a write to the default or the named bundle can now be SHADOWED by an instance overlay, and naming such a route would send someone to restart a process for a value it was never going to see. Step 5 adds the instance-write reporting alongside the door that does the writing.
- [ ] **Step 1: Write the failing test**
@@ -1989,6 +2026,9 @@ Expected: all three fail — the route is named in every case, because nothing c
// a bundle. One listing for the app, then a Get per route that has an
// overlay; both live for one call, like the caches above.
overlayFor := instanceOverlayLoader(ctx, s)
+ // One line per app when its listing fails, not one per route: every
+ // route of that app fails identically and for the same reason.
+ said := map[groupKey]bool{}
```
Inside the route loop, after the opt-in verdict's `if !v.On { continue }` (`:841-843`):
@@ -2080,6 +2120,7 @@ git commit -m "appconfig: the restart notice knows an instance overlay shadows a
**Files:**
- Modify: `internal/appconfig/configack.go` (`ConfigAck` at `:36-64`), `internal/appconfig/sync.go` (`syncHost` at `:343`, its ack at `:451`), `internal/appconfig/syncexec.go` (`syncExecHost` at `:948`, its ack at `:1046`), `internal/appconfig/configack_test.go`
+- Modify, because they call the two functions this task re-signs and the package will not compile without them: `internal/appconfig/sync_test.go` (`syncHost` at `:403`, `:1057`, `:1075`) and `internal/appconfig/syncexec_test.go` (`syncExecHost` at `:776`). Each gets `""` for the new `instETag`, in the position shown in the Produces block below: after `restart` for `syncHost`, after `header` for `syncExecHost`. These are four one-word edits, and skipping them makes Step 4's "Expected: PASS" impossible to reach.
**Interfaces:**
- Produces:
@@ -2258,369 +2299,7 @@ git commit -m "appconfig: the config ack carries its schema and the instance ove
---
-### Task 10: The agent acks every pass, and a refresh never un-says a delivery
-
-**Files:**
-- Modify: `internal/appconfig/configack.go` (`WriteConfigAck` at `:81`), `internal/appconfig/sync.go` (`syncHost`'s up-to-date return at `:433`), `internal/appconfig/syncexec.go` (`syncExecHost`'s at `:1016`), `internal/host/agent.go` (`configAckFunc` at `:935`), `internal/appconfig/configack_test.go`, `internal/host/agent_test.go`
-
-**Interfaces:**
-- Produces:
- ```go
- type ConfigAck struct { ...; Refresh bool `json:"-"` } // which door, never wire state
- func RefreshConfigAck(ctx context.Context, s release.Store, ack ConfigAck) error
- ```
-
-**The regression this task exists to avoid.** An ack is written today only when a pass changes a file (`configack.go:66-68`), so a fresh instance, or one whose config has not moved since its agent was rolled, has no ack at all and absence cannot be read as "old agent" (design §6.5). The fix is to rewrite the ack every pass. Done naively it would break the thing acks are already used for: `carlos relay status` reads `Restarted`, `Hibernating`, `RestartError` and `RelaySuffixes` off the latest ack and prints "adopted at <At>, restarted" (`internal/console/apirelay.go:414-430`), so an every-pass rewrite that reported this pass's facts would replace a real adoption with `Restarted: false` an hour later and tell a member their route is still waiting. A refresh therefore carries the previous ack's delivery facts forward and changes only what the AGENT is stating about itself: its schema and the overlay it rendered.
-
-- [ ] **Step 1: Write the failing test**
-
-`internal/appconfig/configack_test.go` (append):
-
-```go
-// A pass that changes nothing still leaves a statement behind, because
-// silence is the one thing a reader cannot interpret: no ack at all means
-// either an agent too old to render the instance layer or a route whose
-// config simply has not moved, and those need different answers.
-func TestSyncAcksEveryPassIncludingUnchangedOnes(t *testing.T) {
- ctx := context.Background()
- s := release.NewDirStore(t.TempDir())
- reg := testRegistry(t)
- envDir := t.TempDir()
- priv, _ := syncRelayTestKeys(t)
- if err := SetEnv(ctx, s, "bab", "titogo", map[string]string{"APP_WIDE": "yes"}); err != nil {
- t.Fatalf("SetEnv: %v", err)
- }
- _, want, err := ApplyInstance(ctx, s, "bab", "titogo", "book.example.com",
- map[string]Var{"TENANT": {Kind: "env", Value: "book"}}, nil)
- if err != nil {
- t.Fatalf("ApplyInstance: %v", err)
- }
- if err := reg.Upsert(registry.Route{
- Host: "book.example.com", Kind: "instance", App: "titogo", Account: "bab", Status: "ready",
- Backing: registry.BackingUnit,
- }); err != nil {
- t.Fatalf("Upsert: %v", err)
- }
- var acks []ConfigAck
- collect := func(a ConfigAck) { acks = append(acks, a) }
- for i := 0; i < 2; i++ {
- if err := Sync(ctx, reg, s, priv, envDir, func(string) error { return nil }, collect); err != nil {
- t.Fatalf("Sync %d: %v", i, err)
- }
- }
- if len(acks) != 2 {
- t.Fatalf("acks = %d, want one per pass: the second pass changed nothing and still has to state its schema", len(acks))
- }
- if acks[0].Refresh {
- t.Fatal("the delivering pass marked its ack as a refresh")
- }
- if !acks[1].Refresh {
- t.Fatal("the unchanged pass did not mark its ack as a refresh, so it would overwrite the delivery facts")
- }
- for i, a := range acks {
- if a.Schema != ConfigAckSchema || a.InstanceConfigETag != want {
- t.Fatalf("ack %d = %+v, want schema %d and etag %q", i, a, ConfigAckSchema, want)
- }
- }
-}
-
-// The refresh keeps every word the delivering pass said. `carlos relay
-// status` reads Restarted, Hibernating, RestartError and RelaySuffixes off
-// the latest ack, so a refresh that reported its own (empty) version of
-// those would turn a reported adoption back into "awaiting delivery" on the
-// next hourly pass.
-func TestRefreshConfigAckKeepsWhatTheDeliveringPassSaid(t *testing.T) {
- ctx := context.Background()
- s := release.NewDirStore(t.TempDir())
- delivered := ConfigAck{
- Schema: ConfigAckSchema, Box: "box-1", Host: "book.example.com", Shape: "unit",
- At: time.Date(2026, 9, 15, 12, 0, 0, 0, time.UTC), Restarted: true,
- RelaySuffixes: map[string]string{"eu-west-1": "bab:titogo:3:k7"},
- InstanceConfigETag: "etag-1",
- }
- if err := WriteConfigAck(ctx, s, delivered); err != nil {
- t.Fatalf("WriteConfigAck: %v", err)
- }
- refresh := ConfigAck{
- Schema: ConfigAckSchema, Box: "box-1", Host: "book.example.com", Shape: "unit",
- At: time.Date(2026, 9, 15, 13, 0, 0, 0, time.UTC), InstanceConfigETag: "etag-2", Refresh: true,
- }
- if err := RefreshConfigAck(ctx, s, refresh); err != nil {
- t.Fatalf("RefreshConfigAck: %v", err)
- }
- body, _, err := s.Get(ctx, ConfigAckKey("box-1", "book.example.com"))
- if err != nil {
- t.Fatalf("read ack: %v", err)
- }
- var got ConfigAck
- if err := json.Unmarshal(body, &got); err != nil {
- t.Fatalf("decode ack: %v", err)
- }
- if !got.Restarted || !got.At.Equal(delivered.At) || got.RelaySuffixes["eu-west-1"] != "bab:titogo:3:k7" {
- t.Fatalf("the refresh un-said the delivery: %+v", got)
- }
- if got.InstanceConfigETag != "etag-2" || got.Schema != ConfigAckSchema {
- t.Fatalf("the refresh did not state what the agent rendered: %+v", got)
- }
- if strings.Contains(string(body), "refresh") {
- t.Fatalf("Refresh reached the wire: %s", body)
- }
-}
-
-// The steady state costs a read and nothing else. Every converged route on
-// the box passes through here on every pass, so a Put per route per pass
-// would be a write storm for a statement nobody's changed.
-func TestRefreshConfigAckWritesNothingInTheSteadyState(t *testing.T) {
- ctx := context.Background()
- base := release.NewDirStore(t.TempDir())
- ack := ConfigAck{Schema: ConfigAckSchema, Box: "box-1", Host: "book.example.com", Shape: "unit",
- At: time.Now(), InstanceConfigETag: "etag-1"}
- if err := WriteConfigAck(ctx, base, ack); err != nil {
- t.Fatalf("WriteConfigAck: %v", err)
- }
- s := newCountingStore(base)
- ack.Refresh = true
- if err := RefreshConfigAck(ctx, s, ack); err != nil {
- t.Fatalf("RefreshConfigAck: %v", err)
- }
- if got := s.n(s.puts, ConfigAckKey("box-1", "book.example.com")); got != 0 {
- t.Fatalf("puts = %d, want none: the ack already says exactly this", got)
- }
- ack.InstanceConfigETag = "etag-2"
- if err := RefreshConfigAck(ctx, s, ack); err != nil {
- t.Fatalf("RefreshConfigAck (moved): %v", err)
- }
- if got := s.n(s.puts, ConfigAckKey("box-1", "book.example.com")); got != 1 {
- t.Fatalf("puts = %d, want 1 once the overlay moved", got)
- }
-}
-
-// A route that has never changed since its agent was rolled has no ack at
-// all, and the refresh is what gives it one. This is the case design §6.5
-// names: absence must stop meaning "old agent".
-func TestRefreshConfigAckCreatesTheFirstAck(t *testing.T) {
- ctx := context.Background()
- s := release.NewDirStore(t.TempDir())
- if err := RefreshConfigAck(ctx, s, ConfigAck{Schema: ConfigAckSchema, Box: "box-1",
- Host: "book.example.com", Shape: "unit", At: time.Now(), Refresh: true}); err != nil {
- t.Fatalf("RefreshConfigAck: %v", err)
- }
- body, _, err := s.Get(ctx, ConfigAckKey("box-1", "book.example.com"))
- if err != nil {
- t.Fatalf("no ack was created: %v", err)
- }
- var got ConfigAck
- if err := json.Unmarshal(body, &got); err != nil {
- t.Fatalf("decode ack: %v", err)
- }
- if got.Schema != ConfigAckSchema || got.Restarted {
- t.Fatalf("first ack = %+v, want the schema and no claim of a restart", got)
- }
-}
-```
-
-Add `"encoding/json"`, `"strings"` and `"time"` to `configack_test.go`'s imports as needed.
-
-`internal/host/agent_test.go` (append):
-
-```go
-// The agent's ack callback has two doors, and a refresh must go through the
-// one that carries the last delivery forward: the hourly pass rewrites an
-// ack for every converged route, and the console reads adoption off that
-// object (internal/console/apirelay.go).
-func TestConfigAckFuncRoutesARefreshThroughRefreshConfigAck(t *testing.T) {
- ctx := context.Background()
- s := release.NewDirStore(t.TempDir())
- write := configAckFunc(ctx, s, "test")
-
- write(appconfig.ConfigAck{Schema: appconfig.ConfigAckSchema, Host: "book.example.com",
- Shape: "unit", At: time.Now(), Restarted: true, InstanceConfigETag: "etag-1"})
- write(appconfig.ConfigAck{Schema: appconfig.ConfigAckSchema, Host: "book.example.com",
- Shape: "unit", At: time.Now(), InstanceConfigETag: "etag-2", Refresh: true})
-
- body, _, err := s.Get(ctx, appconfig.ConfigAckKey(BoxLabel(), "book.example.com"))
- if err != nil {
- t.Fatalf("read ack: %v", err)
- }
- var got appconfig.ConfigAck
- if err := json.Unmarshal(body, &got); err != nil {
- t.Fatalf("decode ack: %v", err)
- }
- if !got.Restarted {
- t.Fatal("the refresh erased the restart the delivering pass reported")
- }
- if got.InstanceConfigETag != "etag-2" {
- t.Fatalf("InstanceConfigETag = %q, want the refreshed etag", got.InstanceConfigETag)
- }
-}
-```
-
-Add `"encoding/json"` to `internal/host/agent_test.go`'s imports if it is not already there.
-
-- [ ] **Step 2: Run it to verify it fails**
-
-```bash
-go test -run 'TestSyncAcksEveryPass|TestRefreshConfigAck' ./internal/appconfig/
-go test -run TestConfigAckFuncRoutesARefresh ./internal/host/
-```
-
-Expected: build failure, `undefined: RefreshConfigAck`.
-
-- [ ] **Step 3: Write the code**
-
-`internal/appconfig/configack.go`, on `ConfigAck`:
-
-```go
- // Refresh marks an ack written by a pass that changed NOTHING: the file
- // on disk already matched what this pass rendered. It selects the writer
- // (RefreshConfigAck rather than WriteConfigAck) and is deliberately not
- // wire state — the object on the bucket describes a route, not which
- // code path last touched it.
- Refresh bool `json:"-"`
-```
-
-`internal/appconfig/configack.go` (append):
-
-```go
-// RefreshConfigAck rewrites the ack for a route a pass VISITED but did not
-// change, updating only what the agent is stating about itself — its schema
-// and the instance overlay it rendered — and carrying every delivery fact
-// from the existing ack forward.
-//
-// The carry-forward is the whole point. An ack is otherwise written only
-// when a file changes (see WriteConfigAck's callers), so a route whose
-// config has not moved since its agent was rolled has no ack at all, and a
-// reader cannot tell that apart from an agent too old to render the
-// instance config layer (senders design §6.5). Rewriting the ack every pass
-// closes that, but a rewrite that reported THIS pass's facts would report
-// Restarted: false for a route that really was restarted an hour ago, and
-// `carlos relay status` would go back to "written, awaiting delivery" for a
-// route it had already called adopted (internal/console/apirelay.go).
-//
-// Nothing to say is nothing written: when the existing ack already carries
-// this schema, this etag, this shape and this box, the function returns
-// after its read. Every converged route on the box comes through here on
-// every pass, and a Put each would be a write storm to restate a fact.
-//
-// A first ack (no object at all) is written as-is, which is how a route
-// that has never changed gets a statement at last.
-func RefreshConfigAck(ctx context.Context, s release.Store, ack ConfigAck) error {
- key := ConfigAckKey(ack.Box, ack.Host)
- var lastErr error
- for attempt := 0; attempt < 2; attempt++ {
- etag := ""
- body, curEtag, err := s.Get(ctx, key)
- switch {
- case errors.Is(err, os.ErrNotExist):
- // No ack yet: this pass's statement is the route's first.
- case err != nil:
- return err
- default:
- etag = curEtag
- var prev ConfigAck
- // An ack this binary cannot decode is replaced rather than
- // carried forward: there is nothing in it to preserve, and
- // leaving it would keep a reader unable to answer the question
- // this function exists to answer.
- if json.Unmarshal(body, &prev) == nil {
- if prev.Schema == ack.Schema && prev.InstanceConfigETag == ack.InstanceConfigETag &&
- prev.Shape == ack.Shape && prev.Box == ack.Box {
- return nil
- }
- ack.At = prev.At
- ack.Restarted, ack.RestartError, ack.Hibernating = prev.Restarted, prev.RestartError, prev.Hibernating
- ack.RelaySuffixes = prev.RelaySuffixes
- }
- }
- raw, err := json.Marshal(ack)
- if err != nil {
- return err
- }
- if _, err := s.Put(ctx, key, raw, etag); err != nil {
- lastErr = err
- if errors.Is(err, release.ErrCASConflict) {
- continue
- }
- return err
- }
- return nil
- }
- return lastErr
-}
-```
-
-`internal/appconfig/sync.go`, `syncHost`'s up-to-date return (`:433-435`):
-
-```go
- if existed && bytes.Equal(cur, rendered) {
- // Up to date, and still worth saying so ONCE per pass: an ack is
- // otherwise written only when a file changes, so a route whose config
- // has not moved would have no statement at all and absence would be
- // unreadable (senders design §6.5). The refresh carries this pass's
- // schema and overlay etag and keeps everything the delivering pass
- // said about the restart.
- if ack != nil {
- ack(ConfigAck{Schema: ConfigAckSchema, Host: rt.Host, Shape: shapeOf(rt), At: time.Now(),
- InstanceConfigETag: instETag, Refresh: true})
- }
- return // stay quiet on the periodic tick
- }
-```
-
-`internal/appconfig/syncexec.go`, `syncExecHost`'s (`:1016-1018`): the same block, with `dataDir`'s shape.
-
-```go
- if existed && bytes.Equal(cur, rendered) {
- // See sync.go's twin: an unchanged pass still states this agent's
- // schema and the overlay it rendered, and says nothing new about the
- // bounce.
- if ack != nil {
- ack(ConfigAck{Schema: ConfigAckSchema, Host: rt.Host, Shape: shapeOf(rt), At: time.Now(),
- InstanceConfigETag: instETag, Refresh: true})
- }
- return // up to date — stay quiet on the periodic tick
- }
-```
-
-`internal/host/agent.go`, `configAckFunc` (`:935-943`):
-
-```go
- return func(a appconfig.ConfigAck) {
- a.Box = box
- // Two doors, chosen by the pass that built the ack: a delivering pass
- // states everything afresh, while a pass that changed nothing
- // refreshes only what this agent says about itself and keeps the last
- // delivery's facts (appconfig.RefreshConfigAck).
- write := appconfig.WriteConfigAck
- if a.Refresh {
- write = appconfig.RefreshConfigAck
- }
- if err := write(ctx, store, a); err != nil {
- log.Printf("%s: config ack %s: %v", logPrefix, a.Host, err)
- }
- }
-```
-
-- [ ] **Step 4: Run it to verify it passes**
-
-```bash
-go test ./internal/appconfig/ ./internal/host/ -count=1
-```
-
-Expected: PASS.
-
-- [ ] **Step 5: Gate and commit**
-
-```bash
-gofmt -w internal/appconfig/configack.go internal/appconfig/sync.go internal/appconfig/syncexec.go internal/appconfig/configack_test.go internal/host/agent.go internal/host/agent_test.go
-go build ./... && go vet ./... && go test ./... -count=1 && gofmt -l .
-git add internal/appconfig internal/host
-git commit -m "appconfig: acknowledge every pass, carrying the last delivery's facts forward (plan 4, task 10)"
-```
-
----
-
-### Task 11: Reading the ack: delivered, stale, or never delivered
+### Task 10: Reading the ack: delivered, stale, or not yet delivered
**Files:**
- Modify: `internal/appconfig/configack.go` (`ConfigAckKey` at `:69`), `internal/appconfig/configack_test.go`
@@ -2633,13 +2312,13 @@ git commit -m "appconfig: acknowledge every pass, carrying the last delivery's f
InstanceConfigDelivered = "delivered"
InstanceConfigStale = "stale"
InstanceConfigUnknownAgent = "unknown-agent"
- InstanceConfigNeverDelivered = "never-delivered"
+ InstanceConfigNotYetDelivered = "not-yet-delivered"
)
func LatestConfigAck(ctx context.Context, s release.Store, host string) (*ConfigAck, error)
func InstanceConfigDelivery(a *ConfigAck, want string) string
```
-**Who calls this, and when.** Nothing in this plan. Step 5's instance `enable` refuses a box that cannot render the layer, and `carlos email status --instance <host>` prints delivered, stale or never delivered (design §6.4, §6.5); both are that step's doors. What lands here is the decidable part, with the four states named and tested, so step 5 spends its judgement on the refusal's wording rather than on re-deriving what an etag comparison means. The console's own `configAckPrefix` (`internal/console/relayops.go:73`) is the same string and is left alone: `relayConfigAcks` batches a whole app's routes through one listing and has no reason to change for this.
+**Who calls this, and when.** Nothing in this plan. Step 5's instance `enable` refuses a box that cannot render the layer, and `carlos email status --instance <host>` prints delivered, stale or not yet delivered (design §6.4, §6.5); both are that step's doors. What lands here is the decidable part, with the four states named and tested, so step 5 spends its judgement on the refusal's wording rather than on re-deriving what an etag comparison means. The console's own `configAckPrefix` (`internal/console/relayops.go:73`) is the same string and is left alone: `relayConfigAcks` batches a whole app's routes through one listing and has no reason to change for this.
- [ ] **Step 1: Write the failing test**
@@ -2657,7 +2336,7 @@ func TestInstanceConfigDeliveryVerdicts(t *testing.T) {
want string
got string
}{
- {"never acked", nil, "etag-1", InstanceConfigNeverDelivered},
+ {"not yet acked", nil, "etag-1", InstanceConfigNotYetDelivered},
{"agent older than the layer", &ConfigAck{Schema: 0, InstanceConfigETag: ""}, "etag-1", InstanceConfigUnknownAgent},
{"rendered this overlay", &ConfigAck{Schema: ConfigAckSchema, InstanceConfigETag: "etag-1"}, "etag-1", InstanceConfigDelivered},
{"rendered an older one", &ConfigAck{Schema: ConfigAckSchema, InstanceConfigETag: "etag-0"}, "etag-1", InstanceConfigStale},
@@ -2702,8 +2381,8 @@ func TestLatestConfigAckPicksTheFreshestForTheHost(t *testing.T) {
if err != nil || none != nil {
t.Fatalf("LatestConfigAck for a host with no ack = %+v %v, want (nil, nil)", none, err)
}
- if v := InstanceConfigDelivery(none, "etag-1"); v != InstanceConfigNeverDelivered {
- t.Fatalf("delivery = %q, want %q", v, InstanceConfigNeverDelivered)
+ if v := InstanceConfigDelivery(none, "etag-1"); v != InstanceConfigNotYetDelivered {
+ t.Fatalf("delivery = %q, want %q", v, InstanceConfigNotYetDelivered)
}
}
```
@@ -2727,7 +2406,7 @@ const ConfigAckPrefix = "control/config-ack/"
// ConfigAckKey is where a route's config-adoption ack lives: additive, one
// object per (box, route host), rewritten (never appended to) on every sync
-// pass — by WriteConfigAck when delivery changed, by RefreshConfigAck when
+// pass, by WriteConfigAck, when what it delivers changes
// it did not.
func ConfigAckKey(box, host string) string {
return ConfigAckPrefix + box + "/" + host + ".json"
@@ -2748,8 +2427,11 @@ const (
// config layer, so its ack says nothing about it. Not "no" — the answer
// cannot be had until the box is rolled.
InstanceConfigUnknownAgent = "unknown-agent"
- // InstanceConfigNeverDelivered: no ack for this host at all.
- InstanceConfigNeverDelivered = "never-delivered"
+ // InstanceConfigNotYetDelivered: no ack for this host at all. Acks are
+ // change-only, so this is the honest answer for a route whose file has
+ // not moved since its agent rolled. It never means "old agent": that
+ // question is the box record's agent version, which step 5 reads.
+ InstanceConfigNotYetDelivered = "not-yet-delivered"
)
// InstanceConfigDelivery compares an ack with the etag a write returned and
@@ -2763,7 +2445,7 @@ const (
func InstanceConfigDelivery(a *ConfigAck, want string) string {
switch {
case a == nil:
- return InstanceConfigNeverDelivered
+ return InstanceConfigNotYetDelivered
case a.Schema < ConfigAckSchema:
return InstanceConfigUnknownAgent
case a.InstanceConfigETag == want:
@@ -2807,8 +2489,8 @@ func LatestConfigAck(ctx context.Context, s release.Store, host string) (*Config
continue
}
if best == nil || a.At.After(best.At) {
- copy := a
- best = &copy
+ latest := a
+ best = &latest
}
}
return best, nil
@@ -2831,12 +2513,12 @@ Expected: PASS.
gofmt -w internal/appconfig/configack.go internal/appconfig/configack_test.go
go build ./... && go vet ./... && go test ./... -count=1 && gofmt -l .
git add internal/appconfig
-git commit -m "appconfig: read a route's ack and say delivered, stale or never delivered (plan 4, task 11)"
+git commit -m "appconfig: read a route's ack and say delivered, stale or not yet delivered (plan 4, task 10)"
```
---
-### Task 12: The signal, and one pass end to end
+### Task 11: The signal, and one pass end to end
**Files:**
- Modify: `internal/appconfig/appconfig.go` (the package's bucket layout at `:6-11`), `internal/appconfig/routefingerprint.go` (`ConfigRouteFingerprints`'s doc at `:34-57`), `internal/appconfig/instances_test.go`
@@ -3029,14 +2711,14 @@ Expected: PASS.
gofmt -w internal/appconfig/appconfig.go internal/appconfig/routefingerprint.go internal/appconfig/instances_test.go
go build ./... && go vet ./... && go test ./... -count=1 && gofmt -l .
git add internal/appconfig
-git commit -m "appconfig: pin the instance layer's signal and one pass over every shape of it (plan 4, task 12)"
+git commit -m "appconfig: pin the instance layer's signal and one pass over every shape of it (plan 4, task 11)"
```
---
## Self-review
-**Spec coverage.** §6.2's third bundle at `apps/<acct>/<app>/instances-config/<host>.json`, same manifest shape, same sealed envelopes, same name and value rules: tasks 1 and 2. §6.2's "`LoadMerged` keeps its signature; `LoadMergedHost` adds the third overlay": task 3. §6.2's "`Sync`/`SyncExec` call it per route after resolving the group base, so a pass costs one extra bucket read per instance that has an overlay and none for instances that do not": tasks 4 and 6, with the cost pinned by a counting store and the one deviation (the sweeps use the two functions `LoadMergedHost` is built from, not `LoadMergedHost` itself) stated in the header and pinned by an equivalence test. §6.2's "writing the layer bumps the app's generation object, as `SetExecDeliveryAt` does": task 2, with task 12 pinning that the route fingerprint does not move. §6.2's four guards: `lostKeys`/`defaultMissing` in task 5, the mass-removal guard in task 7, `AwaitingRestart` and `allShadowed` in task 8. §6.2's "`ExecDelivery` is not read from this layer": tasks 3 and 6. §6.2's "only the platform writes this layer: no CLI verb, no console door": nothing in this plan adds either, and the only callers of `ApplyInstance` in the tree when this plan ends are tests. §6.5's `Schema` and `InstanceConfigETag`: task 9. §6.5's "the agent rewrites the ack with its `Schema` on every pass, changed or not": task 10. §6.5's "the console compares the etag with the one its own `Put` returned": task 11 lands the comparison and the four states; the door that calls it is step 5's.
+**Spec coverage.** §6.2's third bundle at `apps/<acct>/<app>/instances-config/<host>.json`, same manifest shape, same sealed envelopes, same name and value rules: tasks 1 and 2. §6.2's "`LoadMerged` keeps its signature; `LoadMergedHost` adds the third overlay": task 3. §6.2's "`Sync`/`SyncExec` call it per route after resolving the group base, so a pass costs one extra bucket read per instance that has an overlay and none for instances that do not": tasks 4 and 6, with the cost pinned by a counting store and the one deviation (the sweeps use the two functions `LoadMergedHost` is built from, not `LoadMergedHost` itself) stated in the header and pinned by an equivalence test. §6.2's "writing the layer bumps the app's generation object, as `SetExecDeliveryAt` does": task 2, with task 12 pinning that the route fingerprint does not move. §6.2's four guards: `lostKeys`/`defaultMissing` in task 5, the mass-removal guard in task 7, `AwaitingRestart` and `allShadowed` in task 8. §6.2's "`ExecDelivery` is not read from this layer": tasks 3 and 6. §6.2's "only the platform writes this layer: no CLI verb, no console door": nothing in this plan adds either, and the only callers of `ApplyInstance` in the tree when this plan ends are tests. §6.5's `Schema` and `InstanceConfigETag`: task 9. §6.5's "acks stay change-only, and the capability question belongs to the box record's agent version": nothing here writes an extra ack, and the agent-version refusal is step 5's. §6.5's "the console compares the etag with the one its own `Put` returned": task 10 lands the comparison and the states; the door that calls it is step 5's.
**Out of scope and deliberately absent.** No instance senders, no `PerInstance` flag, no per-instance mode transition, no instance email doors, no `svcrole.Mail`, no uid or backing check at enable, no `claimDomain` change, no CLI verb, no console door, no mail package change of any kind. Nothing in this plan writes an overlay outside a test, so nothing in the fleet behaves differently after it rolls except that every converged route now leaves an ack behind on every pass.
@@ -3044,10 +2726,17 @@ git commit -m "appconfig: pin the instance layer's signal and one pass over ever
**The orphan question, and why it is step 5's.** Nothing deletes `instances-config/<host>.json` when an instance is retired. `release.PurgeApp`'s prefix sweep covers a deleted APP, since the key sits under `apps/<acct>/<app>/`, but a deleted INSTANCE leaves its overlay behind, holding sealed secrets, ready to be re-applied to whatever takes its host name next. That is a real hazard and it is not reachable from this plan: nothing here writes an overlay, so nothing here can orphan one. The sweep belongs beside the writer — step 5's instance teardown calls `DeleteInstanceConfig`, and the reconcile pass that retires a record is where a belt-and-braces sweep goes. This paragraph is the note that says so, and step 5's plan owes the task.
-**Cost, stated plainly.** Per sync pass, per (account, app) with routes on the box: one `List` of `instances-config/`. Per route with an overlay: one `Get`. Per converged route: one `Get` of its ack, and a `Put` only when the ack's content actually changes (task 10's steady-state skip). The hourly pass on a box with two hundred routes therefore adds two hundred small reads an hour and roughly no writes, which is the shape the fast loop already assumes.
+**Cost, stated plainly.** Per sync pass, per (account, app) with routes on the box: one `List` of `instances-config/`. Per route with an overlay: one `Get`. Per converged route whose delivered file CHANGED: one `Put` of its ack, exactly as today. A pass that changes nothing writes nothing and reads nothing extra beyond the one `List` per app, so a box with two hundred settled routes pays one listing per app per pass and no writes at all.
**Placeholder scan.** None. Every task carries its test code and its implementation code in full, including the three fixtures the tests share (`putInstanceRaw`, `countingStore`, `syncInstanceKit`), each defined in the task that first uses it. `execKit` (`internal/appconfig/syncexec_test.go:24`), `testRegistry` (`sync_test.go:13`), `captureStdout` (`syncexec_test.go:46`), `failingKeyStore` (`syncexec_test.go:71`), `addrRoute` (`awaitingrestart_test.go:14`) and `syncRelayTestKeys` (`configack_test.go:17`) already exist and are used as they are.
-**Type consistency.** `InstancesConfigPrefix`, `InstanceConfigKey`, `ValidInstanceHost`, `LoadInstanceConfig`, `ApplyInstance`, `DeleteInstanceConfig`, `LoadMergedHost`, `applyInstanceOverlay`, `instanceOverlays`, `instanceOverlayLoader`, `getIfExistsAtETag`, `allShadowed`, `ConfigAckSchema`, `ConfigAckPrefix`, `RefreshConfigAck`, `LatestConfigAck`, `InstanceConfigDelivery`, `InstanceConfigDelivered`, `InstanceConfigStale`, `InstanceConfigUnknownAgent` and `InstanceConfigNeverDelivered` are spelled identically in every task that produces or consumes them.
+**Type consistency.** `InstancesConfigPrefix`, `InstanceConfigKey`, `ValidInstanceHost`, `LoadInstanceConfig`, `ApplyInstance`, `DeleteInstanceConfig`, `LoadMergedHost`, `applyInstanceOverlay`, `instanceOverlays`, `instanceOverlayLoader`, `getIfExistsAtETag`, `allShadowed`, `ConfigAckSchema`, `ConfigAckPrefix`, `LatestConfigAck`, `InstanceConfigDelivery`, `InstanceConfigDelivered`, `InstanceConfigStale` and `InstanceConfigNotYetDelivered` are spelled identically in every task that produces or consumes them.
+
+**Two things worth saying plainly, because a reader will wonder.** The etag this plan puts in the ack is an S3 etag over sealed JSON, so it names a version without revealing anything about the values inside it; it is safe in a reply a member can read. And a flat exec route still shares the agent's uid, so every sibling on that box can read its `<host>.config.env` whatever this layer does — per-instance delivery is a scoping mechanism, not an isolation one, and the isolation is step 5's uid refusal plus #126's per-instance uids.
+
+**The roll note this plan owes.** Four lines.
-**The roll note this plan owes.** Two lines. Every config sync pass now rewrites a small ack object per converged route, so `control/config-ack/` sees reads on every pass and writes only when an agent's statement changes. And an ack written by a box still on an older agent decodes with `schema: 0`, which is the state step 5's enable refuses on, so boxes meant to host instance senders have to be rolled onto this release before that step can use them.
+1. **Every box's store policy needs `s3:ListBucket` on the deployment bucket before this rolls.** A pass now lists `instances-config/` once per app, and a listing that fails leaves every route of that app with the file it already has — correct, but it is a freeze, and it would hit routes that have no overlay at all. In-repo policies grant it unconditionally (`infra/modules/region-host/main.tf:174`) and boxes already list `apps/` during adoption (`internal/provision/reconcile.go:651`), so platform boxes are safe; a hand-edited or self-hosted policy might not be. Check before rolling, not after.
+2. **A fleet-door box cannot write config acks at all.** `control/config-ack/` is outside the door's read and write scopes (`internal/edge/storedoor.go:389`, `:489`), so delivery to an instance on such a box reads as unconfirmed however well it actually went. Granting the door `control/config-ack/<box>/` is its own piece of work; until then, instance senders should not be placed there.
+3. An ack written by a box still on an older agent decodes with `schema: 0`. Step 5's enable refuses on the box record's agent version rather than on that, but the field is what makes an old ack legible rather than confusing.
+4. Acks stay change-only, so this plan adds no new write pressure: a settled route writes nothing, exactly as today.