amadan
fix(domains): trashed apps with unstamped routes still drop — stamp first, then remove
Excluding trashed apps from the app→account linkage left a route with an empty account column reading as ops, so the removal lookup missed a marker written under the tenant's sqid: the app went on serving from its custom domain, and even minted itself an ops apex. The spec says ALL of them. Trashed apps are stamped like any other now — one write on a row that is about to be deleted, and the stamp is exactly what makes the removal match.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 files changed,
+33
−6
internal/domains/reconcile.go+10 −6internal/domains/reconcile_test.go+23 −0
diff --git a/internal/domains/reconcile.go b/internal/domains/reconcile.go| index bc4133abd..cda5ff5d9 100644 |
| --- a/internal/domains/reconcile.go |
| +++ b/internal/domains/reconcile.go |
| @@ -140,9 +140,9 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg |
| // believe must not destroy routes, so the app stays live and the log |
| // says why, every pass, until someone fixes or removes the object. |
| // |
| - // Read BEFORE the linkage below so a trashed app can be excluded from it: |
| - // nothing should be re-homing, minting for, or otherwise reasoning about |
| - // an app whose owner has already said stop. |
| + // Collected before the linkage below purely so the set is complete when |
| + // the removal pass (right after 1b's stamping) reads it. The linkage |
| + // itself does NOT skip trashed apps — see the stamping loop for why. |
| trashed := map[appKey]*release.AppTombstone{} |
| for _, key := range keys { |
| acct, app, prefix, ok := appKeyOwner(key, known) |
| @@ -176,13 +176,17 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg |
| if !ok { |
| continue |
| } |
| - if trashed[appKey{acct, app}] != nil { |
| - continue // a trashed app owns nothing here; its routes are going |
| - } |
| if !slices.Contains(owners[app], acct) { |
| owners[app] = append(owners[app], acct) |
| } |
| } |
| + // Trashed apps are stamped here like any other, deliberately: the stamp is |
| + // what makes the removal lookup below match. A route with an empty account |
| + // column resolves to ops until 1b writes the account the BUCKET asserts, |
| + // so skipping trashed apps here would leave such a row reading "ops" while |
| + // its marker sits under the tenant's sqid — and the app would go on |
| + // serving from its custom domain forever. Stamping a row that is about to |
| + // be deleted costs one write and buys "ALL of them". |
| for i := range routes { |
| if routes[i].App == "" { |
| continue // an app-less route (status page, bare proxy) has no account |
diff --git a/internal/domains/reconcile_test.go b/internal/domains/reconcile_test.go| index e4cbe8dc8..21cb9c3b6 100644 |
| --- a/internal/domains/reconcile_test.go |
| +++ b/internal/domains/reconcile_test.go |
| @@ -701,6 +701,29 @@ func TestReconcileTrashedAppDropsAllRoutesAndMintsNothing(t *testing.T) { |
| } |
| } |
| +// "ALL of them" includes a route that never got its account column stamped. |
| +// Such a row reads as ops until step 1b stamps it, so the removal lookup would |
| +// miss a marker written under the tenant's account — the app would keep |
| +// serving from its custom domain forever. The stamp has to land FIRST, from |
| +// the account the bucket actually asserts, and then the removal matches. |
| +func TestReconcileTrashedAppWithUnstampedRouteStillDrops(t *testing.T) { |
| + s, reg := dirStore(t), regWith(t, |
| + registry.Route{Host: "derp.custom", Kind: "static", App: "derp", |
| + Status: "ready", Channel: "stable"}, // no Account column at all |
| + ) |
| + sqid := writeAccountManifest(t, s, 793481920157, "Kass") |
| + putKey(t, s, "apps/"+sqid+"/derp/channels/stable.json") |
| + if err := release.TrashApp(ctx, s, sqid, "derp", "paul@keymail.dev", time.Unix(1753900000, 0)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := domains.Reconcile(ctx, s, reg, cfg(t)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if got := routesFor(t, reg, "derp"); len(got) != 0 { |
| + t.Errorf("unstamped route of a trashed app survived: %+v", got) |
| + } |
| +} |
| + |
| // A marker we cannot fully believe must not destroy routes: ReadAppTombstone |
| // errors, reconcile logs it and treats the app as live, every pass, until |
| // someone fixes or removes the object. |