--- id: review-cascade-data-integrity name: Review Cascade Decisions and Data Integrity Specs status: completed depends_on: [cascade-decisions, data-integrity-specs, resolve-sessions-accountid-cascade, resolve-audit-logs-ownerid-cascade, add-missing-cascade-entries, define-sync-field-split, enforce-parts-session-id-invariant, specify-task-body-append-concurrency, resolve-org-dual-ownership, resolve-message-id-adr-contradiction] scope: moderate risk: critical impact: project level: review --- ## Description Independent review of the cascade decisions chain and data integrity specifications — the highest-risk path in the architecture spec. This review covers: 1. **Cascade consistency**: The D1 cascade policy defaults (RESTRICT for audit, SET NULL for live data, CASCADE for ephemeral config) must be consistently applied across every FK relationship in `table-reference.md`. This is the foundation the entire storage layer sits on. 2. **NOT NULL / SET NULL contradictions**: The original C01 contradiction (`sessions.accountId NOT NULL + SET NULL`, `audit_logs.ownerId NOT NULL + SET NULL`) was resolved, but the resolution must be verified in all referencing docs. 3. **Field authority splits**: The authored-vs-runtime field split in `tasks.md` must cover all columns, and the sync upsert must explicitly exclude runtime-managed fields. 4. **Denormalization invariants**: `parts.sessionId` immutability and `organizations.ownerId ⊆ membershipLevel='owner'` must be documented and their enforceability verified. 5. **Operations/registrations split**: The two-table design must be consistent across `spokes.md`, `call-graph.md`, `coordination.md`, and `table-reference.md`. All 10 dependent tasks are completed but none have Summary sections filled. This review must verify that the spec documents actually reflect the decisions, not just that tasks were marked complete. ## Acceptance Criteria - [ ] Every FK in `table-reference.md` has an explicit `onDelete` action with a rationale — no gaps - [ ] No remaining `NOT NULL + SET NULL` contradictions exist in any doc - [ ] `sessions.accountId` is declared `text` (nullable) in `sessions.md` with the orphan-preservation rationale - [ ] `audit_logs.ownerId` cascade is RESTRICT in `table-reference.md` with the audit-integrity rationale - [ ] The D1 cascade policy table in `storage-spec-phase1-resolutions.md` is consistent with `table-reference.md` entries - [ ] `tasks.md` Field Authority Split lists ALL columns as either authored or runtime-managed with no gaps - [ ] `parts.sessionId` immutability is documented with application-level enforcement note in `sessions.md` - [ ] `organizations.ownerId` invariant (always references a member with `membershipLevel: 'owner'`) is documented in `identity.md` - [ ] `hub.task.addNote` specifies DB-level concatenation (`COALESCE(body, '') || $note`) in `tasks.md` - [ ] ADR-003 correctly scopes sortable IDs to `parts` only (not `messages`) - [ ] `operations` / `operation_registrations` table schemas in `spokes.md` match `table-reference.md` entries - [ ] Spoke disconnect lifecycle (deactivate registrations, keep definitions) is documented in `spokes.md` ## References - docs/decisions/storage-spec-phase1-resolutions.md (D1 cascade policy) - docs/architecture/storage/table-reference.md (cascade reference table) - docs/architecture/storage/identity.md (accounts, orgs, audit_logs) - docs/architecture/storage/sessions.md (sessions, parts, messages) - docs/architecture/storage/tasks.md (field authority split, addNote concurrency) - docs/architecture/storage/spokes.md (operations/registrations split) - docs/reviews/storage-architecture-review-2026-04-21.md (original review that spawned these tasks) ## Notes This is the most critical review in the architecture. The cascade decisions chain was the highest-risk path identified by `taskgraph risk-path`. Getting these wrong means either data loss in production or account deletion failures that cascade unexpectedly. ## Summary Review completed 2026-04-23. 12 acceptance criteria checked: 9 PASS, 3 FAIL (2 critical, 1 warning-level). **Critical issues found and fixed:** 1. `detections.resolvedBy → accounts.id (SET NULL)` FK was documented in coordination.md but missing from the canonical cascade table in table-reference.md → **Fixed**: added to cascade table. 2. `tasks.projectId` was missing from the Field Authority Split in tasks.md → **Fixed**: added to authored fields with note about project context. **Warnings noted:** - Audit FKs use SET NULL for nullable columns vs D1 RESTRICT default → **Fixed**: updated D1 rationale to clarify "RESTRICT on NOT NULL FKs; SET NULL on nullable FKs" - No remaining NOT NULL + SET NULL contradictions All fixes committed in same change set as this review.