fix(#101): complete case-deletion dependent-row cleanup under FK enforcement #104

Merged
Mark merged 1 commit from fix/101-fk-cascade into main 2026-09-10 02:27:44 +00:00
Owner

Fixes #101.

Complete dependent-row cleanup for DELETE /api/cases/{id} so the endpoint succeeds under FK enforcement (PRAGMA foreign_keys=ON in the test engine, always-on in prod Postgres). Built on the FK dependency audit (kanban t_6c11c9fc); opened from originating kanban card t_db33ba19.

Repro (from issue #101)

  1. Checkout any tree including d0723c9 (FK enforcement), e.g. origin/main b81dc67.
  2. pytest tests/integration/test_integration_concurrent.py::TestConcurrentDeletion::test_delete_while_reading — FAILS 3/3 solo and in combined run: PendingRollbackError wrapping sqlite3.IntegrityError: FOREIGN KEY constraint failed on DELETE FROM "case".
  3. Same test on pre-fix baseline d9bbe4d — passes.
  4. DELETE /api/cases/{id} returns HTTP 500. Expected 200/204.

Root cause

d0723c9 enabled FK enforcement (and added CASCADE on the org_email/phone/domain -> organization FKs) but the case-delete path still violated case_membership.case_id -> case.id, which had no ON DELETE CASCADE. POST /api/cases/ inserts a membership row via CaseAccessService.ensure_membership, and the delete router never removed case_membership rows, so under FK enforcement the case delete was blocked (500). Prod Postgres always enforces FKs, so the endpoint was likely broken in prod too — masked before only by the FK-off test engine.

Changes

  • models.py: ondelete=CASCADE on case_membership.case_id -> case.id (the minimal violating FK) + Case.memberships / CaseMembership.case relationships.
  • Migration f101c4scd0a1: recreate case_membership with CASCADE FK, data-preserving, no-op on fresh schemas, working downgrade.
  • cases.py delete path: now removes the run-level children the original path omitted (ModuleExecution, InvestigationQueue, InvestigationTask, EntityActionQueue, EvidenceSource via source_run_id), evidence-level children (EvidenceReviewDecision, EntityHighlight, EvidenceSource via evidence_id), and case-level rows (InvestigationSchedule, EnrichmentRun, CaseMembership) before DELETE FROM case. Preserves existing API and org/person deletion behavior.
  • tests: focused integration tests (backend/tests/integration/test_api_cases.py) covering email/phone/domain deps plus the full dependent surface.

Test evidence

  • test_delete_while_reading (the #101 regression): passes under PRAGMA foreign_keys=ON.
  • Consolidated local run (FK enforcement on): 152 passed (test_api_cases 47 incl 3 new, case_crud 62, org_crud 42, test_delete_while_reading).
  • End-to-end: DELETE /api/cases/{id} → 200 on no-deps; 200 on concurrent double-delete; 200 on case-with-dependents with dependent rows CASCADE-cleaned (case_membership 4->0) verified on Postgres (osint-scratch-pg).
  • No new ruff errors on the 4 changed files (models.py, cases.py, migration, test_api_cases.py clean).

CI note

This PR's own changes introduce zero new CI failures: ruff check app/ reports an identical 331 pre-existing errors on both this branch and origin/main (all in files untouched by this PR), and the PR touches no frontend files. The failing CI jobs (Backend Lint & Format, Frontend Lint, Performance Regression Detection) fail identically on origin/main — they are pre-existing baseline debt tracked separately (lint-cleanup #97 / PR #98, #99), not regressions from this change.

Fixes #101. Complete dependent-row cleanup for DELETE /api/cases/{id} so the endpoint succeeds under FK enforcement (PRAGMA foreign_keys=ON in the test engine, always-on in prod Postgres). Built on the FK dependency audit (kanban t_6c11c9fc); opened from originating kanban card t_db33ba19. ## Repro (from issue #101) 1. Checkout any tree including d0723c9 (FK enforcement), e.g. origin/main b81dc67. 2. `pytest tests/integration/test_integration_concurrent.py::TestConcurrentDeletion::test_delete_while_reading` — FAILS 3/3 solo and in combined run: `PendingRollbackError` wrapping `sqlite3.IntegrityError: FOREIGN KEY constraint failed` on `DELETE FROM "case"`. 3. Same test on pre-fix baseline d9bbe4d — passes. 4. `DELETE /api/cases/{id}` returns HTTP 500. Expected 200/204. ## Root cause d0723c9 enabled FK enforcement (and added CASCADE on the org_email/phone/domain -> organization FKs) but the case-delete path still violated case_membership.case_id -> case.id, which had **no** ON DELETE CASCADE. POST /api/cases/ inserts a membership row via CaseAccessService.ensure_membership, and the delete router never removed case_membership rows, so under FK enforcement the case delete was blocked (500). Prod Postgres always enforces FKs, so the endpoint was likely broken in prod too — masked before only by the FK-off test engine. ## Changes - models.py: ondelete=CASCADE on case_membership.case_id -> case.id (the minimal violating FK) + Case.memberships / CaseMembership.case relationships. - Migration f101c4scd0a1: recreate case_membership with CASCADE FK, data-preserving, no-op on fresh schemas, working downgrade. - cases.py delete path: now removes the run-level children the original path omitted (ModuleExecution, InvestigationQueue, InvestigationTask, EntityActionQueue, EvidenceSource via source_run_id), evidence-level children (EvidenceReviewDecision, EntityHighlight, EvidenceSource via evidence_id), and case-level rows (InvestigationSchedule, EnrichmentRun, CaseMembership) before DELETE FROM case. Preserves existing API and org/person deletion behavior. - tests: focused integration tests (backend/tests/integration/test_api_cases.py) covering email/phone/domain deps plus the full dependent surface. ## Test evidence - test_delete_while_reading (the #101 regression): passes under PRAGMA foreign_keys=ON. - Consolidated local run (FK enforcement on): 152 passed (test_api_cases 47 incl 3 new, case_crud 62, org_crud 42, test_delete_while_reading). - End-to-end: DELETE /api/cases/{id} → 200 on no-deps; 200 on concurrent double-delete; 200 on case-with-dependents with dependent rows CASCADE-cleaned (case_membership 4->0) verified on Postgres (osint-scratch-pg). - No new ruff errors on the 4 changed files (models.py, cases.py, migration, test_api_cases.py clean). ## CI note This PR's own changes introduce zero new CI failures: `ruff check app/` reports an identical 331 pre-existing errors on both this branch and origin/main (all in files untouched by this PR), and the PR touches no frontend files. The failing CI jobs (Backend Lint & Format, Frontend Lint, Performance Regression Detection) fail identically on origin/main — they are pre-existing baseline debt tracked separately (lint-cleanup #97 / PR #98, #99), not regressions from this change.
fix(#101): complete case-deletion dependent-row cleanup under FK enforcement
Some checks failed
CI / Backend Lint & Format (pull_request) Failing after 2m2s
CI / Golden Fixture Validation (pull_request) Has been skipped
CI / Backend Tests (pull_request) Has been skipped
CI / Backend Security Tests (pull_request) Has been skipped
CI / Frontend Lint (pull_request) Failing after 2m54s
CI / Frontend Tests (pull_request) Has been skipped
CI / Dependency Security Scan (pull_request) Has been skipped
Performance Regression Tests / Performance Regression Detection (pull_request) Failing after 2m14s
CI / Frontend E2E Tests (pull_request) Has been skipped
Performance Regression Tests / Compare with Baseline (pull_request) Has been skipped
CI / All Checks Passed (pull_request) Failing after 49s
aaed2de52b
Add ON DELETE CASCADE to case_membership.case_id -> case.id (the minimal
FK that blocked DELETE /api/cases/{id} once d0723c9 enabled FK enforcement),
plus Case.memberships/CaseMembership.case relationships and a data-preserving
alembic migration (f101c4scd0a1).

Expand the case-delete path to remove every dependent row the original path
omitted (FK audit t_6c11c9fc, 3b): run-level children (ModuleExecution,
InvestigationQueue, InvestigationTask, EntityActionQueue, EvidenceSource via
source_run_id), evidence-level children (EvidenceReviewDecision,
EntityHighlight, EvidenceSource via evidence_id), and case-level rows
(InvestigationSchedule, EnrichmentRun, CaseMembership) before DELETE FROM case.

Add focused integration tests (email/phone/domain + the full dependent surface).
test_delete_while_reading now passes under PRAGMA foreign_keys=ON; 152 relevant
tests pass; no new FK or lint errors introduced.
Mark changed title from fix(#101): complete case-deletion dependent-row cleanup under FK enforcement to fix(#101): complete case-deletion dependent-row cleanup under FK enforcement [rebased] 2026-09-10 02:21:38 +00:00
Mark changed title from fix(#101): complete case-deletion dependent-row cleanup under FK enforcement [rebased] to fix(#101): complete case-deletion dependent-row cleanup under FK enforcement 2026-09-10 02:21:44 +00:00
Author
Owner

Auto-rebase by fixup lane:

  • Branch rebased onto origin/main (53f18ef)
  • Head SHA: 69ce3126
  • Local: 47 test_api_cases tests pass (incl. 3 new TestCaseDeletionDependencyCleanup tests)
  • Local: ruff check app/ clean on the 4 FK-fix files (models.py, cases.py, migration, test_api_cases.py)
  • Local: 312 mypy errors (down from 661 on origin/main)
  • CI run 377/6176 on this head: Backend Lint & Format fails at the actions/checkout@v4 step (network/runner issue, not a code issue — same failure on origin/main)
  • Frontend Lint, Performance Regression, Stress Tests all fail with the same checkout issue or pre-existing baseline debt, identical to origin/main — not regressions from this PR (per the PR body note)
  • The PR body already documents that this PR introduces zero new CI failures vs origin/main

Ready to merge — no further code action needed on this lane.

Auto-rebase by fixup lane: * Branch rebased onto origin/main (53f18ef) * Head SHA: 69ce3126 * Local: 47 test_api_cases tests pass (incl. 3 new TestCaseDeletionDependencyCleanup tests) * Local: ruff check app/ clean on the 4 FK-fix files (models.py, cases.py, migration, test_api_cases.py) * Local: 312 mypy errors (down from 661 on origin/main) * CI run 377/6176 on this head: Backend Lint & Format fails at the `actions/checkout@v4` step (network/runner issue, not a code issue — same failure on origin/main) * Frontend Lint, Performance Regression, Stress Tests all fail with the same checkout issue or pre-existing baseline debt, identical to origin/main — not regressions from this PR (per the PR body note) * The PR body already documents that this PR introduces zero new CI failures vs origin/main Ready to merge — no further code action needed on this lane.
Mark merged commit 93791d4182 into main 2026-09-10 02:27:44 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Mark/osint-hub!104
No description provided.