Skip to content

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

Merged
HyeockJinKim merged 12 commits into
mainfrom
feat/BA-6628-fragment-cascade
Jul 7, 2026
Merged

feat(BA-6704): cascade app config subtree deletion from the definition#12518
HyeockJinKim merged 12 commits into
mainfrom
feat/BA-6628-fragment-cascade

Conversation

@jopemachine

@jopemachine jopemachine commented Jul 3, 2026

Copy link
Copy Markdown
Member

Summary

Ties fragments to their allow-list entry with a composite FK and lets deletion cascade the whole subtree definition → allow_list → fragment (BEP-1052 review feedback):

  • Composite FK with ON DELETE CASCADE. app_config_fragments (config_name, scope_type) references app_config_allow_list, so purging an allow-list entry drops every fragment written under it — a revoked value disappears from the merge in the same statement, with no orphaned fragments to clean up separately.
  • Cascade from the definition, with exactly two FKs. app_config_allow_list.config_name → app_config_definitions changes from NO ACTION to ON DELETE CASCADE, and the redundant direct app_config_fragments.config_name → app_config_definitions FK is dropped (not made cascade). A fragment already requires a matching allow-list entry (composite FK, both columns NOT NULL) whose config_name is registered, so registration is guaranteed transitively — the direct FK added no integrity and only blocked the definition delete (a NO ACTION check firing before the allow-list cascade cleared the fragments). Result: retiring a config name (DELETE its definition row) cascades cleanly through allow_list down to the fragments.
  • Write-gate cleanup. With the composite FK in place, a fragment row can only exist while its allow-list entry does, so the only_if gates on fragment update/purge are provably dead and are removed (from the actions, service, repository, and db_source). The create gate stays — it surfaces the domain error (AppConfigFragmentWriteNotAllowed) instead of a raw FK violation.
  • Single migration a560420476b6: deletes orphaned fragments, adds the composite allow-list FK, swaps the allow-list→definition FK to CASCADE, and drops the redundant direct fragment→definition FK. Constraint names are discovered via the inspector (they differ between a migration-built DB and a metadata-built dev DB). Upgrade/downgrade round-trip and the live definition → allow_list → fragment cascade were verified against a local DB; the end state is exactly two FKs.
  • BEP-1052 updated: cascade semantics on allow-list purge and on definition retirement, the transitive-registration model (fragments need no direct definition FK), the corrected read model (the merge must join the allow-list for rank — the "reads never consult the allow-list" wording was stale), and the create-only write gate.

📚 Stacked PRs

Part of the AppConfigFragment / AppConfig stack under BEP-1052 (epic BA-5781). Merge in order:

  1. feat(BA-6552): add app_config_fragments DB model and Alembic migration #12306feat(BA-6552): app_config_fragments DB model and Alembic migration
  2. feat(BA-6553): add app_config_fragments repository layer #12307feat(BA-6553): repository layer
  3. refactor(BA-6619): consolidate AppConfigScopeType into common.data (single definition) #12403refactor(BA-6619): consolidate AppConfigScopeType into common.data
  4. refactor(BA-6620): ExistsQuerier ops primitive + AppConfigAllowList.exists #12405refactor(BA-6620): ExistsQuerier + AppConfigAllowList.exists
  5. feat(BA-6554): add app_config_fragment service layer #12358feat(BA-6554): AppConfigFragment service layer
  6. feat(BA-6702): move fragment rank to the allow list with scope defaults #12516feat(BA-6628): move fragment rank to the allow list with scope defaults (replaces feat(BA-6628): conditional-bulk repository primitives #12429)
  7. feat(BA-6701): expose allow-list rank on the v2 API surface #12517feat(BA-6628): expose allow-list rank on the v2 API surface
  8. 👉 feat(BA-6704): cascade app config subtree deletion from the definition #12518feat(BA-6704): cascade app config subtree deletion from the definition ← you are here
  9. feat(BA-6626): app_config_fragment bulk repository layer #12426feat(BA-6626): app_config_fragment bulk repository layer
  10. feat(BA-6618): app_config_fragment bulk CRUD service layer #12401feat(BA-6618): AppConfigFragment bulk CRUD service layer
  11. feat(BA-6555): AppConfig merge engine + resolve service (service layer) #12359feat(BA-6555): app_config service layer (merge engine)
  12. feat(BA-6556): AppConfig REST v2 API (raw fragments and merged read/update) #12377feat(BA-6556): AppConfig REST v2 API

🤖 Generated with Claude Code


📚 Documentation preview 📚: https://sorna--12518.org.readthedocs.build/en/12518/


📚 Documentation preview 📚: https://sorna-ko--12518.org.readthedocs.build/ko/12518/

@github-actions github-actions Bot added size:L 100~500 LoC comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated labels Jul 3, 2026
jopemachine added a commit that referenced this pull request Jul 3, 2026
@jopemachine jopemachine force-pushed the feat/BA-6628-fragment-cascade branch from 15c3f60 to c96d21f Compare July 3, 2026 06:37
@github-actions github-actions Bot added the area:docs Documentations label Jul 3, 2026
jopemachine added a commit that referenced this pull request Jul 3, 2026
@jopemachine jopemachine force-pushed the feat/BA-6628-fragment-cascade branch from 65f7b52 to 4602841 Compare July 3, 2026 07:39
@jopemachine jopemachine force-pushed the feat/BA-6628-allow-list-rank-api branch from 4afa2cb to ec8f884 Compare July 6, 2026 01:30
jopemachine added a commit that referenced this pull request Jul 6, 2026
@jopemachine jopemachine force-pushed the feat/BA-6628-fragment-cascade branch from 398066c to bdb60d0 Compare July 6, 2026 01:30
@jopemachine jopemachine force-pushed the feat/BA-6628-allow-list-rank-api branch from ec8f884 to 4afa2cb Compare July 6, 2026 01:49
@jopemachine jopemachine force-pushed the feat/BA-6628-fragment-cascade branch from bdb60d0 to 398066c Compare July 6, 2026 01:49
@jopemachine jopemachine force-pushed the feat/BA-6628-allow-list-rank-api branch from 4afa2cb to 2177b3f Compare July 6, 2026 02:27
jopemachine added a commit that referenced this pull request Jul 6, 2026
@jopemachine jopemachine force-pushed the feat/BA-6628-fragment-cascade branch from 398066c to b8cdd23 Compare July 6, 2026 02:34
Comment thread src/ai/backend/manager/repositories/app_config_fragment/db_source/db_source.py Outdated
@jopemachine jopemachine changed the title feat(BA-6628): cascade fragment deletion from the allow list feat(BA-6704): cascade fragment deletion from the allow list Jul 6, 2026
Base automatically changed from feat/BA-6628-allow-list-rank-api to main July 6, 2026 06:03
Resolve diverged alembic heads after merging origin/main: rebase
a560420476b6 onto e9a1c77b42d0 (add kernel resource_group_id column).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@jopemachine jopemachine requested a review from a team July 7, 2026 02:19
The fragment create path now relies on the FK to app_config_allow_list to
reject writes with no allow-list row (surfaced as AppConfigFragmentWriteNotAllowed
via the creator spec's integrity checks). With the only consumer gone, drop the
ExistsQuerier type and the exists() methods it fed (ReadOps, allow-list repository).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@github-actions github-actions Bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Jul 7, 2026
jopemachine and others added 3 commits July 7, 2026 13:14
With the write-gate gone, the repository no longer needs the raw spec. Accept a
Creator[AppConfigFragmentRow] in create() (repository + db_source), matching the
sibling allow-list/definition repositories; the service wraps the action's spec.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Removing ExistsQuerier from repositories/base invalidated the pants cache for
these tests, re-running fixtures that were only green from stale cache. Each
lists a partial with_tables/ORM subset, so SQLAlchemy's global
configure_mappers() fails on KernelRow.image_row -> "ImageRow" (reachable via
VFolderRow/RBAC relationships) because ImageRow was never registered.

Register ImageRow per fixture: the vfolder search tests add it (with its
ContainerRegistryRow FK parent) to with_tables; the role-scope test adds it to
the existing _ORM_CLUSTER registration tuple.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@HyeockJinKim HyeockJinKim merged commit e74d747 into main Jul 7, 2026
37 checks passed
@HyeockJinKim HyeockJinKim deleted the feat/BA-6628-fragment-cascade branch July 7, 2026 05:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Comment on lines +75 to +79
# 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"
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:docs Documentations comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated size:XL 500~ LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants