Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-07-15
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Design: add-composite-tenant-pk

## Context

User isolation is implemented at three levels (the Edge Function filters by `p_user_id`, the RPC filters every query, RLS policies), but the database schema does not enforce it: `PRIMARY KEY (id)` is global and FKs do not verify ownership. The project is not in production — no data migration; the database is recreated manually. The Testcontainers environment (`packages/integration/docker-compose.yml`) uses `supabase/postgres:15.14.1.122`.

Driven by FR1–FR7 from the proposal.

## Goals / Non-Goals

**Goals:**

- Composite PK `(user_id, id)` and composite ownership-enforcing FKs (FR1, FR2).
- Zero changes in observable sync protocol and client behavior (FR6).
- Edits only in existing migration files (FR5).

**Non-Goals:**

- Data migration, `files`, `settings`, `sync_meta`, the GAS backend (NG1–NG5 from the proposal).

## Decisions

### D1: Explicit FK constraint names in the `<table>_<field>_fkey` format

Implements FR4. The default name for a composite FK in Postgres is `tasks_user_id_goal_id_fkey`. The regexp in `push_records` (`'^.*?_(.+?)_fkey$'`) would extract `user_id_goal_id` from it, breaking client-side self-healing: `pushRejectionHandler.ts` matches the field against the `CLEARABLE_FK_FIELDS` / `DELETE_FK_FIELDS` sets (`goal_id`, `context_id`, `category_id`, `task_id`).

Decision: set names explicitly — `CONSTRAINT tasks_goal_id_fkey FOREIGN KEY (user_id, goal_id) REFERENCES goals (user_id, id) ...`.

Alternative (rejected): change the regexp in the RPC and/or the client handler — touches two packages and the rejection-reason contract for the sake of schema cosmetics.

### D2: `ON DELETE SET NULL (<column>)` — column-list form

Implements FR3. A plain `ON DELETE SET NULL` on a composite FK would null both columns, including `user_id NOT NULL` → an error when triggered. Postgres 15+ supports specifying a column subset: `ON DELETE SET NULL (goal_id)`. The infrastructure runs PG 15.14 — the syntax is available.

`checklist_items.task_id` keeps `ON DELETE CASCADE` — cascading works with composite FKs without caveats.

Alternative (rejected): drop the `ON DELETE` actions, since rows are only deleted during purge where dependencies are cleaned up manually. Rejected: the cascade from `auth.users` (account deletion) and hard deletes in `purge_deleted_records` rely on the current FK actions.

### D3: RPC functions remain unchanged

`push_records` (003) and `purge_deleted_records` (004) already filter every query by `user_id` and insert `user_id` explicitly — with a composite PK their SQL is correct without edits. Only comments change, if they describe key structure. This keeps the diff minimal and confirms FR6.

Semantic consequence: the `unique_violation` branch in `push_records` becomes unreachable for cross-user collisions (another user's INSERT with the same `id` is now legal); the handler itself stays — it also catches other 23505 cases.

### D4: Scope — only tables with client-generated UUIDs

The PK changes on `contexts`, `categories`, `goals`, `ideas`, `tasks`, `checklist_items`, `attachments`. `files.file_id` is server-generated and not part of the push protocol; `settings`/`sync_meta` are already composite (NG1, NG2).

### D5: No index changes

Implements NFR-P1. The keyset indexes `idx_<table>_user_revision_id (user_id, revision, id)` remain. The composite PK provides a `(user_id, id)` index — it covers all point lookups in the RPC (`WHERE id = ... AND user_id = ...`). The old unique index on `(id)` disappears — no query looks up by `id` without `user_id`.

### D6: Cross-tenant integration tests live in the existing poison-pill suite

Implements FR7. The scenarios (same UUID for two users; FK to another user's goal) require two registered users in one Testcontainers instance. The poison-pill suite (`packages/integration/src/tests/push-poison-pill-*.spec.ts`) already exercises push rejection paths and the self-healing flow — the new scenarios extend it rather than introducing a new suite.

## Risks / Trade-offs

- [Cross-tenant scenarios cannot be verified without two users] → integration test with two registered users in one Testcontainers instance (D6).
- [Editing already-applied migration files diverges from deployed environments] → deliberate: the user recreates dev/qa/prod manually (`scripts/reset.sh`); the edit is visible in git history.
- [`SET NULL (column)` syntax unsupported on older PG] → infrastructure is pinned to PG 15.14; verified by the docker-compose migration run (M3).
- [An unnoticed dependency on global `id` uniqueness] → pull, push, purge, and the client all operate on data within a single user; confirmed by the full integration test run (M1).

## Migration Plan

1. Update `001_create_tables.sql`: composite PKs, explicitly named composite FKs, `SET NULL (<column>)`.
2. Refresh comments in `003`/`004` if they describe key structure (SQL logic unchanged).
3. Run integration tests (Testcontainers rebuilds the schema from scratch).
4. The user manually recreates environments (`bash scripts/reset.sh <env>`).

Rollback: git revert of the migration edits + re-reset of environments.

## Open Questions

_none — Q1 from the proposal resolved in favor of adding cross-tenant integration tests to the poison-pill suite (D6)._
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# add-composite-tenant-pk

## Why

Supabase entity tables (`contexts`, `categories`, `goals`, `ideas`, `tasks`, `checklist_items`, `attachments`) use a global `PRIMARY KEY (id)` while UUIDs are generated client-side. User isolation rests solely on `user_id` filters in code (Edge Functions, RPC, RLS), not on the database schema:

- a UUID collision between users (or a maliciously supplied foreign UUID) results in a permanent `rejected: unique_violation` for the record;
- FK constraints (`tasks.goal_id → goals(id)` etc.) do not verify ownership — an authenticated user can create a reference to another user's entity.

The project is not in production yet — the schema can be rebuilt from scratch with no data migration. This is the last cheap moment to add schema-level defense-in-depth.

## What Changes

- **MODIFIED**: entity tables get a composite `PRIMARY KEY (user_id, id)` instead of `PRIMARY KEY (id)`.
- **MODIFIED**: FK constraints become composite and enforce ownership: `(user_id, goal_id) REFERENCES goals (user_id, id)` etc.
- **MODIFIED**: `ON DELETE SET NULL` uses the column-list form (PG 15+) so only the reference column is nulled, never `user_id`.
- Existing migration files are updated **in place** (no new files) — the database is recreated manually.
- Sync protocol behavior (push/pull/purge) and the client remain unchanged.

## Capabilities

### New Capabilities

_none_

### Modified Capabilities

- `supabase-schema`: primary key and FK constraint requirements for entity tables — composite PK `(user_id, id)`, composite ownership-enforcing FKs, stable constraint names.

## Goals

- G1: User isolation is enforced at the database schema level (PK and FK), not only in code.
- G2: Cross-user UUID collisions become impossible by construction (`id` is unique per user).
- G3: A reference to another user's entity is rejected by the database as `fk_violation`.
- G4: The number of migration files does not grow — edits go into existing files.

## Non-Goals

- NG1: The `files` table — its PK `file_id` is server-generated (`gen_random_uuid()`) and has no incoming FKs; untouched.
- NG2: `settings` and `sync_meta` — already have composite PK `(user_id, key)`; untouched.
- NG3: Data migration path for existing data — no production yet, the database is recreated manually.
- NG4: Changes to the sync protocol, API formats, or client code (except tests if needed).
- NG5: The GAS backend.

## Users & Scenarios

- U1: A user syncs data — behavior is unchanged, push/pull work as before.
- U2: Two users accidentally (or one maliciously) use the same UUID — both records are stored independently, no `unique_violation`.
- U3: A malicious authenticated user pushes a task with the `goal_id` of another user's goal — the database rejects the record as `fk_violation:goal_id`, and client-side self-healing clears the reference.

## Requirements

### Functional

- FR1: Tables `contexts`, `categories`, `goals`, `ideas`, `tasks`, `checklist_items`, `attachments` have `PRIMARY KEY (user_id, id)`.
- FR2: FK constraints reference the composite key and include `user_id`: `tasks (user_id, goal_id) → goals (user_id, id)`, `tasks (user_id, context_id) → contexts (user_id, id)`, `tasks (user_id, category_id) → categories (user_id, id)`, `checklist_items (user_id, task_id) → tasks (user_id, id)`. The `DEFERRABLE INITIALLY DEFERRED` property and `ON DELETE` actions are preserved.
- FR3: For nullable references, `ON DELETE SET NULL` applies only to the reference column (the `ON DELETE SET NULL (<column>)` form); `user_id` is never nulled.
- FR4: FK constraint names keep the `<table>_<field>_fkey` format (set explicitly) so the rejection reason parsing in `push_records` (`fk_violation:<field>`) and client-side self-healing (`pushRejectionHandler`) keep working unchanged.
- FR5: Edits go into the existing files `001_create_tables.sql`–`004_purge_bump_dependent_revision.sql`; no new migration files are created.
- FR6: Observable behavior of `push_records` and `purge_deleted_records` for valid data does not change (same statuses, response formats, reason codes).
- FR7: Cross-tenant scenarios (U2, U3) are covered by integration tests added to the existing poison-pill suite in `packages/integration`.

### Non-Functional

#### Performance

- NFR-P1: The keyset pagination index `(user_id, revision, id)` is preserved on all entity tables; pull query plans do not degrade.

#### Accessibility

_not applicable — server-side schema change_

#### Responsive

_not applicable — server-side schema change_

## UX Acceptance Criteria

- UX1: The user notices no difference: sync, conflicts, and self-healing behave as before.

## UI States Matrix

_not applicable — no UI affected_

## Behavior

No new user-facing scenarios — this is a database schema change. Behavior is verified by integration tests (`packages/integration`, Testcontainers applies the updated migrations) and existing contract tests. No Gherkin features are added.

## Visual Reference

_not applicable_

## Affected IA

no changes

## Success Metrics

- M1: All existing integration and contract tests pass without changing expectations (100%).
- M2: The number of files in `packages/adapter-supabase/supabase/migrations/` stays at 4.
- M3: A full from-scratch schema rebuild (docker-compose from `packages/integration`) applies migrations without errors.
- M4: Cross-tenant scenarios (U2, U3) are covered by at least two automated checks.

## Open Questions

_none — Q1 (whether to add cross-tenant integration tests requiring two users in one instance) resolved: yes, added to the existing poison-pill suite (FR7)._
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Delta: supabase-schema — add-composite-tenant-pk

## MODIFIED Requirements

### Requirement: Entity tables with user_id
The database SHALL have tables for `tasks`, `goals`, `ideas`, `contexts`, `categories`, `checklist_items`. Each table SHALL have a `user_id UUID NOT NULL REFERENCES auth.users(id)` column. Each entity table (including `attachments`) SHALL have a composite `PRIMARY KEY (user_id, id)` — the `id` UUID is unique per user, not globally. Each table SHALL have a composite index on `(user_id, revision)` for efficient pull queries.

#### Scenario: Task table schema
- **WHEN** migration is applied
- **THEN** `tasks` table exists with columns: `id` (UUID, part of composite PK `(user_id, id)`), `user_id` (UUID FK to auth.users), `name` (TEXT NOT NULL), `description` (TEXT DEFAULT ''), `box` (TEXT NOT NULL CHECK in inbox/today/week/later), `goal_id` (UUID FK to goals ON DELETE SET NULL, DEFERRABLE), `context_id` (UUID FK to contexts ON DELETE SET NULL, DEFERRABLE), `category_id` (UUID FK to categories ON DELETE SET NULL, DEFERRABLE), `is_completed` (BOOLEAN DEFAULT false), `completed_at` (TIMESTAMPTZ), `repeat_rule` (JSONB, nullable), `is_hidden` (BOOLEAN DEFAULT false), `next_date` (DATE), `appear_date` (DATE), `original_task_id` (UUID, no FK constraint), `sort_order` (INTEGER DEFAULT 0), `is_deleted` (BOOLEAN DEFAULT false), `created_at` (TIMESTAMPTZ NOT NULL), `updated_at` (TIMESTAMPTZ NOT NULL), `revision` (BIGINT DEFAULT 0)

#### Scenario: Goals table schema
- **WHEN** migration is applied
- **THEN** `goals` table exists with columns: `id` (UUID, part of composite PK `(user_id, id)`), `user_id` (UUID FK), `name` (TEXT NOT NULL), `description` (TEXT DEFAULT ''), `cover_hash` (TEXT NOT NULL DEFAULT ''), `status` (TEXT NOT NULL CHECK in planning/in_progress/paused/completed/cancelled), `sort_order` (INTEGER DEFAULT 0), `is_deleted` (BOOLEAN DEFAULT false), `created_at` (TIMESTAMPTZ NOT NULL), `updated_at` (TIMESTAMPTZ NOT NULL), `revision` (BIGINT DEFAULT 0)

#### Scenario: Index on user_id + revision
- **WHEN** migration is applied
- **THEN** each entity table has index `idx_{table}_user_revision ON {table} (user_id, revision)`

#### Scenario: Same UUID for two different users
- **WHEN** user A has a record with id `X` and user B inserts a record with the same id `X` into the same table
- **THEN** both rows coexist — no unique violation occurs

### Requirement: FK constraints on reference fields
FK reference fields SHALL use `UUID` type with `DEFERRABLE INITIALLY DEFERRED` constraints. FK constraints SHALL be composite — they SHALL include `user_id` and reference the composite primary key of the parent table, enforcing that referenced entities belong to the same user. FK constraint names SHALL be explicitly set to the `<table>_<field>_fkey` format (e.g. `tasks_goal_id_fkey`) so structured rejection reasons (`fk_violation:<field>`) remain stable. Nullable FK fields store `NULL` when unset; their `ON DELETE SET NULL` action SHALL use the column-list form (`ON DELETE SET NULL (<field>)`) so only the reference column is nulled, never `user_id`. Tables SHALL be created in dependency order: contexts, categories, covers, goals, ideas, tasks, checklist_items. Self-referencing FK fields (`original_task_id`) SHALL NOT have FK constraints. The `goals.cover_hash` field SHALL be `TEXT NOT NULL DEFAULT ''` without FK constraint — cover integrity is managed by the cover sync protocol.

#### Scenario: Task FK fields reference parent tables
- **WHEN** migration is applied
- **THEN** `tasks` has `CONSTRAINT tasks_goal_id_fkey FOREIGN KEY (user_id, goal_id) REFERENCES goals (user_id, id)` ON DELETE SET NULL (goal_id) DEFERRABLE INITIALLY DEFERRED
- **AND** `tasks` has `CONSTRAINT tasks_context_id_fkey FOREIGN KEY (user_id, context_id) REFERENCES contexts (user_id, id)` ON DELETE SET NULL (context_id) DEFERRABLE INITIALLY DEFERRED
- **AND** `tasks` has `CONSTRAINT tasks_category_id_fkey FOREIGN KEY (user_id, category_id) REFERENCES categories (user_id, id)` ON DELETE SET NULL (category_id) DEFERRABLE INITIALLY DEFERRED
- **AND** `tasks.original_task_id` is `UUID` type without FK constraint

#### Scenario: Goal cover_hash has no FK constraint
- **WHEN** migration is applied
- **THEN** `goals.cover_hash` is `TEXT NOT NULL DEFAULT ''` without any FK constraint

#### Scenario: Checklist item FK field references tasks
- **WHEN** migration is applied
- **THEN** `checklist_items` has `CONSTRAINT checklist_items_task_id_fkey FOREIGN KEY (user_id, task_id) REFERENCES tasks (user_id, id)` ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED

#### Scenario: Reference to another user's entity is rejected
- **WHEN** a task is inserted for user B with `goal_id` pointing to a goal owned by user A
- **THEN** the FK constraint fails and `push_records` returns `status: "rejected"` with `reason: "fk_violation:goal_id"` for that record
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Tasks: add-composite-tenant-pk

## 1. Schema migration (in-place edits)

- [x] 1.1 `001_create_tables.sql`: replace `PRIMARY KEY` with composite `PRIMARY KEY (user_id, id)` in `contexts`, `categories`, `goals`, `ideas`, `tasks`, `checklist_items`, `attachments` (FR1, FR5)
- [x] 1.2 `001_create_tables.sql`: rewrite FKs `tasks.goal_id`/`context_id`/`category_id` as composite `FOREIGN KEY (user_id, <field>) REFERENCES <parent> (user_id, id)` with explicit names `tasks_<field>_fkey` and `ON DELETE SET NULL (<field>)`, preserving `DEFERRABLE INITIALLY DEFERRED` (FR2, FR3, FR4)
- [x] 1.3 `001_create_tables.sql`: rewrite FK `checklist_items.task_id` as composite with explicit name `checklist_items_task_id_fkey` and `ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED` (FR2, FR4)
- [x] 1.4 Refresh comments in `003_create_push_rpc.sql` / `004_purge_bump_dependent_revision.sql` if they describe key structure; do not change SQL logic (FR6)

## 2. Verification — schema applies cleanly

- [x] 2.1 Run migrations from scratch in the Testcontainers environment (`packages/integration`, `supabase/postgres:15.14.1.122`) — all 4 files apply without errors, including the `ON DELETE SET NULL (<column>)` syntax (M3)
- [x] 2.2 Verify via `pg_constraint`/`\d` (SQL query in the integration setup or manually) that FK names are `tasks_goal_id_fkey`, `tasks_context_id_fkey`, `tasks_category_id_fkey`, `checklist_items_task_id_fkey` (FR4)

## 3. Verification — cross-tenant isolation (automated)

- [x] 3.1 Integration test in the existing poison-pill suite: two users push records with the same UUID — both get `status: created`, both rows exist (U2, FR7, M4)
- [x] 3.2 Integration test in the existing poison-pill suite: user B pushes a task with `goal_id` of user A's goal — response is `status: rejected, reason: fk_violation:goal_id`; a repeat push after client-side self-healing succeeds (U3, FR7, M4)

## 4. Verification — no behavior regression

- [x] 4.1 Run the entire existing integration suite (`packages/integration`) — once, at the end, no parallel runs (M1, FR6, NFR-P1)
- [x] 4.2 Confirm the migration file count is still 4 and client code (`pushRejectionHandler`, sync) required no changes (M2, FR5, FR6)

## 5. Manual follow-up (user)

- [x] 5.1 Recreate environments manually: `bash scripts/reset.sh dev|qa` (+ the prod project if needed) — outside the agent's scope
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-07-15
Loading
Loading