Files
hub/tasks/architecture/storage/review-cascade-data-integrity.md
glm-5.1 2b63cda1c7 Setup repo: migrate architecture specs, code stubs, and tasks from alkhub_ts
Copy architecture docs, ADRs, storage domain specs, research, reviews,
and 56 storage architecture tasks from the alkhub_ts monorepo. Adapt for
standalone @alkdev/hub repo structure (src/ not packages/hub/).

Sanitize all sensitive information:
- Replace private IPs (10.0.0.1) with localhost defaults
- Remove internal server hostnames (dev1, ns528096)
- Replace /workspace/ private paths with npm package references
- Remove hardcoded credentials from examples
- Rewrite infrastructure.md without private network details

Add Deno project scaffolding: deno.json (pinned deps), .gitignore,
AGENTS.md, entry point. Migrate existing code stubs (crypto, config
types, logger) with updated import paths.
2026-05-25 10:56:32 +00:00

69 lines
4.7 KiB
Markdown

---
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.