amadan
adopt: reserve the migrate advice for apps that actually need it
A missing channel pointer has two causes and only one is a migration. Adopt was printing the full migrate paragraph for both — so kass, which follows stable on the flagship and has no stable pointer until the bake window ends on Aug 5, would have been told hourly to migrate an app that is fully migrated. Advice that fires every hour on a healthy fleet is advice nobody reads, and the hint is worth reading. Adopt now applies AppHasQualifiedObjects, the rule PromoteOnLadder already uses, hoisted so there is one rule and two callers: nothing under the app's qualified prefix is the deployment that upgraded without migrating and gets the store's error, which names the verb; anything under it gets the plain "channel has no release yet". A listing that FAILS suppresses the advice rather than inventing it — a bucket the box cannot list is not evidence that an app never migrated. Cached per (account, app) for the pass, so a box following several of an app's channels pays for the distinction once, not once per route. Pinned: the kass shape verbatim gets no hint, an unmigrated app still does, and three channels of one app cost exactly one listing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5 files changed,
+211
−17
docs/superpowers/specs/2026-08-02-multi-account-design.md+16 −4internal/release/adopt.go+31 −7internal/release/qualified.go+23 −0internal/release/qualified_test.go+137 −0internal/release/release.go+4 −6
diff --git a/docs/superpowers/specs/2026-08-02-multi-account-design.md b/docs/superpowers/specs/2026-08-02-multi-account-design.md| index 641ca0416..530998f0e 100644 |
| --- a/docs/superpowers/specs/2026-08-02-multi-account-design.md |
| +++ b/docs/superpowers/specs/2026-08-02-multi-account-design.md |
| @@ -405,10 +405,22 @@ job, not a side effect of the next deploy — and it is why a miss names that |
| verb. In exchange, promote and rollback hold a real etag for the object they |
| write, so the CAS is honest rather than a create-only workaround. |
| -**Adopt skips, loudly.** A legacy-only app leaves adopt printing the store's |
| -error — which carries the hint — and moving on. It never fails the pass: one |
| -app's missing pointer must not stop every other app on the box from |
| -installing, which is already how a channel with nothing promoted behaves. |
| +**Adopt skips, loudly — but only where the advice applies.** A legacy-only app |
| +leaves adopt printing the store's error, which carries the hint, and moving on. |
| +It never fails the pass: one app's missing pointer must not stop every other |
| +app on the box from installing, which is already how a channel with nothing |
| +promoted behaves. |
| + |
| +A missing pointer has two causes and only one of them is a migration. A |
| +MIGRATED app waiting on its first promotion to a channel is ordinary — kass |
| +follows `stable` on the flagship, whose pointer does not exist until the bake |
| +window ends — and an hourly pass that told it to migrate would be wrong every |
| +hour for days. So adopt applies `AppHasQualifiedObjects`, the same rule |
| +`PromoteOnLadder` uses: nothing under the qualified prefix gets the hint, |
| +anything under it gets the plain "channel has no release yet". One listing per |
| +(account, app) per pass, cached; a listing that FAILS suppresses the advice |
| +rather than inventing it, since a bucket the box cannot list is not evidence |
| +an app never migrated. |
| **Deleting the legacy objects is still a separate operator step.** No code |
| path has ever removed one and none does now. They are inert. |
diff --git a/internal/release/adopt.go b/internal/release/adopt.go| index 0473c7e96..9e9a31eaf 100644 |
| --- a/internal/release/adopt.go |
| +++ b/internal/release/adopt.go |
| @@ -113,6 +113,11 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551 |
| // silently install it on both. |
| type groupKey struct{ acct, app, channel string } |
| pointers := map[groupKey]*ChannelPointer{} |
| + // Whether an app has anything under its qualified prefix, cached for the |
| + // pass: one listing per (account, app), not one per channel. It decides |
| + // which of the two missing-pointer lines below an app gets. |
| + type acctApp struct{ acct, app string } |
| + migrated := map[acctApp]bool{} |
| for _, rt := range routes { |
| if !adopts(rt) { |
| continue |
| @@ -126,13 +131,32 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551 |
| pointers[k] = nil // resolved-and-failed: don't retry per host |
| if errors.Is(err, os.ErrNotExist) { |
| // SKIP, never abort: one app with no pointer must not stop |
| - // every other app on the box from installing. Since the |
| - // read-fallback was retired an app still on legacy keys lands |
| - // here too, so the store's error is printed rather than |
| - // swallowed — it names `accounts migrate -from legacy`, which |
| - // is the difference between "nothing promoted yet" and "this |
| - // deployment never migrated". |
| - fmt.Printf(" %s/%s/%s: channel has no release yet (%v)\n", k.acct, k.app, k.channel, err) |
| + // every other app on the box from installing. |
| + // |
| + // Two deployments produce a missing pointer, and only one of |
| + // them should be told to migrate. A channel with nothing |
| + // promoted yet is ordinary — a migrated app waiting on its |
| + // first stable promotion sits here for days — and gets the |
| + // plain line. An app with NOTHING under its qualified prefix |
| + // upgraded without migrating, and gets the store's error, |
| + // which names the verb. Advice that fires every hour on a |
| + // healthy fleet is advice nobody reads. |
| + ak := acctApp{k.acct, k.app} |
| + has, cached := migrated[ak] |
| + if !cached { |
| + var lerr error |
| + has, lerr = AppHasQualifiedObjects(ctx, s, k.acct, k.app) |
| + // A listing we could not perform suppresses the advice |
| + // rather than inventing it: a bucket this box cannot list |
| + // is not evidence that an app never migrated. |
| + has = has || lerr != nil |
| + migrated[ak] = has |
| + } |
| + if has { |
| + fmt.Printf(" %s/%s/%s: channel has no release yet\n", k.acct, k.app, k.channel) |
| + } else { |
| + fmt.Printf(" %s/%s/%s: channel has no release yet (%v)\n", k.acct, k.app, k.channel, err) |
| + } |
| } else { |
| fmt.Printf(" %s/%s/%s: channel pointer rejected: %v\n", k.acct, k.app, k.channel, err) |
| } |
diff --git a/internal/release/qualified.go b/internal/release/qualified.go| index 10e226653..2fdb1f685 100644 |
| --- a/internal/release/qualified.go |
| +++ b/internal/release/qualified.go |
| @@ -129,6 +129,29 @@ func migrationHint(key string) string { |
| legacyAppPrefix(app), app, acct) |
| } |
| +// AppHasQualifiedObjects reports whether app has ANYTHING under its |
| +// account-qualified prefix. It is the whole distinction between the two |
| +// deployments a missing object can mean: |
| +// |
| +// - nothing there at all — this deployment upgraded from a pre-accounts |
| +// binary and never migrated. `accounts migrate -from legacy` is the fix, |
| +// and saying so is the point of the hint. |
| +// - something there, but not this key — ordinary. A migrated app waiting on |
| +// its first promotion to a channel is in exactly this state, and telling |
| +// it to migrate would be wrong every hour of every day until somebody |
| +// promotes. |
| +// |
| +// "Anything at all" rather than "any releases": an app that has shipped since |
| +// accounts landed is not unmigrated however new the channel is. Both readings |
| +// come from the qualified prefix; neither touches a legacy key. |
| +// |
| +// Callers that ask per channel should cache it per (account, app) — one |
| +// listing answers for every channel an app has. |
| +func AppHasQualifiedObjects(ctx context.Context, s Store, acct, app string) (bool, error) { |
| + keys, err := s.List(ctx, AppPrefix(acct, app)) |
| + return len(keys) > 0, err |
| +} |
| + |
| // GetMigrationSource reads key, and on NOT-FOUND reads legacy instead. It is |
| // the shape the retired read-fallback had, kept for the ONE caller whose |
| // access to the account-less key space is explicit and permanent: `carlos |
diff --git a/internal/release/qualified_test.go b/internal/release/qualified_test.go| index b3cff27b9..2e740dcad 100644 |
| --- a/internal/release/qualified_test.go |
| +++ b/internal/release/qualified_test.go |
| @@ -1,12 +1,16 @@ |
| package release |
| import ( |
| + "bytes" |
| "context" |
| "encoding/json" |
| "errors" |
| + "fmt" |
| "os" |
| "strings" |
| "testing" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| ) |
| // erroringStore is a Store whose every operation fails with a NON-not-found |
| @@ -289,6 +293,139 @@ func TestGetMigrationSourceStillReadsLegacy(t *testing.T) { |
| } |
| } |
| +// A MIGRATED app waiting on its first promotion to a channel is ordinary, |
| +// and must not be told to migrate. This is kass's live shape on the flagship: |
| +// fully migrated to its own account, with a route following stable whose |
| +// pointer does not exist until the bake window ends — so the hourly adopt |
| +// would otherwise nag about migrating an app that is already migrated, every |
| +// hour, forever. |
| +// |
| +// The distinction is the same one PromoteOnLadder makes, and it is computed |
| +// from the qualified prefix alone: nothing there at all means the deployment |
| +// never migrated; anything there means this channel simply has no pointer yet. |
| +func TestAdoptDoesNotSuggestMigrationForAMigratedApp(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt, reg := t.TempDir(), testRegistry(t) |
| + |
| + // Migrated and live on its own account — just not on stable yet. |
| + shipRelease(t, s, "bad", "kass", "v1", adoptPlatform, []byte("build one")) |
| + if err := Promote(ctx, s, priv, "bad", "kass", "canary/x", "v1"); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := reg.Upsert(registry.Route{Host: "kass.bad.oncarlos.test", Kind: "instance", |
| + App: "kass", Account: "bad", Channel: "stable"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + var out bytes.Buffer |
| + withCapturedStdout(t, &out, func() { |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Errorf("Pass: %v", err) |
| + } |
| + }) |
| + if !strings.Contains(out.String(), "channel has no release yet") { |
| + t.Errorf("want the plain skip line, got:\n%s", out.String()) |
| + } |
| + if strings.Contains(out.String(), "accounts migrate") { |
| + t.Errorf("a migrated app was told to migrate:\n%s", out.String()) |
| + } |
| +} |
| + |
| +// countingStore counts listings of one exact prefix. |
| +type countingStore struct { |
| + Store |
| + prefix string |
| + n int |
| +} |
| + |
| +func (c *countingStore) List(ctx context.Context, prefix string) ([]string, error) { |
| + if prefix == c.prefix { |
| + c.n++ |
| + } |
| + return c.Store.List(ctx, prefix) |
| +} |
| + |
| +// The check that decides the line costs one listing per (account, app) per |
| +// pass, not one per channel. A box following several of an app's channels — |
| +// the ordinary shape for a canary fleet — must not pay for the distinction |
| +// once per route. |
| +func TestAdoptListsAnAppsPrefixOncePerPass(t *testing.T) { |
| + dir := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt, reg := t.TempDir(), testRegistry(t) |
| + |
| + shipRelease(t, dir, "bad", "kass", "v1", adoptPlatform, []byte("build one")) |
| + if err := Promote(ctx, dir, priv, "bad", "kass", "canary/x", "v1"); err != nil { |
| + t.Fatal(err) |
| + } |
| + // Three routes, three channels with no pointer, one app. |
| + for i, ch := range []string{"stable", "beta", "edge"} { |
| + if err := reg.Upsert(registry.Route{Host: fmt.Sprintf("h%d.example.com", i), |
| + Kind: "instance", App: "kass", Account: "bad", Channel: ch}); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + |
| + s := &countingStore{Store: dir, prefix: AppPrefix("bad", "kass")} |
| + var out bytes.Buffer |
| + withCapturedStdout(t, &out, func() { |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Errorf("Pass: %v", err) |
| + } |
| + }) |
| + if s.n != 1 { |
| + t.Errorf("listed %s %d times in one pass, want 1", s.prefix, s.n) |
| + } |
| + if strings.Contains(out.String(), "accounts migrate") { |
| + t.Errorf("a migrated app was told to migrate:\n%s", out.String()) |
| + } |
| +} |
| + |
| +// The other side of the same line: an app with NOTHING under its qualified |
| +// prefix is the deployment that upgraded without migrating, and it gets the |
| +// verb by name. Reserving the advice is what makes it worth reading. |
| +func TestAdoptSuggestsMigrationForAnUnmigratedApp(t *testing.T) { |
| + dir := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt, reg := t.TempDir(), testRegistry(t) |
| + |
| + scratch := NewDirStore(t.TempDir()) |
| + shipRelease(t, scratch, OpsAccount, "old", "v1", adoptPlatform, []byte("build one")) |
| + if err := Promote(ctx, scratch, priv, OpsAccount, "old", "stable", "v1"); err != nil { |
| + t.Fatal(err) |
| + } |
| + keys, err := scratch.List(ctx, AppPrefix(OpsAccount, "old")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, k := range keys { |
| + body, _, err := scratch.Get(ctx, k) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + legacy := LegacyAppPrefix("old") + strings.TrimPrefix(k, AppPrefix(OpsAccount, "old")) |
| + if _, err := dir.Put(ctx, legacy, body, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + |
| + s := &legacyTripwire{Store: dir, t: t, app: "old"} |
| + addRoute(t, reg, "old.example.com", "old", "stable", false) |
| + var out bytes.Buffer |
| + withCapturedStdout(t, &out, func() { |
| + if err := Pass(ctx, reg, s, pub, opt, func(string) error { return nil }); err != nil { |
| + t.Errorf("Pass: %v", err) |
| + } |
| + }) |
| + if !strings.Contains(out.String(), "carlos accounts migrate -app old -from legacy -to bab") { |
| + t.Errorf("an unmigrated app was not told how to migrate:\n%s", out.String()) |
| + } |
| +} |
| + |
| // Promoting an app on a deployment that never migrated must say SO. Left to |
| // the ladder, the answer is "stable is entered from beta only, and this build |
| // is on no rung yet" — true (nothing qualified means no rung) and entirely |
diff --git a/internal/release/release.go b/internal/release/release.go| index 42067a922..9964b9cea 100644 |
| --- a/internal/release/release.go |
| +++ b/internal/release/release.go |
| @@ -619,13 +619,11 @@ func PromoteOnLadder(ctx context.Context, s Store, priv ed25519.PrivateKey, acct |
| // read cannot say it either: an absent pointer is the legitimate first |
| // promotion of a fresh channel, so the miss has to stay silent there. |
| // |
| - // "Nothing qualified AT ALL" rather than "no qualified releases": an app |
| - // that has shipped since accounts landed is not unmigrated, however new |
| - // this channel is, and must not be told to migrate. Both readings are |
| - // computed from the qualified prefix — no legacy key is touched. |
| - if objs, err := s.List(ctx, AppPrefix(acct, app)); err != nil { |
| + // AppHasQualifiedObjects is the same test adopt applies to decide whether |
| + // a missing pointer deserves the advice — one rule, two callers. |
| + if has, err := AppHasQualifiedObjects(ctx, s, acct, app); err != nil { |
| return res, fmt.Errorf("promote: list %s: %w", AppPrefix(acct, app), err) |
| - } else if len(objs) == 0 { |
| + } else if !has { |
| return res, fmt.Errorf("promote: nothing has ever been shipped to %s under account %s%s", |
| app, acct, migrationHint(AppPrefix(acct, app))) |
| } |