fix(budget): cascade-delete orphaned categories when deleting a budget

docker-compose.dev.yml

@@ -27,7 +27,7 @@ services:
       retries: 5
 
   minio:
-    image: quay.io/minio/minio:latest
+    image: pgsty/minio:RELEASE.2026-08-04T00-00-00Z
     container_name: grandflow-minio
     command: server /data --console-address ":9001"
     ports:

docker-compose.local.yml

@@ -30,7 +30,7 @@ services:
       - grandflow
 
   minio:
-    image: quay.io/minio/minio:latest
+    image: pgsty/minio:RELEASE.2026-08-04T00-00-00Z
     container_name: grandflow-minio-local
     command: server /data --console-address ":9001"
     ports:

openspec/changes/budget-fix-295-category-cascade-delete-fix/design.md

@@ -8,26 +8,37 @@ See proposal.md - Why for the failure mode. Relevant existing state:
 - `BudgetModel.categories` (`services/budget/app/models/budget.py`) is a plain `relationship(..., back_populates="budget")` with no `cascade` and no `passive_deletes` setting.
 - `delete_budget_service` (`services/budget/app/services/budget_services.py`) calls `get_budget_service`, which does **not** eager-load `categories` in this path, then calls `session.delete(budget)` + `commit()`.
 - Without `passive_deletes=True`, SQLAlchemy's unit-of-work treats "this budget is being deleted" as "I must disassociate its `categories` collection" — since there's no delete cascade configured, it lazy-loads the (unloaded) collection during flush and issues an `UPDATE budget_categories SET budget_id = NULL ...` for each row, which the DB rejects (`budget_id` is `nullable=False`), surfacing as `IntegrityError` → the generic 400.
+- The `minio` service in `docker-compose.local.yml` (used by the `E2E Tests` workflow) and `docker-compose.dev.yml` uses `quay.io/minio/minio:latest`. That image now needs authentication (401), and `minio/minio` no longer exists on Docker Hub (404). Developer machines still have a cached copy, but a fresh CI runner can't pull one, so e2e fails before any test runs.
 - Because the collection load happens inside `flush()`, which the SQLAlchemy asyncio extension runs inside a greenlet, this doesn't crash as a `MissingGreenlet` — it produces a normal `IntegrityError`, so it slipped past the `async-persistence` capability's existing "no implicit lazy-load" guard without tripping any greenlet-specific detection.
 
 ## Goals / Non-Goals
 
 **Goals:**
 - Make `DELETE /api/v1/budgets/{id}` succeed for a budget whose only remaining "problem" is a category row (no lines, reports, receipts, or currency conversions).
 - Keep the DB as the single source of truth for the cascade, rather than teaching the ORM to duplicate it.
+- Restore a pullable MinIO image so the `E2E Tests` workflow can verify the fix on a fresh runner.
 
 **Non-Goals:**
 - Cleaning up orphaned `budget_categories` rows when a line is deleted (separate, pre-existing bookkeeping gap; not required for this fix — see proposal.md).
 - Changing cascade behavior for `budgets.lines` or `budgets.reports` — those intentionally block budget deletion via `IntegrityError` today (a budget with real lines/reports/receipts should not be silently hard-deletable), and that behavior is unaffected by this change.
 
 ## Decisions
 
-**Use `passive_deletes=True` on `BudgetModel.categories`, not ORM-level `cascade="all, delete-orphan"`.**
-- `passive_deletes=True` tells SQLAlchemy "don't manage this relationship's deletes yourself, the DB's `ON DELETE CASCADE` already does it" — it skips the lazy-load-and-null-out step entirely and just issues `DELETE FROM budgets WHERE id = ...`, letting Postgres cascade.
-- Alternative considered: `cascade="all, delete-orphan"`. This would make SQLAlchemy load the collection and emit an explicit `DELETE FROM budget_categories WHERE id IN (...)` before deleting the budget — functionally similar end state, but it still requires the lazy-load (same eager-load discipline the `async-persistence` capability calls for) and does redundant work the DB already does via its own `ON DELETE CASCADE`. `passive_deletes=True` is the smaller, more precise fix that matches the existing DB-level intent from migration `000014`.
-- `passive_deletes=True` requires the DB constraint to actually be `ON DELETE CASCADE` (true here, confirmed in the migration) — it would be the wrong choice if the constraint were `RESTRICT`/`NO ACTION`, since then rows would just silently fail to delete.
+**Use `cascade="all, delete-orphan", passive_deletes=True` together on `BudgetModel.categories`, not `passive_deletes=True` alone.**
+- `passive_deletes=True` tells SQLAlchemy "don't manage this relationship's deletes yourself, the DB's `ON DELETE CASCADE` already does it" — when the collection is *unloaded* (true today, since `get_budget` doesn't eager-load `categories`), it skips the lazy-load-and-null-out step entirely and just issues `DELETE FROM budgets WHERE id = ...`, letting Postgres cascade.
+- `passive_deletes=True` alone is not sufficient by itself, though: it only changes what SQLAlchemy does with an *unmanaged* collection. If `categories` is already loaded on the instance being deleted (e.g. a future caller adds `selectinload(BudgetModel.categories)` to `get_budget`, or otherwise touches the collection before `session.delete()`), plain `passive_deletes=True` still walks the already-materialized collection and nulls out each child's `budget_id`, hitting the same `IntegrityError` this change fixes — verified experimentally (SQLite, FK enforcement on).
+- Pairing it with `cascade="all, delete-orphan"` closes that gap: with both set, an *unloaded* collection still takes the passive path (no lazy-load, DB cascades), and a *loaded* collection is instead cleaned up by SQLAlchemy's own delete-orphan cascade rather than being nulled out — correct either way, at the cost of one extra `DELETE FROM budget_categories WHERE id IN (...)` in the loaded case, which the DB's `ON DELETE CASCADE` would have done anyway. This is SQLAlchemy's documented pairing for "let the DB cascade when possible, but stay correct if the collection happens to be loaded."
+- Both settings still require the DB constraint to actually be `ON DELETE CASCADE` (true here, confirmed in migration `000014`) — the wrong choice if the constraint were `RESTRICT`/`NO ACTION`, since then rows would just silently fail to delete in the unloaded case.
+
+**Swap the `minio` image to `pgsty/minio:RELEASE.2026-08-04T00-00-00Z`, pinned, in both `docker-compose.local.yml` and `docker-compose.dev.yml`.**
+- `pgsty/minio` is a community rebuild of the same MinIO server using upstream's own Dockerfile and entrypoint. Checked against the registry and compared with the cached upstream image: same entrypoint (`docker-entrypoint.sh`) and `Cmd` (`minio`), identical env (`MINIO_ROOT_*_FILE`, `MC_CONFIG_DIR`, minisign key), and `/usr/bin/mc` is present (a symlink to `mcli`), so the existing `mc ready local` healthcheck still works. Builds exist for amd64 and arm64. The rest of the service block stays unchanged.
+- Pin a release tag rather than `:latest`, so the next upstream change is a deliberate bump, not a CI break.
+- Alternatives rejected: `chainguard/minio` (free tier offers `:latest` only, can't be pinned); `bitnamilegacy/minio` (frozen in August 2025, different entrypoint and env scheme, would need the service block rewritten); switching to a different S3 server such as Garage, SeaweedFS or RustFS (a storage decision far larger than a CI fix).
+- Production (`docker-compose.prod.yml`) doesn't run MinIO, so it's unaffected.
 
 ## Risks / Trade-offs
 
 - [`passive_deletes=True` silently relies on the DB constraint matching model expectations; if a future migration changes `budget_categories_budget_id_fkey` back to `RESTRICT` without updating the model, budget deletion would start failing again with a real (unmasked) `IntegrityError`] → Acceptable: that failure mode is the same generic `IntegrityError` → 400 path already in place for lines/reports, so it fails safe (blocks deletion, doesn't corrupt data) rather than silently.
 - [Orphaned categories with zero lines still accumulate until their budget is deleted] → Out of scope per Non-Goals; tracked separately as a known bookkeeping gap, not a correctness or security issue.
+- [`pgsty/minio` is a single-maintainer community build and could go stale or disappear the same way] → The pinned tag keeps CI reproducible while it exists. If it goes away, the swap is the same one-line change per file, and moving to another S3-compatible server becomes the follow-up decision.
+- [Existing dev/local `minio_data` volumes were written by an older MinIO release] → MinIO reads data written by older releases. Developers pick up the new image only when they recreate their own minio container.

openspec/changes/budget-fix-295-category-cascade-delete-fix/proposal.md

@@ -6,8 +6,9 @@
 
 ## What Changes
 
-- `BudgetModel.categories` gets `passive_deletes=True` so SQLAlchemy's ORM defers to the database's existing `ON DELETE CASCADE` on `budget_categories.budget_id` instead of trying to lazy-load the collection and null out each child's non-nullable `budget_id` during flush (which raises the spurious `IntegrityError`).
-- Out of scope: cleaning up an orphaned category when its last line is deleted is a separate, pre-existing bookkeeping gap (also related to the already-known global-category-namespace issue) and is not needed to fix this bug — `passive_deletes=True` makes budget deletion correct regardless of whether an orphaned category row exists.
+- `BudgetModel.categories` gets `cascade="all, delete-orphan", passive_deletes=True` so SQLAlchemy defers to the database's existing `ON DELETE CASCADE` on `budget_categories.budget_id` instead of lazy-loading the collection and nulling out each child's non-nullable `budget_id` during flush (which raises the spurious `IntegrityError`), and deletes the children itself if the collection happens to be loaded (see design.md).
+- The `minio` service in `docker-compose.local.yml` and `docker-compose.dev.yml` switches from `quay.io/minio/minio:latest` to the pinned community build `pgsty/minio:RELEASE.2026-08-04T00-00-00Z`. MinIO stopped publishing public images (quay.io now returns 401, Docker Hub's `minio/minio` returns 404), so a fresh CI runner can't start the e2e stack — without this, the `E2E Tests` workflow can't verify the fix above.
+- Out of scope: cleaning up an orphaned category when its last line is deleted is a separate, pre-existing bookkeeping gap (also related to the already-known global-category-namespace issue) and is not needed to fix this bug — the cascade settings make budget deletion correct regardless of whether an orphaned category row exists.
 
 ## Capabilities
 
@@ -23,4 +24,5 @@ None — this restores compliance with the already-declared `async-persistence`
 
 - `services/budget/app/models/budget.py` (`BudgetModel.categories` relationship)
 - Fixes `DELETE /api/v1/budgets/{id}` for any budget that ever had a line added and removed
-- Unblocks `frontend-typescript/e2e/specs/api/auth-budget-chain.spec.ts` in the `E2E Tests` CI workflow
+- `docker-compose.local.yml`, `docker-compose.dev.yml` (`minio` service image only; `command`, env, volumes and healthcheck unchanged)
+- Unblocks `frontend-typescript/e2e/specs/api/auth-budget-chain.spec.ts` in the `E2E Tests` CI workflow, which currently fails at image pull on every branch, `main` included

openspec/changes/budget-fix-295-category-cascade-delete-fix/tasks.md

@@ -2,9 +2,22 @@
 
 Workflow rule: one task group = one GitHub sub-issue (of this change's parent issue) = one PR, merged before the next group starts.
 
-## 1. Fix budget deletion cascade for orphaned categories
+<!-- flow:tracking:start -->
+## Tracking
 
-- [ ] 1.1 Add `passive_deletes=True` to `BudgetModel.categories` in `services/budget/app/models/budget.py`
-- [ ] 1.2 Add/extend a `services/budget` test that creates a budget, adds a line (auto-creating a category), deletes the line, then deletes the budget, and verify `DELETE /budgets/{id}` returns 200 with `success: true` instead of a 400
-- [ ] 1.3 Run `frontend-typescript/e2e/specs/api/auth-budget-chain.spec.ts` against the local docker-compose e2e stack (or the `E2E Tests` CI workflow) and verify the DELETE-budget assertion passes
-- [ ] 1.4 Run `services/budget`'s test suite and lint clean; PR merged
+Parent issue: [#295](https://github.com/arutsh/OpenGrantFlow/issues/295) · [Project board](https://github.com/users/arutsh/projects/8)
+
+| Group | Sub-issue | Branch | State |
+| --- | --- | --- | --- |
+| 1. Fix budget deletion cascade for orphaned categories | [#346](https://github.com/arutsh/OpenGrantFlow/issues/346) | `Budget/fix/Issue-346/category-cascade-delete-fix-group1` | open |
+
+_Generated by `scripts/flow.py sync budget-fix-295-category-cascade-delete-fix` — do not edit by hand._
+<!-- flow:tracking:end -->
+
+## 1. Fix budget deletion cascade for orphaned categories — Issue #346
+
+- [x] 1.1 Add `cascade="all, delete-orphan", passive_deletes=True` to `BudgetModel.categories` in `services/budget/app/models/budget.py`
+- [x] 1.2 Add/extend real-DB `services/budget` tests: (a) create a budget, add a line (auto-creating a category), delete the line, delete the budget, verify success with `categories` unloaded; (b) same, but force `categories` loaded (e.g. re-fetch with `selectinload`) before deleting, verify success there too
+- [x] 1.3 Swap the `minio` service image in `docker-compose.local.yml` and `docker-compose.dev.yml` from `quay.io/minio/minio:latest` to `pgsty/minio:RELEASE.2026-08-04T00-00-00Z` (see design.md), so the `E2E Tests` workflow can pull it on a fresh runner
+- [ ] 1.4 Run `frontend-typescript/e2e/specs/api/auth-budget-chain.spec.ts` against the local docker-compose e2e stack (or the `E2E Tests` CI workflow) and verify the DELETE-budget assertion passes
+- [ ] 1.5 Run `services/budget`'s test suite and lint clean; PR merged

services/budget/app/models/budget.py

@@ -81,7 +81,10 @@ class BudgetModel(Base, AuditMixin):
     )
     reports: Mapped[list["ReportModel"]] = relationship("ReportModel", back_populates="budget")
     categories: Mapped[list["BudgetCategoryModel"]] = relationship(
-        "BudgetCategoryModel", back_populates="budget"
+        "BudgetCategoryModel",
+        back_populates="budget",
+        cascade="all, delete-orphan",
+        passive_deletes=True,
     )
 
 

services/budget/tests/test_budget_crud.py

@@ -1,8 +1,13 @@
 import uuid
 
 import pytest
+from sqlalchemy import select
+from sqlalchemy.orm import selectinload
 
-from app.crud.budget_crud import create_budget, update_budget
+from app.crud.budget_crud import create_budget, delete_budget, update_budget
+from app.crud.budget_category_crud import create_budget_category
+from app.crud.budget_line_crud import create_budget_line, delete_budget_line
+from app.models.budget import BudgetModel
 from shared.security.current_user_context import reset_current_user_id, set_current_user_id
 from tests.factories.user import ValidUserFactory
 
@@ -67,3 +72,48 @@ async def test_updated_by_reflects_the_editing_user_not_the_creator(self, db):
             reset_current_user_id(token)
 
         assert updated.updated_by == editor_id
+
+
+async def _budget_with_orphaned_category(db) -> BudgetModel:
+    """A budget with a line added then removed, leaving its
+    auto-created category behind (see #295's failure mode)."""
+    user = ValidUserFactory()
+    budget = await create_budget(
+        session=db, user_id=user["user_id"], name="Cascade", owner_id=user["customer_id"]
+    )
+    category = await create_budget_category(
+        session=db, user_id=user["user_id"], budget_id=budget.id, name="Travel"
+    )
+    line = await create_budget_line(
+        session=db,
+        user_id=user["user_id"],
+        budget_id=budget.id,
+        category_id=category.id,
+        description="Flight",
+        amount=100.0,
+    )
+    await delete_budget_line(session=db, budget_line=line)
+    return budget
+
+
+@pytest.mark.anyio
+class TestDeleteBudgetCascadesOrphanedCategory:
+    async def test_delete_succeeds_with_categories_unloaded(self, db):
+        budget = await _budget_with_orphaned_category(db)
+
+        assert await delete_budget(session=db, budget=budget) is True
+
+    async def test_delete_succeeds_with_categories_already_loaded(self, db):
+        # Distinguishes cascade+passive_deletes from passive_deletes alone,
+        # which only fixes the unloaded case.
+        budget = await _budget_with_orphaned_category(db)
+
+        result = await db.execute(
+            select(BudgetModel)
+            .where(BudgetModel.id == budget.id)
+            .options(selectinload(BudgetModel.categories))
+        )
+        loaded_budget = result.scalar_one()
+        assert len(list(loaded_budget.categories)) == 1
+
+        assert await delete_budget(session=db, budget=loaded_budget) is True