feat(BA-6704): cascade app config subtree deletion from the definition

Extend the cascade so retiring a config name (deleting its
app_config_definitions row) removes its allow-list entries and fragments
in one statement, via a single migration and exactly two FK constraints:

- app_config_allow_list.config_name -> app_config_definitions: CASCADE.
- Drop the redundant direct app_config_fragments.config_name ->
  app_config_definitions FK. A fragment already requires a matching
  allow-list entry (composite FK, NOT NULL) whose config_name is
  registered, so registration is guaranteed transitively; the direct FK
  only blocked the cascade. Fragments still cascade from their allow-list
  entry via the composite FK.

Folded into migration a560420476b6 (no separate migration). Cascade,
downgrade/upgrade round-trip, and the 2-FK end state verified against a
local DB. BEP-1052 updated to match.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: jopemachine

changes/12518.feature.md

@@ -1 +1 @@
-Cascade app config fragment deletion when their allow-list entry is purged (composite FK with `ON DELETE CASCADE`), and drop the now-redundant update/purge write gates
+Cascade app config deletion down the `definition → allow-list → fragment` subtree via `ON DELETE CASCADE` FKs, so a fragment can no longer exist without its allow-list entry

proposals/BEP-1052-scoped-app-config-redesign.md

@@ -66,12 +66,16 @@ Three scopes cover the use cases (`public` for the pre-login shell):
   `domain` (same-domain read / admin write), `user` (owner+admin read /
   owner-modify + admin write).
 - **Explicitly registered names.** Every config name lives in
-  `app_config_definitions`; fragments and allow-list entries reference it by
-  foreign key. No fragment may exist for an unregistered `config_name`.
-- **Reads are unconditional.** The merge reads `app_config_fragments`
-  joined to `app_config_allow_list` only for each fragment's `rank` — an
-  indexed `(config_name, scope_type)` join. No permission or policy is
-  evaluated at read time.
+  `app_config_definitions`; allow-list entries reference it by foreign key,
+  and fragments reference it transitively through their allow-list entry (a
+  fragment requires an entry, and the entry requires a registered name — so
+  no direct fragment FK is needed). No fragment may exist for an
+  unregistered `config_name`.
+- **Reads are unconditional.** The merge **must** join
+  `app_config_fragments` to `app_config_allow_list` — `rank` lives only on
+  the allow-list entry, so the ordering cannot be computed without it (an
+  indexed `(config_name, scope_type)` join). The join is for `rank` alone;
+  no permission or policy is evaluated at read time.
 - **Allow-list = the write gate and the merge order.** `app_config_allow_list`
   holds **one record per `(config_name, scope_type)`**; a fragment at
   that scope may be created **only if** the record exists — through the
@@ -150,18 +154,23 @@ purging one cascades to its fragments.
 
 ### Integrity
 
-- **Every** fragment create (admin or regular) requires (a) a registered
-  `config_name` (FK to `app_config_definitions`) and (b) an
+- **Every** fragment create (admin or regular) requires an
   `app_config_allow_list` row for the write's `(config_name,
   scope_type)` — enforced both by the write gate (domain error) and by
-  the composite FK. Updates and purges of an existing fragment need no
-  gate: the FK guarantees the entry exists while the fragment does.
+  the composite FK. That entry references a registered `config_name`, so
+  registration is guaranteed transitively; the fragment needs no direct
+  FK to `app_config_definitions`. Updates and purges of an existing
+  fragment need no gate: the FK guarantees the entry exists while the
+  fragment does.
 - A regular (non-admin) mutation is further restricted to the caller's
   own `user` row; admin mutations may target any scope (still gated by
   the allow-list) and are the only writes that may touch the allow-list
   and `app_config_definitions`.
-- `app_config_definitions` purge is rejected while any fragment or allow-list
-  entry still references the `config_name` (`ON DELETE NO ACTION`).
+- `app_config_definitions` purge **cascades down the whole subtree**: its
+  allow-list entries are removed by the `config_name` FK (`ON DELETE
+  CASCADE`), and their fragments cascade from those entries in turn — so
+  retiring a config name clears its allow-list and fragments in one
+  statement.
 - `app_config_allow_list` purge **revokes the grant and drops its data**:
   the fragments at that `(config_name, scope_type)` are removed by the
   `ON DELETE CASCADE` FK, so a revoked value disappears from the merge
@@ -307,8 +316,8 @@ likewise `null` — clients fall back to their built-in defaults.
   grant; the cascade drops the existing `user` fragments with it.
 - **Admin reorders contributions** — set the allow-list entries'
   `rank`s (per `(config_name, scope_type)`, not per fragment).
-- **Admin retires a config name** — purge the allow-list entries (their
-  fragments cascade), then the `app_config_definitions` row (purge is rejected
-  while references remain).
+- **Admin retires a config name** — purge the `app_config_definitions`
+  row; the allow-list entries and their fragments cascade with it, so no
+  prior cleanup is needed.
 - **Admin audit** — cross-scope fragment search and cross-user merged
   search for support.

src/ai/backend/manager/models/alembic/versions/a560420476b6_cascade_fragment_deletion_from_allow_list.py

@@ -1,11 +1,17 @@
-"""cascade fragment deletion from app_config_allow_list
+"""cascade app config subtree deletion
 
-Add a composite FK from ``app_config_fragments (config_name, scope_type)`` to
-``app_config_allow_list`` with ``ON DELETE CASCADE`` (BEP-1052): removing an
-allow-list entry removes every fragment written under it, so a revoked value
-disappears from the merge in the same statement. Fragments whose allow-list
-entry was already removed (previously kept and still merged) are deleted before
-the FK is added — the same outcome the cascade would have produced.
+Cascade deletion down ``definition -> allow_list -> fragment`` (BEP-1052):
+
+1. Composite FK ``app_config_fragments (config_name, scope_type) ->
+   app_config_allow_list`` ``ON DELETE CASCADE`` (orphaned fragments deleted first).
+2. ``app_config_allow_list.config_name -> app_config_definitions`` -> ``CASCADE``.
+3. Drop the redundant direct ``app_config_fragments.config_name ->
+   app_config_definitions`` FK — the composite FK already guarantees a registered
+   ``config_name`` transitively, and this FK only blocked the definition delete.
+
+The released allow-list creation migration entered a wrong (non-convention) name
+for the definitions FK, so its name differs between DBs; we drop both candidate
+names with ``IF EXISTS``.
 
 Revision ID: a560420476b6
 Revises: 66d0f891ed20
@@ -25,8 +31,7 @@
 
 
 def upgrade() -> None:
-    # Fragments without an allow-list entry are unrepresentable once the FK exists;
-    # remove them as the cascade would have when the entry was purged.
+    # 1. fragment -> allow_list composite FK (drop pre-existing orphans first).
     op.execute(
         sa.text(
             """
@@ -48,9 +53,53 @@ def upgrade() -> None:
         ["config_name", "scope_type"],
         ondelete="CASCADE",
     )
+    # 2. allow_list -> definitions: NO ACTION -> CASCADE.
+    # The released creation migration entered a wrong (non-convention) FK name, so the
+    # actual name differs by DB — drop whichever exists before recreating.
+    op.execute(
+        "ALTER TABLE app_config_allow_list "
+        "DROP CONSTRAINT IF EXISTS fk_app_config_allow_list_config_name"
+    )
+    op.execute(
+        "ALTER TABLE app_config_allow_list "
+        "DROP CONSTRAINT IF EXISTS fk_app_config_allow_list_config_name_app_config_definitions"
+    )
+    op.create_foreign_key(
+        "fk_app_config_allow_list_config_name_app_config_definitions",
+        "app_config_allow_list",
+        "app_config_definitions",
+        ["config_name"],
+        ["config_name"],
+        ondelete="CASCADE",
+    )
+    # 3. fragments -> definitions: drop the redundant direct FK (it blocks the cascade).
+    op.execute(
+        "ALTER TABLE app_config_fragments "
+        "DROP CONSTRAINT IF EXISTS fk_app_config_fragments_config_name_app_config_definitions"
+    )
 
 
 def downgrade() -> None:
+    op.create_foreign_key(
+        "fk_app_config_fragments_config_name_app_config_definitions",
+        "app_config_fragments",
+        "app_config_definitions",
+        ["config_name"],
+        ["config_name"],
+        ondelete="NO ACTION",
+    )
+    op.execute(
+        "ALTER TABLE app_config_allow_list "
+        "DROP CONSTRAINT IF EXISTS fk_app_config_allow_list_config_name_app_config_definitions"
+    )
+    op.create_foreign_key(
+        "fk_app_config_allow_list_config_name_app_config_definitions",
+        "app_config_allow_list",
+        "app_config_definitions",
+        ["config_name"],
+        ["config_name"],
+        ondelete="NO ACTION",
+    )
     op.drop_constraint(
         "fk_app_config_fragments_config_name_scope_type",
         "app_config_fragments",

src/ai/backend/manager/models/app_config_allow_list/row.py

@@ -18,13 +18,14 @@
 class AppConfigAllowListRow(Base):  # type: ignore[misc]
     """Permission to write config fragments for one config name at one scope type.
 
-    A config fragment may be created only when a matching row exists here; without
-    one, the write is rejected. Fragments reference this table by
-    ``(config_name, scope_type)`` with ``ON DELETE CASCADE``, so removing an entry
-    also removes every fragment written under it. ``rank`` is the merge priority the
-    entry's fragments carry (low → high; higher wins) — admin-owned here so fragment
-    owners cannot re-order the merge. There is at most one row per
-    ``(config_name, scope_type)``, and only admins create or remove these rows.
+    A config fragment may be created only when a matching row exists here.
+    Deletion cascades both ways: fragments reference this table by
+    ``(config_name, scope_type)`` and this table references
+    ``app_config_definitions`` by ``config_name``, both ``ON DELETE CASCADE`` —
+    so retiring a config name clears its entries and fragments. ``rank`` is the
+    merge priority the entry's fragments carry (low → high; higher wins) —
+    admin-owned so fragment owners cannot re-order the merge. At most one row per
+    ``(config_name, scope_type)``; only admins create or remove these rows.
     """
 
     __tablename__ = "app_config_allow_list"
@@ -43,7 +44,7 @@ class AppConfigAllowListRow(Base):  # type: ignore[misc]
     config_name: Mapped[str] = mapped_column(
         "config_name",
         sa.String(length=128),
-        sa.ForeignKey("app_config_definitions.config_name", ondelete="NO ACTION"),
+        sa.ForeignKey("app_config_definitions.config_name", ondelete="CASCADE"),
         nullable=False,
     )
     scope_type: Mapped[AppConfigScopeType] = mapped_column(

src/ai/backend/manager/models/app_config_definition/row.py

@@ -13,7 +13,11 @@
 
 
 class AppConfigDefinitionRow(Base):  # type: ignore[misc]
-    """One registered ``config_name`` (admin-managed)."""
+    """One registered ``config_name`` (admin-managed).
+
+    Purging a row cascades to its allow-list entries (``ON DELETE CASCADE``) and,
+    through them, to their fragments.
+    """
 
     __tablename__ = "app_config_definitions"
 

src/ai/backend/manager/models/app_config_fragment/row.py

@@ -20,10 +20,8 @@
 class AppConfigFragmentRow(Base):  # type: ignore[misc]
     """One scoped app config fragment — a single JSON document at ``(config_name, scope_type, scope_id)``.
 
-    A fragment hangs off its ``app_config_allow_list`` entry (FK on
-    ``(config_name, scope_type)``, ``ON DELETE CASCADE``): it exists only while the
-    entry does, and its merge priority is the entry's ``rank`` — the fragment row
-    carries no rank of its own.
+    A fragment's merge priority is its allow-list entry's ``rank`` — the fragment
+    row carries no rank of its own.
     """
 
     __tablename__ = "app_config_fragments"
@@ -51,7 +49,6 @@ class AppConfigFragmentRow(Base):  # type: ignore[misc]
     config_name: Mapped[str] = mapped_column(
         "config_name",
         sa.String(length=128),
-        sa.ForeignKey("app_config_definitions.config_name", ondelete="NO ACTION"),
         nullable=False,
     )
     scope_type: Mapped[AppConfigScopeType] = mapped_column(

tests/unit/manager/repositories/app_config_allow_list/test_repository.py

@@ -267,6 +267,42 @@ async def test_purge_cascades_to_fragments(
             )
         assert remaining == 0
 
+    async def test_definition_purge_cascades_to_allow_list_and_fragments(
+        self,
+        database: ExtendedAsyncSAEngine,
+        repository: AppConfigAllowListRepository,
+        definition_repository: AppConfigDefinitionRepository,
+    ) -> None:
+        # Deleting the definition cascades to its allow-list entry and its fragment.
+        definition = await definition_repository.create(
+            Creator(spec=AppConfigDefinitionCreatorSpec(config_name="theme"))
+        )
+        entry = await _create_entry(repository, "theme", AppConfigScopeType.PUBLIC)
+        async with database.begin_session() as db_sess:
+            db_sess.add(
+                AppConfigFragmentRow(
+                    config_name=entry.config_name,
+                    scope_type=entry.scope_type,
+                    scope_id="public",
+                    config={"k": "v"},
+                )
+            )
+            await db_sess.flush()
+
+        await definition_repository.purge(
+            Purger(row_class=AppConfigDefinitionRow, pk_value=definition.id)
+        )
+
+        async with database.begin_readonly_session() as db_sess:
+            remaining_entries = await db_sess.scalar(
+                sa.select(sa.func.count()).select_from(AppConfigAllowListRow)
+            )
+            remaining_fragments = await db_sess.scalar(
+                sa.select(sa.func.count()).select_from(AppConfigFragmentRow)
+            )
+        assert remaining_entries == 0
+        assert remaining_fragments == 0
+
 
 class TestSearch:
     async def test_search_returns_all_with_total_count(