amadan

carlos / platform Public

adopt: a listing failure is neither advice nor a healthy skip

The migrated-app check forced has=true on a listing failure so the
migrate advice could not be invented — correct — but the plain "channel
has no release yet" it printed instead was a claim the box never
established (realistic trigger: GetObject IAM without ListBucket). The
failure now rides the per-pass cache and is printed as "channel state
unknown" with the listing error attached.

Also pins the plain-skip assertion to the line ENDING: a bare Contains
on the prefix also matched the hint line, so the test's positive half
proved nothing about which branch printed (2026-08-03 review, both
minors from the APPROVE).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul Campbell f72cfe4d8b10f93ba3a2738b05515f95e97a925f parent 5b3293e
2 files changed, +30 −11
  • internal/release/adopt.go +23 −9
  • internal/release/qualified_test.go +7 −2
diff --git a/internal/release/adopt.go b/internal/release/adopt.go
index 9e9a31eaf..895ecf69d 100644
--- a/internal/release/adopt.go
+++ b/internal/release/adopt.go
@@ -117,7 +117,15 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551
// 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{}
+ // migratedState caches one pass's answer to "has this app anything under
+ // its qualified prefix", together with the listing failure when there was
+ // one — a failed listing must neither invent the migrate advice nor claim
+ // the healthy skip (2026-08-03 review).
+ type migratedState struct {
+ has bool
+ listErr error
+ }
+ migrated := map[acctApp]migratedState{}
for _, rt := range routes {
if !adopts(rt) {
continue
@@ -142,19 +150,25 @@ func PassFilter(ctx context.Context, reg *registry.Registry, s Store, pub ed2551
// 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]
+ st, cached := migrated[ak]
if !cached {
- var lerr error
- has, lerr = AppHasQualifiedObjects(ctx, s, k.acct, k.app)
+ 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
+ // is not evidence that an app never migrated. It is not
+ // evidence of a healthy skip either, so the failure rides
+ // along and is printed instead of a claim this box never
+ // established (realistic trigger: GetObject IAM without
+ // ListBucket).
+ st = migratedState{has: has || lerr != nil, listErr: lerr}
+ migrated[ak] = st
}
- if has {
+ switch {
+ case st.listErr != nil:
+ fmt.Printf(" %s/%s/%s: channel state unknown (could not list the app's prefix: %v)\n", k.acct, k.app, k.channel, st.listErr)
+ case st.has:
fmt.Printf(" %s/%s/%s: channel has no release yet\n", k.acct, k.app, k.channel)
- } else {
+ default:
fmt.Printf(" %s/%s/%s: channel has no release yet (%v)\n", k.acct, k.app, k.channel, err)
}
} else {
diff --git a/internal/release/qualified_test.go b/internal/release/qualified_test.go
index 2e740dcad..ea53feed1 100644
--- a/internal/release/qualified_test.go
+++ b/internal/release/qualified_test.go
@@ -325,8 +325,13 @@ func TestAdoptDoesNotSuggestMigrationForAMigratedApp(t *testing.T) {
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())
+ // The PLAIN line exactly — "channel has no release yet" followed by
+ // nothing. A bare Contains on the prefix would also match the hint line
+ // ("channel has no release yet (…migrate…)"), making the positive half of
+ // this test vacuous (2026-08-03 review): only pinning the line ending
+ // keeps it a real assertion about WHICH branch printed.
+ if !strings.Contains(out.String(), "channel has no release yet\n") {
+ t.Errorf("want the plain skip line and nothing appended to it, 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())