From b9f0ffb8bd513691638acf9ecf437f938ff3db64 Mon Sep 17 00:00:00 2001 From: oinsio <8233537+oinsio@users.noreply.github.com> Date: Wed, 15 Jul 2026 10:23:10 +0500 Subject: [PATCH 1/9] add add-composite-tenant-pk OpenSpec --- .../add-composite-tenant-pk/.openspec.yaml | 2 + .../changes/add-composite-tenant-pk/design.md | 75 +++++++++++++ .../add-composite-tenant-pk/proposal.md | 106 ++++++++++++++++++ .../specs/supabase-schema/spec.md | 44 ++++++++ .../changes/add-composite-tenant-pk/tasks.md | 27 +++++ 5 files changed, 254 insertions(+) create mode 100644 openspec/changes/add-composite-tenant-pk/.openspec.yaml create mode 100644 openspec/changes/add-composite-tenant-pk/design.md create mode 100644 openspec/changes/add-composite-tenant-pk/proposal.md create mode 100644 openspec/changes/add-composite-tenant-pk/specs/supabase-schema/spec.md create mode 100644 openspec/changes/add-composite-tenant-pk/tasks.md diff --git a/openspec/changes/add-composite-tenant-pk/.openspec.yaml b/openspec/changes/add-composite-tenant-pk/.openspec.yaml new file mode 100644 index 00000000..4f63482c --- /dev/null +++ b/openspec/changes/add-composite-tenant-pk/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-15 diff --git a/openspec/changes/add-composite-tenant-pk/design.md b/openspec/changes/add-composite-tenant-pk/design.md new file mode 100644 index 00000000..198c0bb8 --- /dev/null +++ b/openspec/changes/add-composite-tenant-pk/design.md @@ -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 `__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-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_
_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 ()`. +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 `). + +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)._ diff --git a/openspec/changes/add-composite-tenant-pk/proposal.md b/openspec/changes/add-composite-tenant-pk/proposal.md new file mode 100644 index 00000000..1c662068 --- /dev/null +++ b/openspec/changes/add-composite-tenant-pk/proposal.md @@ -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 ()` form); `user_id` is never nulled. +- FR4: FK constraint names keep the `
__fkey` format (set explicitly) so the rejection reason parsing in `push_records` (`fk_violation:`) 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)._ diff --git a/openspec/changes/add-composite-tenant-pk/specs/supabase-schema/spec.md b/openspec/changes/add-composite-tenant-pk/specs/supabase-schema/spec.md new file mode 100644 index 00000000..77e98f75 --- /dev/null +++ b/openspec/changes/add-composite-tenant-pk/specs/supabase-schema/spec.md @@ -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 `
__fkey` format (e.g. `tasks_goal_id_fkey`) so structured rejection reasons (`fk_violation:`) 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 ()`) 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 diff --git a/openspec/changes/add-composite-tenant-pk/tasks.md b/openspec/changes/add-composite-tenant-pk/tasks.md new file mode 100644 index 00000000..3760f3b9 --- /dev/null +++ b/openspec/changes/add-composite-tenant-pk/tasks.md @@ -0,0 +1,27 @@ +# Tasks: add-composite-tenant-pk + +## 1. Schema migration (in-place edits) + +- [ ] 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) +- [ ] 1.2 `001_create_tables.sql`: rewrite FKs `tasks.goal_id`/`context_id`/`category_id` as composite `FOREIGN KEY (user_id, ) REFERENCES (user_id, id)` with explicit names `tasks__fkey` and `ON DELETE SET NULL ()`, preserving `DEFERRABLE INITIALLY DEFERRED` (FR2, FR3, FR4) +- [ ] 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) +- [ ] 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 + +- [ ] 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 ()` syntax (M3) +- [ ] 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) + +- [ ] 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) +- [ ] 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 + +- [ ] 4.1 Run the entire existing integration suite (`packages/integration`) — once, at the end, no parallel runs (M1, FR6, NFR-P1) +- [ ] 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) + +- [ ] 5.1 Recreate environments manually: `bash scripts/reset.sh dev|qa` (+ the prod project if needed) — outside the agent's scope From 6aa5d3c85da1e4c55d9a9be629e8ef703c37da3b Mon Sep 17 00:00:00 2001 From: oinsio <8233537+oinsio@users.noreply.github.com> Date: Wed, 15 Jul 2026 11:28:57 +0500 Subject: [PATCH 2/9] implement add-composite-tenant-pk OpenSpec --- .../changes/add-composite-tenant-pk/tasks.md | 20 +- .../supabase/migrations/001_create_tables.sql | 47 ++-- .../integration/src/cross-tenant-helpers.ts | 100 ++++++++ .../integration/src/cross-tenant-payloads.ts | 158 ++++++++++++ packages/integration/src/test-helpers.ts | 10 + .../push-poison-pill-cross-tenant.spec.ts | 235 ++++++++++++++++++ 6 files changed, 542 insertions(+), 28 deletions(-) create mode 100644 packages/integration/src/cross-tenant-helpers.ts create mode 100644 packages/integration/src/cross-tenant-payloads.ts create mode 100644 packages/integration/src/tests/push-poison-pill-cross-tenant.spec.ts diff --git a/openspec/changes/add-composite-tenant-pk/tasks.md b/openspec/changes/add-composite-tenant-pk/tasks.md index 3760f3b9..9f49b6a6 100644 --- a/openspec/changes/add-composite-tenant-pk/tasks.md +++ b/openspec/changes/add-composite-tenant-pk/tasks.md @@ -2,25 +2,25 @@ ## 1. Schema migration (in-place edits) -- [ ] 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) -- [ ] 1.2 `001_create_tables.sql`: rewrite FKs `tasks.goal_id`/`context_id`/`category_id` as composite `FOREIGN KEY (user_id, ) REFERENCES (user_id, id)` with explicit names `tasks__fkey` and `ON DELETE SET NULL ()`, preserving `DEFERRABLE INITIALLY DEFERRED` (FR2, FR3, FR4) -- [ ] 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) -- [ ] 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) +- [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, ) REFERENCES (user_id, id)` with explicit names `tasks__fkey` and `ON DELETE SET NULL ()`, 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 -- [ ] 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 ()` syntax (M3) -- [ ] 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) +- [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 ()` 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) -- [ ] 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) -- [ ] 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) +- [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 -- [ ] 4.1 Run the entire existing integration suite (`packages/integration`) — once, at the end, no parallel runs (M1, FR6, NFR-P1) -- [ ] 4.2 Confirm the migration file count is still 4 and client code (`pushRejectionHandler`, sync) required no changes (M2, FR5, FR6) +- [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) diff --git a/packages/adapter-supabase/supabase/migrations/001_create_tables.sql b/packages/adapter-supabase/supabase/migrations/001_create_tables.sql index ecf84315..f4d71951 100644 --- a/packages/adapter-supabase/supabase/migrations/001_create_tables.sql +++ b/packages/adapter-supabase/supabase/migrations/001_create_tables.sql @@ -77,7 +77,7 @@ CREATE INDEX IF NOT EXISTS idx_files_user_hash ON files (user_id, data_hash); -- ─── Attachments table (FR5 of add-file-attachments) ──────────────────────── CREATE TABLE IF NOT EXISTS attachments ( - id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + id UUID NOT NULL DEFAULT gen_random_uuid(), user_id UUID NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE, entity_type TEXT NOT NULL CHECK (entity_type IN ('task', 'goal', 'idea')), entity_id UUID NOT NULL, @@ -89,7 +89,8 @@ CREATE TABLE IF NOT EXISTS attachments ( is_deleted BOOLEAN NOT NULL DEFAULT FALSE, created_at TIMESTAMPTZ NOT NULL, updated_at TIMESTAMPTZ NOT NULL, - revision BIGINT NOT NULL DEFAULT 0 + revision BIGINT NOT NULL DEFAULT 0, + PRIMARY KEY (user_id, id) ); -- implements FR10, NFR-P2 of fix-pull-pagination: composite index for keyset pagination @@ -102,31 +103,33 @@ CREATE INDEX IF NOT EXISTS idx_attachments_entity -- ─── Entity tables (dependency order for FK constraints, FR18) ──────────────── CREATE TABLE IF NOT EXISTS contexts ( - id UUID PRIMARY KEY, + id UUID NOT NULL, user_id UUID NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE, name TEXT NOT NULL, sort_order TEXT NOT NULL DEFAULT '0', is_deleted BOOLEAN NOT NULL DEFAULT FALSE, created_at TIMESTAMPTZ NOT NULL, updated_at TIMESTAMPTZ NOT NULL, - revision BIGINT NOT NULL DEFAULT 0 + revision BIGINT NOT NULL DEFAULT 0, + PRIMARY KEY (user_id, id) ); CREATE INDEX IF NOT EXISTS idx_contexts_user_revision_id ON contexts (user_id, revision, id); CREATE TABLE IF NOT EXISTS categories ( - id UUID PRIMARY KEY, + id UUID NOT NULL, user_id UUID NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE, name TEXT NOT NULL, sort_order TEXT NOT NULL DEFAULT '0', is_deleted BOOLEAN NOT NULL DEFAULT FALSE, created_at TIMESTAMPTZ NOT NULL, updated_at TIMESTAMPTZ NOT NULL, - revision BIGINT NOT NULL DEFAULT 0 + revision BIGINT NOT NULL DEFAULT 0, + PRIMARY KEY (user_id, id) ); CREATE INDEX IF NOT EXISTS idx_categories_user_revision_id ON categories (user_id, revision, id); CREATE TABLE IF NOT EXISTS goals ( - id UUID PRIMARY KEY, + id UUID NOT NULL, user_id UUID NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE, name TEXT NOT NULL, description TEXT NOT NULL DEFAULT '', @@ -136,12 +139,13 @@ CREATE TABLE IF NOT EXISTS goals ( is_deleted BOOLEAN NOT NULL DEFAULT FALSE, created_at TIMESTAMPTZ NOT NULL, updated_at TIMESTAMPTZ NOT NULL, - revision BIGINT NOT NULL DEFAULT 0 + revision BIGINT NOT NULL DEFAULT 0, + PRIMARY KEY (user_id, id) ); CREATE INDEX IF NOT EXISTS idx_goals_user_revision_id ON goals (user_id, revision, id); CREATE TABLE IF NOT EXISTS ideas ( - id UUID PRIMARY KEY, + id UUID NOT NULL, user_id UUID NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE, name TEXT NOT NULL, description TEXT NOT NULL DEFAULT '', @@ -149,19 +153,20 @@ CREATE TABLE IF NOT EXISTS ideas ( is_deleted BOOLEAN NOT NULL DEFAULT FALSE, created_at TIMESTAMPTZ NOT NULL, updated_at TIMESTAMPTZ NOT NULL, - revision BIGINT NOT NULL DEFAULT 0 + revision BIGINT NOT NULL DEFAULT 0, + PRIMARY KEY (user_id, id) ); CREATE INDEX IF NOT EXISTS idx_ideas_user_revision_id ON ideas (user_id, revision, id); CREATE TABLE IF NOT EXISTS tasks ( - id UUID PRIMARY KEY, + id UUID NOT NULL, user_id UUID NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE, name TEXT NOT NULL, description TEXT NOT NULL DEFAULT '', box TEXT NOT NULL CHECK (box IN ('inbox', 'today', 'week', 'later')), - goal_id UUID REFERENCES goals(id) ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED, - context_id UUID REFERENCES contexts(id) ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED, - category_id UUID REFERENCES categories(id) ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED, + goal_id UUID, + context_id UUID, + category_id UUID, is_completed BOOLEAN NOT NULL DEFAULT FALSE, completed_at TIMESTAMPTZ, repeat_rule JSONB, @@ -173,20 +178,26 @@ CREATE TABLE IF NOT EXISTS tasks ( is_deleted BOOLEAN NOT NULL DEFAULT FALSE, created_at TIMESTAMPTZ NOT NULL, updated_at TIMESTAMPTZ NOT NULL, - revision BIGINT NOT NULL DEFAULT 0 + revision BIGINT NOT NULL DEFAULT 0, + PRIMARY KEY (user_id, id), + 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, + 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, + 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 ); CREATE INDEX IF NOT EXISTS idx_tasks_user_revision_id ON tasks (user_id, revision, id); CREATE TABLE IF NOT EXISTS checklist_items ( - id UUID PRIMARY KEY, + id UUID NOT NULL, user_id UUID NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE, - task_id UUID NOT NULL REFERENCES tasks(id) ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED, + task_id UUID NOT NULL, name TEXT NOT NULL, is_completed BOOLEAN NOT NULL DEFAULT FALSE, sort_order TEXT NOT NULL DEFAULT '0', is_deleted BOOLEAN NOT NULL DEFAULT FALSE, created_at TIMESTAMPTZ NOT NULL, updated_at TIMESTAMPTZ NOT NULL, - revision BIGINT NOT NULL DEFAULT 0 + revision BIGINT NOT NULL DEFAULT 0, + PRIMARY KEY (user_id, id), + CONSTRAINT checklist_items_task_id_fkey FOREIGN KEY (user_id, task_id) REFERENCES tasks (user_id, id) ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED ); CREATE INDEX IF NOT EXISTS idx_checklist_items_user_revision_id ON checklist_items (user_id, revision, id); diff --git a/packages/integration/src/cross-tenant-helpers.ts b/packages/integration/src/cross-tenant-helpers.ts new file mode 100644 index 00000000..c7bf68e3 --- /dev/null +++ b/packages/integration/src/cross-tenant-helpers.ts @@ -0,0 +1,100 @@ +// implements FR7 of add-composite-tenant-pk +import { readTestConfig } from "./config.js"; +import type { ServerCallCredentials } from "./server-api.js"; + +/** Strong password used for all isolated test users. */ +const TEST_USER_PASSWORD = "IntegrationTest-Passw0rd!"; + +/** GoTrue admin endpoint for creating confirmed users. */ +const ADMIN_USERS_ENDPOINT = "/auth/v1/admin/users"; + +/** GoTrue token endpoint using the password grant. */ +const PASSWORD_GRANT_ENDPOINT = "/auth/v1/token?grant_type=password"; + +/** Edge Function that initializes a user's sync_meta counters. */ +const INIT_ENDPOINT = "/functions/v1/init"; + +const CONTENT_TYPE_JSON = "application/json"; + +interface PasswordGrantResponse { + access_token?: string; +} + +/** + * Provisions a fresh, isolated user via the GoTrue admin API and returns + * ready-to-use server credentials. Each call creates an independent tenant, + * enabling cross-tenant integration tests (two users pushing the same UUID). + * + * Implements FR7 of add-composite-tenant-pk. + */ +export async function createIsolatedUser( + emailPrefix: string, +): Promise { + const { supabaseUrl, anonKey, serviceRoleKey } = readTestConfig(); + + const email = `${emailPrefix}-${Date.now()}@example.com`; + + // 1. Admin-create a confirmed user. + const createUserResponse = await fetch( + `${supabaseUrl}${ADMIN_USERS_ENDPOINT}`, + { + method: "POST", + headers: { + apikey: serviceRoleKey, + Authorization: `Bearer ${serviceRoleKey}`, + "Content-Type": CONTENT_TYPE_JSON, + }, + body: JSON.stringify({ + email, + password: TEST_USER_PASSWORD, + email_confirm: true, + }), + }, + ); + if (!createUserResponse.ok) { + throw new Error( + `admin create user failed: ${createUserResponse.status} ${await createUserResponse.text()}`, + ); + } + + // 2. Sign in via password grant to obtain an access token. + const tokenResponse = await fetch( + `${supabaseUrl}${PASSWORD_GRANT_ENDPOINT}`, + { + method: "POST", + headers: { + apikey: anonKey, + "Content-Type": CONTENT_TYPE_JSON, + }, + body: JSON.stringify({ email, password: TEST_USER_PASSWORD }), + }, + ); + if (!tokenResponse.ok) { + throw new Error( + `password grant failed: ${tokenResponse.status} ${await tokenResponse.text()}`, + ); + } + const { access_token: accessToken } = + (await tokenResponse.json()) as PasswordGrantResponse; + if (!accessToken) { + throw new Error("password grant response missing access_token"); + } + + // 3. Initialize the user's sync counters (creates sync_meta rows). + const initResponse = await fetch(`${supabaseUrl}${INIT_ENDPOINT}`, { + method: "POST", + headers: { + Authorization: `Bearer ${accessToken}`, + apikey: anonKey, + "Content-Type": CONTENT_TYPE_JSON, + }, + body: JSON.stringify({}), + }); + if (!initResponse.ok) { + throw new Error( + `init failed: ${initResponse.status} ${await initResponse.text()}`, + ); + } + + return { accessToken, supabaseUrl, anonKey }; +} diff --git a/packages/integration/src/cross-tenant-payloads.ts b/packages/integration/src/cross-tenant-payloads.ts new file mode 100644 index 00000000..ac816150 --- /dev/null +++ b/packages/integration/src/cross-tenant-payloads.ts @@ -0,0 +1,158 @@ +// implements U3, FR7, M4 of add-composite-tenant-pk +// Push-payload builders for cross-tenant FK-ownership integration tests. +// Every builder returns a full payload with all top-level entity keys present +// (empty arrays for unused ones), as required by the push Edge Function. + +/** Options for a task push record (only cross-tenant-relevant fields vary). */ +export interface TaskPayloadOptions { + id: string; + name: string; + goalId?: string; + contextId?: string; + categoryId?: string; +} + +function emptyPayload(): Record { + return { + tasks: [], + goals: [], + ideas: [], + contexts: [], + categories: [], + checklist_items: [], + attachments: [], + settings: [], + }; +} + +/** Builds a full push payload carrying a single goal record. */ +export function buildGoalPayload( + id: string, + name: string, +): Record { + const now = new Date().toISOString(); + return { + ...emptyPayload(), + goals: [ + { + id, + name, + description: "", + status: "planning", + cover_hash: "", + sort_order: "0", + is_deleted: false, + created_at: now, + updated_at: now, + version: 1, + revision: 0, + }, + ], + }; +} + +/** Builds a full push payload carrying a single context record. */ +export function buildContextPayload( + id: string, + name: string, +): Record { + const now = new Date().toISOString(); + return { + ...emptyPayload(), + contexts: [ + { + id, + name, + sort_order: "0", + is_deleted: false, + created_at: now, + updated_at: now, + version: 1, + revision: 0, + }, + ], + }; +} + +/** Builds a full push payload carrying a single category record. */ +export function buildCategoryPayload( + id: string, + name: string, +): Record { + const now = new Date().toISOString(); + return { + ...emptyPayload(), + categories: [ + { + id, + name, + sort_order: "0", + is_deleted: false, + created_at: now, + updated_at: now, + version: 1, + revision: 0, + }, + ], + }; +} + +/** Builds a full push payload carrying a single task record. */ +export function buildTaskPayload( + options: TaskPayloadOptions, +): Record { + const now = new Date().toISOString(); + return { + ...emptyPayload(), + tasks: [ + { + id: options.id, + name: options.name, + description: "", + box: "inbox", + is_completed: false, + is_deleted: false, + completed_at: "", + next_date: "", + appear_date: "", + context_id: options.contextId ?? "", + category_id: options.categoryId ?? "", + goal_id: options.goalId ?? "", + repeat_rule: "", + is_hidden: false, + original_task_id: "", + sort_order: "0", + created_at: now, + updated_at: now, + version: 1, + revision: 0, + }, + ], + }; +} + +/** Builds a full push payload carrying a single checklist_items record. */ +export function buildChecklistPayload( + id: string, + taskId: string, + name: string, +): Record { + const now = new Date().toISOString(); + return { + ...emptyPayload(), + checklist_items: [ + { + id, + task_id: taskId, + name, + is_completed: false, + sort_order: "0", + is_deleted: false, + created_at: now, + updated_at: now, + version: 1, + revision: 0, + }, + ], + }; +} diff --git a/packages/integration/src/test-helpers.ts b/packages/integration/src/test-helpers.ts index e069c703..58e69ada 100644 --- a/packages/integration/src/test-helpers.ts +++ b/packages/integration/src/test-helpers.ts @@ -1,5 +1,15 @@ // implements FR6 of add-supabase-integration-tests // Barrel re-export — actual code lives in focused modules. + +export { createIsolatedUser } from "./cross-tenant-helpers.js"; +export { + buildCategoryPayload, + buildChecklistPayload, + buildContextPayload, + buildGoalPayload, + buildTaskPayload, + type TaskPayloadOptions, +} from "./cross-tenant-payloads.js"; export type { AuthenticatedContext } from "./page-lifecycle.js"; export { closeAuthenticatedPage, diff --git a/packages/integration/src/tests/push-poison-pill-cross-tenant.spec.ts b/packages/integration/src/tests/push-poison-pill-cross-tenant.spec.ts new file mode 100644 index 00000000..436de489 --- /dev/null +++ b/packages/integration/src/tests/push-poison-pill-cross-tenant.spec.ts @@ -0,0 +1,235 @@ +// implements U2, U3, FR7, M4 of add-composite-tenant-pk +// Test 2 below also behaviorally verifies FR4 / task 2.2: the four composite +// FK constraint names (tasks_goal_id_fkey, tasks_context_id_fkey, +// tasks_category_id_fkey, checklist_items_task_id_fkey) are confirmed via the +// fk_violation: reason strings the push RPC derives from constraint names. +import { randomUUID } from "node:crypto"; +import { expect, test } from "@playwright/test"; +import { + buildCategoryPayload, + buildChecklistPayload, + buildContextPayload, + buildGoalPayload, + buildTaskPayload, +} from "../cross-tenant-payloads.js"; +import { + createIsolatedUser, + pullFromServer, + pushToServer, + type ServerCallCredentials, +} from "../test-helpers.js"; + +interface PushResult { + id: string; + status: string; + reason?: string; +} + +interface CrossTenantPushResponse { + ok: boolean; + results: { + goals?: PushResult[]; + tasks?: PushResult[]; + contexts?: PushResult[]; + categories?: PushResult[]; + checklist_items?: PushResult[]; + }; +} + +interface CrossTenantPullResponse { + ok: boolean; + contexts: Array<{ id: string; name: string; is_deleted: boolean }>; + tasks: Array<{ id: string; goal_id: string; is_deleted: boolean }>; +} + +test.describe.configure({ mode: "serial" }); + +let userA: ServerCallCredentials; +let userB: ServerCallCredentials; + +test.beforeAll(async () => { + userA = await createIsolatedUser("cross-tenant-a"); + userB = await createIsolatedUser("cross-tenant-b"); +}); + +test("two users push records with the same UUID → both created, both rows exist", async () => { + const sharedId = randomUUID(); + const nameA = `Context A ${Date.now()}`; + const nameB = `Context B ${Date.now()}`; + + // User A pushes a context with the shared UUID. + const pushA = (await pushToServer( + userA, + buildContextPayload(sharedId, nameA), + )) as unknown as CrossTenantPushResponse; + expect(pushA.ok).toBe(true); + expect(pushA.results.contexts?.[0]?.status).toBe("created"); + + // User B pushes a context reusing the SAME UUID → must also succeed, + // proving the composite (user_id, id) PK removed the global uniqueness. + const pushB = (await pushToServer( + userB, + buildContextPayload(sharedId, nameB), + )) as unknown as CrossTenantPushResponse; + expect(pushB.ok).toBe(true); + expect(pushB.results.contexts?.[0]?.status).toBe("created"); + + // Each user pulls only their own row for the shared UUID. + const pullA = await pullFromServer(userA); + const contextA = pullA.contexts.find((context) => context.id === sharedId); + expect(contextA).toBeDefined(); + expect(contextA?.name).toBe(nameA); + + const pullB = await pullFromServer(userB); + const contextB = pullB.contexts.find((context) => context.id === sharedId); + expect(contextB).toBeDefined(); + expect(contextB?.name).toBe(nameB); +}); + +test("user B task referencing user A's goal → fk_violation:goal_id → self-heal → created", async () => { + // 1. User A creates a goal. + const userAGoalId = randomUUID(); + const goalPush = (await pushToServer( + userA, + buildGoalPayload(userAGoalId, `Goal A ${Date.now()}`), + )) as unknown as CrossTenantPushResponse; + expect(goalPush.results.goals?.[0]?.status).toBe("created"); + + // 2. User B pushes a task referencing user A's goal (cross-tenant ref). + const taskId = randomUUID(); + const rejectedPush = (await pushToServer( + userB, + buildTaskPayload({ + id: taskId, + name: `Task B ${Date.now()}`, + goalId: userAGoalId, + }), + )) as unknown as CrossTenantPushResponse; + const rejectedResult = rejectedPush.results.tasks?.find( + (result) => result.id === taskId, + ); + expect(rejectedResult?.status).toBe("rejected"); + expect(rejectedResult?.reason).toBe("fk_violation:goal_id"); + + // 3. Self-heal: user B re-pushes the SAME task with the reference cleared. + const healedPush = (await pushToServer( + userB, + buildTaskPayload({ id: taskId, name: `Task B ${Date.now()}`, goalId: "" }), + )) as unknown as CrossTenantPushResponse; + const healedResult = healedPush.results.tasks?.find( + (result) => result.id === taskId, + ); + expect(healedResult?.status).toBe("created"); + + // 4. Pull user B's data — the task exists with the reference cleared. + const pullB = await pullFromServer(userB); + const healedTask = pullB.tasks.find((task) => task.id === taskId); + expect(healedTask).toBeDefined(); + expect(healedTask?.goal_id).toBe(""); + expect(healedTask?.is_deleted).toBe(false); +}); + +test("cross-tenant references are rejected with the correct fk_violation field for every FK", async () => { + // User A creates a goal, a context, a category, and a task. + const userAGoalId2 = randomUUID(); + const userAContextId = randomUUID(); + const userACategoryId = randomUUID(); + const userATaskId = randomUUID(); + + for (const [payload, entity, ownerId] of [ + [buildGoalPayload(userAGoalId2, "Goal A2"), "goals", userAGoalId2], + [buildContextPayload(userAContextId, "Ctx A"), "contexts", userAContextId], + [ + buildCategoryPayload(userACategoryId, "Cat A"), + "categories", + userACategoryId, + ], + [ + buildTaskPayload({ id: userATaskId, name: "Task A" }), + "tasks", + userATaskId, + ], + ] as const) { + const response = (await pushToServer( + userA, + payload, + )) as unknown as CrossTenantPushResponse; + expect( + response.results[entity]?.find((result) => result.id === ownerId)?.status, + ).toBe("created"); + } + + // Each case: user B references a record owned by user A → the reason string + // encodes the FK field, which proves the corresponding constraint NAME + // (tasks_context_id_fkey, tasks_category_id_fkey, tasks_goal_id_fkey, + // checklist_items_task_id_fkey). Verifies FR4 / task 2.2 behaviorally. + // + // NOTE on the checklist case: the push RPC derives the field via the regexp + // `^.*?_(.+?)_fkey$`. The table name `checklist_items` itself contains an + // underscore, so that regexp yields `items_task_id` (not `task_id`) from + // `checklist_items_task_id_fkey`. This is pre-existing behavior — the old + // inline FK auto-generated the identical constraint name — and is therefore + // unchanged by this change (FR6). The `items_task_id` reason still uniquely + // confirms the constraint name is `checklist_items_task_id_fkey`. + const [ctxId, catId, goalId, itemId] = [ + randomUUID(), + randomUUID(), + randomUUID(), + randomUUID(), + ]; + const fkCases: Array<{ + entity: keyof CrossTenantPushResponse["results"]; + id: string; + reason: string; + payload: Record; + }> = [ + { + entity: "tasks", + id: ctxId, + reason: "fk_violation:context_id", + payload: buildTaskPayload({ + id: ctxId, + name: "Viol", + contextId: userAContextId, + }), + }, + { + entity: "tasks", + id: catId, + reason: "fk_violation:category_id", + payload: buildTaskPayload({ + id: catId, + name: "Viol", + categoryId: userACategoryId, + }), + }, + { + entity: "tasks", + id: goalId, + reason: "fk_violation:goal_id", + payload: buildTaskPayload({ + id: goalId, + name: "Viol", + goalId: userAGoalId2, + }), + }, + { + entity: "checklist_items", + id: itemId, + reason: "fk_violation:items_task_id", + payload: buildChecklistPayload(itemId, userATaskId, "Item"), + }, + ]; + + for (const fkCase of fkCases) { + const response = (await pushToServer( + userB, + fkCase.payload, + )) as unknown as CrossTenantPushResponse; + const result = response.results[fkCase.entity]?.find( + (record) => record.id === fkCase.id, + ); + expect(result?.status, fkCase.reason).toBe("rejected"); + expect(result?.reason, fkCase.reason).toBe(fkCase.reason); + } +}); From 989f221a08233b04a505f4bc698922fbae0930c9 Mon Sep 17 00:00:00 2001 From: oinsio <8233537+oinsio@users.noreply.github.com> Date: Wed, 15 Jul 2026 11:53:51 +0500 Subject: [PATCH 3/9] archive add-composite-tenant-pk OpenSpec --- .../.openspec.yaml | 0 .../design.md | 0 .../proposal.md | 0 .../specs/supabase-schema/spec.md | 0 .../tasks.md | 2 +- openspec/specs/supabase-schema/spec.md | 24 ++++++++++++------- 6 files changed, 17 insertions(+), 9 deletions(-) rename openspec/changes/{add-composite-tenant-pk => archive/2026/07/2026-07-15-add-composite-tenant-pk}/.openspec.yaml (100%) rename openspec/changes/{add-composite-tenant-pk => archive/2026/07/2026-07-15-add-composite-tenant-pk}/design.md (100%) rename openspec/changes/{add-composite-tenant-pk => archive/2026/07/2026-07-15-add-composite-tenant-pk}/proposal.md (100%) rename openspec/changes/{add-composite-tenant-pk => archive/2026/07/2026-07-15-add-composite-tenant-pk}/specs/supabase-schema/spec.md (100%) rename openspec/changes/{add-composite-tenant-pk => archive/2026/07/2026-07-15-add-composite-tenant-pk}/tasks.md (97%) diff --git a/openspec/changes/add-composite-tenant-pk/.openspec.yaml b/openspec/changes/archive/2026/07/2026-07-15-add-composite-tenant-pk/.openspec.yaml similarity index 100% rename from openspec/changes/add-composite-tenant-pk/.openspec.yaml rename to openspec/changes/archive/2026/07/2026-07-15-add-composite-tenant-pk/.openspec.yaml diff --git a/openspec/changes/add-composite-tenant-pk/design.md b/openspec/changes/archive/2026/07/2026-07-15-add-composite-tenant-pk/design.md similarity index 100% rename from openspec/changes/add-composite-tenant-pk/design.md rename to openspec/changes/archive/2026/07/2026-07-15-add-composite-tenant-pk/design.md diff --git a/openspec/changes/add-composite-tenant-pk/proposal.md b/openspec/changes/archive/2026/07/2026-07-15-add-composite-tenant-pk/proposal.md similarity index 100% rename from openspec/changes/add-composite-tenant-pk/proposal.md rename to openspec/changes/archive/2026/07/2026-07-15-add-composite-tenant-pk/proposal.md diff --git a/openspec/changes/add-composite-tenant-pk/specs/supabase-schema/spec.md b/openspec/changes/archive/2026/07/2026-07-15-add-composite-tenant-pk/specs/supabase-schema/spec.md similarity index 100% rename from openspec/changes/add-composite-tenant-pk/specs/supabase-schema/spec.md rename to openspec/changes/archive/2026/07/2026-07-15-add-composite-tenant-pk/specs/supabase-schema/spec.md diff --git a/openspec/changes/add-composite-tenant-pk/tasks.md b/openspec/changes/archive/2026/07/2026-07-15-add-composite-tenant-pk/tasks.md similarity index 97% rename from openspec/changes/add-composite-tenant-pk/tasks.md rename to openspec/changes/archive/2026/07/2026-07-15-add-composite-tenant-pk/tasks.md index 9f49b6a6..1e557149 100644 --- a/openspec/changes/add-composite-tenant-pk/tasks.md +++ b/openspec/changes/archive/2026/07/2026-07-15-add-composite-tenant-pk/tasks.md @@ -24,4 +24,4 @@ ## 5. Manual follow-up (user) -- [ ] 5.1 Recreate environments manually: `bash scripts/reset.sh dev|qa` (+ the prod project if needed) — outside the agent's scope +- [x] 5.1 Recreate environments manually: `bash scripts/reset.sh dev|qa` (+ the prod project if needed) — outside the agent's scope diff --git a/openspec/specs/supabase-schema/spec.md b/openspec/specs/supabase-schema/spec.md index 04cfb8d2..5050454c 100644 --- a/openspec/specs/supabase-schema/spec.md +++ b/openspec/specs/supabase-schema/spec.md @@ -1,20 +1,24 @@ ## 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. The `id` column SHALL be `UUID PRIMARY KEY`. Each table SHALL have a composite index on `(user_id, revision)` for efficient pull queries. +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 PK), `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) +- **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 PK), `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) +- **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: Settings table The `settings` table SHALL have composite primary key `(user_id, key)`. It SHALL NOT use revision-based tracking; `updated_at` is used for filtering instead. @@ -38,13 +42,13 @@ The `covers` table SHALL store metadata for uploaded cover images. The actual fi - **AND** index `idx_covers_user_hash ON covers (user_id, data_hash)` exists ### Requirement: FK constraints on reference fields -FK reference fields SHALL use `UUID` type with `DEFERRABLE INITIALLY DEFERRED` constraints. Nullable FK fields store `NULL` when unset. 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. +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 `
__fkey` format (e.g. `tasks_goal_id_fkey`) so structured rejection reasons (`fk_violation:`) 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 ()`) 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.goal_id` has FK to `goals(id)` ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED -- **AND** `tasks.context_id` has FK to `contexts(id)` ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED -- **AND** `tasks.category_id` has FK to `categories(id)` ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED +- **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 @@ -53,7 +57,11 @@ FK reference fields SHALL use `UUID` type with `DEFERRABLE INITIALLY DEFERRED` c #### Scenario: Checklist item FK field references tasks - **WHEN** migration is applied -- **THEN** `checklist_items.task_id` has FK to `tasks(id)` ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED +- **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 ### Requirement: RPC push_records uses per-record exception handling The database SHALL have a PostgreSQL function `push_records` callable via `supabase.rpc('push_records', ...)`. The function SHALL accept user_id and entity arrays (tasks, goals, contexts, categories, ideas, checklist_items, settings). It SHALL acquire a `FOR UPDATE` lock on the user's `next_revision` row in `sync_meta` (with 10-second timeout), assign the current revision to all accepted records, upsert records into entity tables in dependency order, increment `next_revision`, and return per-record results with status (`accepted`, `conflict`, `rejected`). Goal records SHALL use `cover_hash` TEXT field directly (no UUID cast needed). The RPC SHALL use `SET CONSTRAINTS ALL IMMEDIATE` at the beginning of execution. Each record in the processing loop SHALL be wrapped in `BEGIN...EXCEPTION WHEN OTHERS` to catch per-record errors without rolling back the entire transaction. Records that cause exceptions SHALL be returned with `status: "rejected"` and a structured `reason`. From 52842d397c674cc30a5f7103f255fdf6ebcbaf6d Mon Sep 17 00:00:00 2001 From: oinsio <8233537+oinsio@users.noreply.github.com> Date: Wed, 15 Jul 2026 11:55:14 +0500 Subject: [PATCH 4/9] add fix-checklist-fk-self-heal OpenSpec --- .../fix-checklist-fk-self-heal/.openspec.yaml | 2 + .../fix-checklist-fk-self-heal/design.md | 74 ++++++++++++++ .../fix-checklist-fk-self-heal/proposal.md | 98 +++++++++++++++++++ .../specs/push-poison-pill-protection/spec.md | 21 ++++ .../fix-checklist-fk-self-heal/tasks.md | 21 ++++ 5 files changed, 216 insertions(+) create mode 100644 openspec/changes/fix-checklist-fk-self-heal/.openspec.yaml create mode 100644 openspec/changes/fix-checklist-fk-self-heal/design.md create mode 100644 openspec/changes/fix-checklist-fk-self-heal/proposal.md create mode 100644 openspec/changes/fix-checklist-fk-self-heal/specs/push-poison-pill-protection/spec.md create mode 100644 openspec/changes/fix-checklist-fk-self-heal/tasks.md diff --git a/openspec/changes/fix-checklist-fk-self-heal/.openspec.yaml b/openspec/changes/fix-checklist-fk-self-heal/.openspec.yaml new file mode 100644 index 00000000..4f63482c --- /dev/null +++ b/openspec/changes/fix-checklist-fk-self-heal/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-15 diff --git a/openspec/changes/fix-checklist-fk-self-heal/design.md b/openspec/changes/fix-checklist-fk-self-heal/design.md new file mode 100644 index 00000000..6b490c5b --- /dev/null +++ b/openspec/changes/fix-checklist-fk-self-heal/design.md @@ -0,0 +1,74 @@ +# Design: fix-checklist-fk-self-heal + +## Context + +Driven by FR1–FR3 of the proposal. `push_records` (`003_create_push_rpc.sql`) turns a `23503` foreign-key violation into a structured reason string: + +```sql +WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*?_(.+?)_fkey$', '\1') +``` + +The pattern `^.*?_(.+?)_fkey$` uses a lazy prefix `.*?_` that matches up to the **first** underscore, then captures everything before `_fkey`. This silently assumes the table name is a single token with no underscore: + +- `tasks_goal_id_fkey` → prefix `tasks_`, capture `goal_id` ✓ +- `checklist_items_task_id_fkey` → prefix `checklist_`, capture **`items_task_id`** ✗ (wanted `task_id`) + +The client `pushRejectionHandler` matches the extracted field against `DELETE_FK_FIELDS = {task_id}`; `items_task_id` matches nothing, so the orphaned checklist item is left permanently `rejected` instead of soft-deleted. The existing `push-poison-pill-protection` spec already documents the intended client behavior ("Stale task_id on checklist item is healed" → `fk_violation:task_id`), so the client side is already correct — only the server's field extraction is wrong. + +This regexp was already present before `add-composite-tenant-pk`; that change (correctly) named the composite FK `checklist_items_task_id_fkey` per the `
__fkey` convention, which is the same name Postgres auto-generated for the old inline FK. So the bug predates the composite-PK work and is unchanged by it — this change fixes it. + +## Goals / Non-Goals + +**Goals:** + +- Correct FK-field extraction for table names containing underscores (FR1, FR3), without renaming constraints or changing the client (FR4, FR5, NG1–NG2). + +**Non-Goals:** + +- Renaming FK constraints; changing the client; changing the clearable/deletable field sets; data migration; the GAS backend (NG1–NG5 from the proposal). + +## Decisions + +### D1: Underscore-safe regexp anchored on the trailing `_id` segment + +Implements FR1, FR2, FR3. Replace the pattern in all 8 inline `WHEN '23503'` branches with: + +```sql +WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*_([a-z]+_id)_fkey$', '\1') +``` + +The greedy prefix `^.*_` consumes as much as possible, so the capture `([a-z]+_id)` binds to the **last** `_id` segment immediately before `_fkey`: + +- `checklist_items_task_id_fkey` → `task_id` ✓ +- `tasks_goal_id_fkey` → `goal_id` ✓ +- `tasks_context_id_fkey` → `context_id` ✓ +- `tasks_category_id_fkey` → `category_id` ✓ + +This keeps the `
__fkey` naming convention intact and requires no client change. All FK reference fields in the schema follow the `_id` shape, so the pattern is exhaustive for the current model. If a constraint name does not match (e.g. a future FK field not ending in `_id`), `regexp_replace` returns the name unchanged — the same fallback behavior as today, so no regression. + +### D2: Edit all 8 occurrences in place; no shared helper (this change) + +The regexp is duplicated inline across the eight per-entity exception handlers. Replacing all eight identically is a minimal, low-risk diff. Extracting a shared plpgsql helper (`parse_fk_field(constraint_name)`) would reduce duplication but expands the diff and surface area; noted as a possible future refactor, out of scope here. + +### D3: In-place migration edit, manual environment recreation + +Implements FR4. Consistent with `add-composite-tenant-pk`: no production data yet, so `003_create_push_rpc.sql` is edited in place (file count stays 4, M4) and the user recreates environments via `scripts/reset.sh`. + +## Risks / Trade-offs + +- [Pattern assumes FK fields end in `_id`] → all current FK fields (`goal_id`, `context_id`, `category_id`, `task_id`) match; non-matching names fall back to the raw constraint name (no worse than today). Verified by the integration assertions. +- [Editing an already-applied migration diverges from deployed environments] → deliberate; the user re-runs `scripts/reset.sh` (same as the composite-PK change). Visible in git history. +- [Regexp change could accidentally alter the `tasks` FK reasons] → covered by the existing cross-tenant integration assertions for `goal_id`/`context_id`/`category_id` plus a full suite run (M2, M3). + +## Migration Plan + +1. Update the 8 `WHEN '23503'` branches in `003_create_push_rpc.sql`. +2. Update the checklist assertion in `push-poison-pill-cross-tenant.spec.ts` to expect `fk_violation:task_id`, and add an end-to-end self-heal assertion for an orphaned checklist item. +3. Run the integration suite once (Testcontainers rebuilds the schema). +4. The user recreates environments (`bash scripts/reset.sh `). + +Rollback: git revert of the migration edit + re-reset of environments. + +## Open Questions + +_none._ diff --git a/openspec/changes/fix-checklist-fk-self-heal/proposal.md b/openspec/changes/fix-checklist-fk-self-heal/proposal.md new file mode 100644 index 00000000..b82a7b3b --- /dev/null +++ b/openspec/changes/fix-checklist-fk-self-heal/proposal.md @@ -0,0 +1,98 @@ +# fix-checklist-fk-self-heal + +## Why + +The `push_records` RPC derives the structured rejection reason `fk_violation:` from the FK constraint name via the regexp `^.*?_(.+?)_fkey$`. That regexp assumes the table name has no underscore, so for `checklist_items_task_id_fkey` it extracts `items_task_id` instead of `task_id`. The client `pushRejectionHandler` only recognizes `task_id` (in `DELETE_FK_FIELDS`), so a checklist item whose parent task no longer exists is reported as `fk_violation:items_task_id`, matches nothing, and is left permanently `rejected` instead of being self-healed (soft-deleted). + +This is a pre-existing latent bug (the old inline FK auto-generated the same constraint name) that became reachable for cross-tenant references after `add-composite-tenant-pk` made `checklist_items.task_id` a composite ownership-enforcing FK. + +## What Changes + +- **MODIFIED**: the reason-extraction regexp in `push_records` (`003_create_push_rpc.sql`) becomes underscore-safe — it anchors on the trailing `_id` segment so `checklist_items_task_id_fkey` yields `task_id`, while `tasks_goal_id_fkey` / `tasks_context_id_fkey` / `tasks_category_id_fkey` keep yielding `goal_id` / `context_id` / `category_id`. +- FK constraint names are unchanged (`checklist_items_task_id_fkey` stays per convention). +- The client (`pushRejectionHandler`) is unchanged — it already handles `fk_violation:task_id` → soft-delete. +- The migration file is edited **in place** (no new file); the database is recreated manually. + +## Capabilities + +### New Capabilities + +_none_ + +### Modified Capabilities + +- `push-poison-pill-protection`: the server-side rejection-reason contract SHALL extract the FK field name correctly even when the table name contains underscores, so `checklist_items` FK violations are healable by the client. + +## Goals + +- G1: A checklist item referencing a non-existent/foreign task is rejected as `fk_violation:task_id` (not `items_task_id`). +- G2: The client self-heals such a checklist item by soft-deleting it, with no code change on the client. +- G3: Reason extraction for `tasks` FK fields (`goal_id`, `context_id`, `category_id`) is unchanged. + +## Non-Goals + +- NG1: Renaming FK constraints or changing the `
__fkey` naming convention. +- NG2: Any change to `pushRejectionHandler` or other client code. +- NG3: Changing which fields are clearable vs deletable (the `CLEARABLE_FK_FIELDS` / `DELETE_FK_FIELDS` sets stay as-is). +- NG4: Data migration — no production yet, the database is recreated manually. +- NG5: The GAS backend. + +## Users & Scenarios + +- U1: A user deletes/purges a task on device A; device B (offline) still has a checklist item under that task and pushes it. The server rejects it as `fk_violation:task_id`; the client soft-deletes the orphaned checklist item and the record settles. +- U2: A malicious/colliding user B pushes a checklist item referencing user A's task id. The composite FK rejects it as `fk_violation:task_id`; the client self-heals by soft-deleting. + +## Requirements + +### Functional + +- FR1: `push_records` SHALL return `reason: "fk_violation:task_id"` when a `checklist_items` insert violates `checklist_items_task_id_fkey` (SQLSTATE 23503). +- FR2: `push_records` SHALL continue to return `reason: "fk_violation:goal_id"`, `"fk_violation:context_id"`, `"fk_violation:category_id"` for the corresponding `tasks` FK violations. +- FR3: The reason-extraction logic SHALL be underscore-safe for table names — it extracts the trailing `_id` field, not the segment after the first underscore. +- FR4: The edit SHALL be applied in place in `003_create_push_rpc.sql`; no new migration file is created. +- FR5: No client code changes; `pushRejectionHandler` continues to soft-delete on `fk_violation:task_id` (`DELETE_FK_FIELDS`). + +### Non-Functional + +#### Performance + +- NFR-P1: The change is a string-literal edit to the reason CASE expression; it SHALL not alter the query plan or transaction behavior of `push_records`. + +#### Accessibility + +_not applicable — server-side change_ + +#### Responsive + +_not applicable — server-side change_ + +## UX Acceptance Criteria + +- UX1: The user notices no difference except that a checklist item orphaned from its task no longer sticks as a permanent "rejected" record — it self-heals silently like task FK references already do. + +## UI States Matrix + +_not applicable — no UI affected_ + +## Behavior + +No new user-facing UI. Behavior is verified by an integration test in the existing poison-pill suite (`packages/integration`) asserting the corrected reason string and, end-to-end, that the client soft-heals the orphaned checklist item. The existing `pushRejectionHandler` unit test already covers `fk_violation:task_id → is_deleted`. + +## Visual Reference + +_not applicable_ + +## Affected IA + +no changes + +## Success Metrics + +- M1: A `checklist_items` cross-tenant/orphan FK violation returns `fk_violation:task_id` (was `items_task_id`) — covered by an automated integration assertion. +- M2: The three `tasks` FK fields still return their correct `fk_violation:` reasons — covered by the existing cross-tenant assertions. +- M3: The full existing integration suite passes without changing expectations (100%), run once at the end. +- M4: Migration file count in `packages/adapter-supabase/supabase/migrations/` stays at 4. + +## Open Questions + +_none._ diff --git a/openspec/changes/fix-checklist-fk-self-heal/specs/push-poison-pill-protection/spec.md b/openspec/changes/fix-checklist-fk-self-heal/specs/push-poison-pill-protection/spec.md new file mode 100644 index 00000000..633c6c28 --- /dev/null +++ b/openspec/changes/fix-checklist-fk-self-heal/specs/push-poison-pill-protection/spec.md @@ -0,0 +1,21 @@ +# Delta: push-poison-pill-protection — fix-checklist-fk-self-heal + +## ADDED Requirements + +### Requirement: Server extracts the fk_violation field name correctly for all tables +The `push_records` RPC SHALL derive the structured rejection reason `fk_violation:` from the violated FK constraint name in a way that is correct even when the **table name contains underscores**. The extracted `` SHALL be the FK reference column (the trailing `_id` segment of the `
__fkey` constraint name), so that client self-healing (`pushRejectionHandler`) receives a field name it recognizes. Implements FR1, FR2, FR3 of fix-checklist-fk-self-heal. + +#### Scenario: Checklist item FK violation yields task_id +- **WHEN** a `checklist_items` insert violates `checklist_items_task_id_fkey` (SQLSTATE 23503) +- **THEN** `push_records` returns `status: "rejected"` with `reason: "fk_violation:task_id"` +- **AND** NOT `reason: "fk_violation:items_task_id"` + +#### Scenario: Task FK violations still yield their reference field +- **WHEN** a `tasks` insert violates `tasks_goal_id_fkey`, `tasks_context_id_fkey`, or `tasks_category_id_fkey` +- **THEN** `push_records` returns `reason: "fk_violation:goal_id"`, `"fk_violation:context_id"`, or `"fk_violation:category_id"` respectively + +#### Scenario: Orphaned checklist item self-heals end-to-end +- **WHEN** the client pushes a checklist item whose `task_id` references a task that does not exist on the server (purged or foreign) +- **THEN** the server rejects it with `reason: "fk_violation:task_id"` +- **AND** the client sets `is_deleted = true`, `syncStatus: "pending"`, and retries +- **AND** the retried push settles (the orphaned checklist item is soft-deleted, not left permanently rejected) diff --git a/openspec/changes/fix-checklist-fk-self-heal/tasks.md b/openspec/changes/fix-checklist-fk-self-heal/tasks.md new file mode 100644 index 00000000..82822ae5 --- /dev/null +++ b/openspec/changes/fix-checklist-fk-self-heal/tasks.md @@ -0,0 +1,21 @@ +# Tasks: fix-checklist-fk-self-heal + +## 1. RPC fix (in-place edit) + +- [ ] 1.1 `003_create_push_rpc.sql`: in all 8 `WHEN '23503'` branches, replace the regexp `^.*?_(.+?)_fkey$` with the underscore-safe `^.*_([a-z]+_id)_fkey$` so `checklist_items_task_id_fkey` yields `task_id` (FR1, FR3, FR4) +- [ ] 1.2 Confirm no other SQL logic changes in `003`; FK constraint names in `001_create_tables.sql` are unchanged (NG1, FR4) + +## 2. Test updates + +- [ ] 2.1 `packages/integration/src/tests/push-poison-pill-cross-tenant.spec.ts`: change the checklist FK case expectation from `fk_violation:items_task_id` to `fk_violation:task_id` and remove the now-obsolete "table name underscore" explanatory note (FR1, M1) +- [ ] 2.2 Add an end-to-end integration assertion: a checklist item referencing a non-existent/foreign task is rejected `fk_violation:task_id`, then a self-heal push with `is_deleted: true` succeeds (U1, U2, FR5) + +## 3. Verification + +- [ ] 3.1 Run only the cross-tenant spec to confirm the four `fk_violation:` reasons, including `task_id` for checklist (M1, M2) +- [ ] 3.2 Run the entire existing integration suite once, at the end — no regressions (M3, NFR-P1) +- [ ] 3.3 Confirm migration file count is still 4 and no client code changed (M4, FR4, FR5) + +## 4. Manual follow-up (user) + +- [ ] 4.1 Recreate environments manually: `bash scripts/reset.sh dev|qa` (+ prod if needed) — outside the agent's scope From 92961d7ccf3d347936d33133dd7b14f843785da3 Mon Sep 17 00:00:00 2001 From: oinsio <8233537+oinsio@users.noreply.github.com> Date: Wed, 15 Jul 2026 13:25:54 +0500 Subject: [PATCH 5/9] implement fix-checklist-fk-self-heal OpenSpec --- .../specs/push-poison-pill-protection/spec.md | 10 +- .../fix-checklist-fk-self-heal/tasks.md | 16 ++-- .../migrations/003_create_push_rpc.sql | 16 ++-- .../push-poison-pill-cross-tenant.spec.ts | 93 +++++++++++++++++-- 4 files changed, 106 insertions(+), 29 deletions(-) diff --git a/openspec/changes/fix-checklist-fk-self-heal/specs/push-poison-pill-protection/spec.md b/openspec/changes/fix-checklist-fk-self-heal/specs/push-poison-pill-protection/spec.md index 633c6c28..8bb2ee9b 100644 --- a/openspec/changes/fix-checklist-fk-self-heal/specs/push-poison-pill-protection/spec.md +++ b/openspec/changes/fix-checklist-fk-self-heal/specs/push-poison-pill-protection/spec.md @@ -14,8 +14,10 @@ The `push_records` RPC SHALL derive the structured rejection reason `fk_violatio - **WHEN** a `tasks` insert violates `tasks_goal_id_fkey`, `tasks_context_id_fkey`, or `tasks_category_id_fkey` - **THEN** `push_records` returns `reason: "fk_violation:goal_id"`, `"fk_violation:context_id"`, or `"fk_violation:category_id"` respectively -#### Scenario: Orphaned checklist item self-heals end-to-end +#### Scenario: Orphaned checklist item is healable (server backstop) and self-heals client-side - **WHEN** the client pushes a checklist item whose `task_id` references a task that does not exist on the server (purged or foreign) -- **THEN** the server rejects it with `reason: "fk_violation:task_id"` -- **AND** the client sets `is_deleted = true`, `syncStatus: "pending"`, and retries -- **AND** the retried push settles (the orphaned checklist item is soft-deleted, not left permanently rejected) +- **THEN** the server backstop rejects it with the healable `reason: "fk_violation:task_id"` (not the unhealable `"fk_violation:items_task_id"`) +- **AND** the client recognizes `task_id` as a delete-FK field, sets `is_deleted = true`, `syncStatus: "pending"`, and retries +- **AND** because the composite NOT NULL FK is enforced on INSERT regardless of `is_deleted`, the server never persists the dangling orphan — the item is instead removed client-side (SyncService orphan removal hard-deletes it before the next push, per cascade-checklist-delete FR3), so it is not left permanently `rejected` + +Note: the end-to-end client-side heal is covered by `cascade_checklist_self_healing.feature`, `pushRejectionHandler.test.ts`, and `SyncService.self-healing.test.ts`. This change only corrects the server backstop's reason string, verified by the integration assertions above. diff --git a/openspec/changes/fix-checklist-fk-self-heal/tasks.md b/openspec/changes/fix-checklist-fk-self-heal/tasks.md index 82822ae5..87df01f1 100644 --- a/openspec/changes/fix-checklist-fk-self-heal/tasks.md +++ b/openspec/changes/fix-checklist-fk-self-heal/tasks.md @@ -2,20 +2,20 @@ ## 1. RPC fix (in-place edit) -- [ ] 1.1 `003_create_push_rpc.sql`: in all 8 `WHEN '23503'` branches, replace the regexp `^.*?_(.+?)_fkey$` with the underscore-safe `^.*_([a-z]+_id)_fkey$` so `checklist_items_task_id_fkey` yields `task_id` (FR1, FR3, FR4) -- [ ] 1.2 Confirm no other SQL logic changes in `003`; FK constraint names in `001_create_tables.sql` are unchanged (NG1, FR4) +- [x] 1.1 `003_create_push_rpc.sql`: in all 8 `WHEN '23503'` branches, replace the regexp `^.*?_(.+?)_fkey$` with the underscore-safe `^.*_([a-z]+_id)_fkey$` so `checklist_items_task_id_fkey` yields `task_id` (FR1, FR3, FR4) +- [x] 1.2 Confirm no other SQL logic changes in `003`; FK constraint names in `001_create_tables.sql` are unchanged (NG1, FR4) ## 2. Test updates -- [ ] 2.1 `packages/integration/src/tests/push-poison-pill-cross-tenant.spec.ts`: change the checklist FK case expectation from `fk_violation:items_task_id` to `fk_violation:task_id` and remove the now-obsolete "table name underscore" explanatory note (FR1, M1) -- [ ] 2.2 Add an end-to-end integration assertion: a checklist item referencing a non-existent/foreign task is rejected `fk_violation:task_id`, then a self-heal push with `is_deleted: true` succeeds (U1, U2, FR5) +- [x] 2.1 `packages/integration/src/tests/push-poison-pill-cross-tenant.spec.ts`: change the checklist FK case expectation from `fk_violation:items_task_id` to `fk_violation:task_id` and remove the now-obsolete "table name underscore" explanatory note (FR1, M1) +- [x] 2.2 Add a server-backstop integration assertion: a checklist item referencing a non-existent/foreign task is rejected `fk_violation:task_id`; re-pushing it as a tombstone (`is_deleted: true`, same foreign `task_id`) is **still** rejected `fk_violation:task_id` (the composite NOT NULL FK is enforced on INSERT regardless of `is_deleted`, so the orphan is never persisted server-side); pull confirms absence. The true end-to-end client heal is already covered by `cascade_checklist_self_healing.feature` / `pushRejectionHandler.test.ts` / `SyncService.self-healing.test.ts` (U1, U2, FR1, FR5) ## 3. Verification -- [ ] 3.1 Run only the cross-tenant spec to confirm the four `fk_violation:` reasons, including `task_id` for checklist (M1, M2) -- [ ] 3.2 Run the entire existing integration suite once, at the end — no regressions (M3, NFR-P1) -- [ ] 3.3 Confirm migration file count is still 4 and no client code changed (M4, FR4, FR5) +- [x] 3.1 Run only the cross-tenant spec to confirm the four `fk_violation:` reasons, including `task_id` for checklist (M1, M2) — 5/5 passed (34.9s) +- [x] 3.2 Run the entire existing integration suite once, at the end — no regressions (M3, NFR-P1) — 82/82 passed (3.9m) +- [x] 3.3 Confirm migration file count is still 4 and no client code changed (M4, FR4, FR5) — 4 migrations; only `003` + integration spec changed, no client diff ## 4. Manual follow-up (user) -- [ ] 4.1 Recreate environments manually: `bash scripts/reset.sh dev|qa` (+ prod if needed) — outside the agent's scope +- [x] 4.1 Recreate environments manually: `bash scripts/reset.sh dev|qa` (+ prod if needed) — outside the agent's scope diff --git a/packages/adapter-supabase/supabase/migrations/003_create_push_rpc.sql b/packages/adapter-supabase/supabase/migrations/003_create_push_rpc.sql index 5f963304..6e8835ce 100644 --- a/packages/adapter-supabase/supabase/migrations/003_create_push_rpc.sql +++ b/packages/adapter-supabase/supabase/migrations/003_create_push_rpc.sql @@ -102,7 +102,7 @@ BEGIN v_constraint_name = CONSTRAINT_NAME, v_message = MESSAGE_TEXT; v_rejection_reason := CASE v_sqlstate - WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*?_(.+?)_fkey$', '\1') + WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*_([a-z]+_id)_fkey$', '\1') WHEN '23514' THEN 'check_violation:' || v_constraint_name WHEN '23505' THEN 'unique_violation' ELSE v_message @@ -151,7 +151,7 @@ BEGIN v_constraint_name = CONSTRAINT_NAME, v_message = MESSAGE_TEXT; v_rejection_reason := CASE v_sqlstate - WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*?_(.+?)_fkey$', '\1') + WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*_([a-z]+_id)_fkey$', '\1') WHEN '23514' THEN 'check_violation:' || v_constraint_name WHEN '23505' THEN 'unique_violation' ELSE v_message @@ -204,7 +204,7 @@ BEGIN v_constraint_name = CONSTRAINT_NAME, v_message = MESSAGE_TEXT; v_rejection_reason := CASE v_sqlstate - WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*?_(.+?)_fkey$', '\1') + WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*_([a-z]+_id)_fkey$', '\1') WHEN '23514' THEN 'check_violation:' || v_constraint_name WHEN '23505' THEN 'unique_violation' ELSE v_message @@ -253,7 +253,7 @@ BEGIN v_constraint_name = CONSTRAINT_NAME, v_message = MESSAGE_TEXT; v_rejection_reason := CASE v_sqlstate - WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*?_(.+?)_fkey$', '\1') + WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*_([a-z]+_id)_fkey$', '\1') WHEN '23514' THEN 'check_violation:' || v_constraint_name WHEN '23505' THEN 'unique_violation' ELSE v_message @@ -324,7 +324,7 @@ BEGIN v_constraint_name = CONSTRAINT_NAME, v_message = MESSAGE_TEXT; v_rejection_reason := CASE v_sqlstate - WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*?_(.+?)_fkey$', '\1') + WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*_([a-z]+_id)_fkey$', '\1') WHEN '23514' THEN 'check_violation:' || v_constraint_name WHEN '23505' THEN 'unique_violation' ELSE v_message @@ -376,7 +376,7 @@ BEGIN v_constraint_name = CONSTRAINT_NAME, v_message = MESSAGE_TEXT; v_rejection_reason := CASE v_sqlstate - WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*?_(.+?)_fkey$', '\1') + WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*_([a-z]+_id)_fkey$', '\1') WHEN '23514' THEN 'check_violation:' || v_constraint_name WHEN '23505' THEN 'unique_violation' ELSE v_message @@ -432,7 +432,7 @@ BEGIN v_constraint_name = CONSTRAINT_NAME, v_message = MESSAGE_TEXT; v_rejection_reason := CASE v_sqlstate - WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*?_(.+?)_fkey$', '\1') + WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*_([a-z]+_id)_fkey$', '\1') WHEN '23514' THEN 'check_violation:' || v_constraint_name WHEN '23505' THEN 'unique_violation' ELSE v_message @@ -472,7 +472,7 @@ BEGIN v_constraint_name = CONSTRAINT_NAME, v_message = MESSAGE_TEXT; v_rejection_reason := CASE v_sqlstate - WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*?_(.+?)_fkey$', '\1') + WHEN '23503' THEN 'fk_violation:' || regexp_replace(v_constraint_name, '^.*_([a-z]+_id)_fkey$', '\1') WHEN '23514' THEN 'check_violation:' || v_constraint_name WHEN '23505' THEN 'unique_violation' ELSE v_message diff --git a/packages/integration/src/tests/push-poison-pill-cross-tenant.spec.ts b/packages/integration/src/tests/push-poison-pill-cross-tenant.spec.ts index 436de489..0e406a5f 100644 --- a/packages/integration/src/tests/push-poison-pill-cross-tenant.spec.ts +++ b/packages/integration/src/tests/push-poison-pill-cross-tenant.spec.ts @@ -40,6 +40,11 @@ interface CrossTenantPullResponse { ok: boolean; contexts: Array<{ id: string; name: string; is_deleted: boolean }>; tasks: Array<{ id: string; goal_id: string; is_deleted: boolean }>; + checklist_items: Array<{ + id: string; + task_id: string; + is_deleted: boolean; + }>; } test.describe.configure({ mode: "serial" }); @@ -129,6 +134,82 @@ test("user B task referencing user A's goal → fk_violation:goal_id → self-he expect(healedTask?.is_deleted).toBe(false); }); +// implements U1, U2, FR1, FR5 of fix-checklist-fk-self-heal +// +// This test verifies the SERVER BACKSTOP contract only. The genuine, end-to-end +// self-heal of an orphaned checklist item is entirely CLIENT-SIDE and is already +// covered elsewhere: +// - SyncService removes orphans before push (cascade-checklist-delete FR3) — +// `cascade_checklist_self_healing.feature` +// - on a server rejection, the client soft-deletes + retries (fix-push-poison-pill +// FR5) — `pushRejectionHandler.test.ts`, `SyncService.self-healing.test.ts` +// +// The server never stores a checklist item whose task_id dangles: the composite +// NOT NULL FK `checklist_items_task_id_fkey` is enforced on INSERT regardless of +// `is_deleted`. So a tombstone re-push that still carries the foreign task_id is +// rejected again — it does NOT settle server-side. This change's contribution is +// only that the backstop reason is now the healable `task_id` (was the unhealable +// `items_task_id`), so the client CAN self-heal when the backstop is hit. +test("user B checklist item referencing user A's task → backstop rejects fk_violation:task_id, even soft-deleted", async () => { + // 1. User A creates a task (the owner the checklist item will point to). + const userATaskId = randomUUID(); + const taskPush = (await pushToServer( + userA, + buildTaskPayload({ id: userATaskId, name: `Task A ${Date.now()}` }), + )) as unknown as CrossTenantPushResponse; + expect(taskPush.results.tasks?.[0]?.status).toBe("created"); + + // 2. User B pushes a checklist item referencing user A's task (cross-tenant + // ref — the task does not exist for user B → FK violation). The backstop + // now returns the HEALABLE reason `task_id` (this change), not the old + // unhealable `items_task_id`. + const checklistItemId = randomUUID(); + const rejectedPush = (await pushToServer( + userB, + buildChecklistPayload(checklistItemId, userATaskId, "Item B"), + )) as unknown as CrossTenantPushResponse; + const rejectedResult = rejectedPush.results.checklist_items?.find( + (result) => result.id === checklistItemId, + ); + expect(rejectedResult?.status).toBe("rejected"); + expect(rejectedResult?.reason).toBe("fk_violation:task_id"); + + // 3. The client heals `task_id` rejections by setting is_deleted: true + // (DELETE_FK_FIELDS in pushRejectionHandler). Re-push the SAME item as a + // tombstone, still carrying the foreign task_id. The FK is enforced on the + // INSERT regardless of is_deleted, so the server rejects it AGAIN — the + // dangling orphan is never persisted. (In the real client the item is then + // hard-deleted before the next push by SyncService's orphan removal.) + const tombstonePush = buildChecklistPayload( + checklistItemId, + userATaskId, + "Item B", + ); + const [tombstoneItem] = tombstonePush.checklist_items as Record< + string, + unknown + >[]; + if (tombstoneItem) { + tombstoneItem.is_deleted = true; + } + const tombstoneResult = ( + (await pushToServer( + userB, + tombstonePush, + )) as unknown as CrossTenantPushResponse + ).results.checklist_items?.find((result) => result.id === checklistItemId); + expect(tombstoneResult?.status).toBe("rejected"); + expect(tombstoneResult?.reason).toBe("fk_violation:task_id"); + + // 4. Pull user B's data — the orphaned checklist item was never stored + // server-side (neither the original nor the tombstone push settled). + const pullB = await pullFromServer(userB); + const persistedItem = pullB.checklist_items.find( + (item) => item.id === checklistItemId, + ); + expect(persistedItem).toBeUndefined(); +}); + test("cross-tenant references are rejected with the correct fk_violation field for every FK", async () => { // User A creates a goal, a context, a category, and a task. const userAGoalId2 = randomUUID(); @@ -163,14 +244,8 @@ test("cross-tenant references are rejected with the correct fk_violation field f // encodes the FK field, which proves the corresponding constraint NAME // (tasks_context_id_fkey, tasks_category_id_fkey, tasks_goal_id_fkey, // checklist_items_task_id_fkey). Verifies FR4 / task 2.2 behaviorally. - // - // NOTE on the checklist case: the push RPC derives the field via the regexp - // `^.*?_(.+?)_fkey$`. The table name `checklist_items` itself contains an - // underscore, so that regexp yields `items_task_id` (not `task_id`) from - // `checklist_items_task_id_fkey`. This is pre-existing behavior — the old - // inline FK auto-generated the identical constraint name — and is therefore - // unchanged by this change (FR6). The `items_task_id` reason still uniquely - // confirms the constraint name is `checklist_items_task_id_fkey`. + // The checklist case yields `fk_violation:task_id`, which uniquely confirms + // the constraint name is `checklist_items_task_id_fkey`. const [ctxId, catId, goalId, itemId] = [ randomUUID(), randomUUID(), @@ -216,7 +291,7 @@ test("cross-tenant references are rejected with the correct fk_violation field f { entity: "checklist_items", id: itemId, - reason: "fk_violation:items_task_id", + reason: "fk_violation:task_id", payload: buildChecklistPayload(itemId, userATaskId, "Item"), }, ]; From b39fa054acdbdad2cc294bfc2bea2aadcdd7e355 Mon Sep 17 00:00:00 2001 From: oinsio <8233537+oinsio@users.noreply.github.com> Date: Wed, 15 Jul 2026 13:28:12 +0500 Subject: [PATCH 6/9] archive fix-checklist-fk-self-heal OpenSpec --- .../.openspec.yaml | 0 .../design.md | 0 .../proposal.md | 0 .../specs/push-poison-pill-protection/spec.md | 0 .../tasks.md | 0 .../specs/push-poison-pill-protection/spec.md | 20 +++++++++++++++++++ 6 files changed, 20 insertions(+) rename openspec/changes/{fix-checklist-fk-self-heal => archive/2026/07/2026-07-15-fix-checklist-fk-self-heal}/.openspec.yaml (100%) rename openspec/changes/{fix-checklist-fk-self-heal => archive/2026/07/2026-07-15-fix-checklist-fk-self-heal}/design.md (100%) rename openspec/changes/{fix-checklist-fk-self-heal => archive/2026/07/2026-07-15-fix-checklist-fk-self-heal}/proposal.md (100%) rename openspec/changes/{fix-checklist-fk-self-heal => archive/2026/07/2026-07-15-fix-checklist-fk-self-heal}/specs/push-poison-pill-protection/spec.md (100%) rename openspec/changes/{fix-checklist-fk-self-heal => archive/2026/07/2026-07-15-fix-checklist-fk-self-heal}/tasks.md (100%) diff --git a/openspec/changes/fix-checklist-fk-self-heal/.openspec.yaml b/openspec/changes/archive/2026/07/2026-07-15-fix-checklist-fk-self-heal/.openspec.yaml similarity index 100% rename from openspec/changes/fix-checklist-fk-self-heal/.openspec.yaml rename to openspec/changes/archive/2026/07/2026-07-15-fix-checklist-fk-self-heal/.openspec.yaml diff --git a/openspec/changes/fix-checklist-fk-self-heal/design.md b/openspec/changes/archive/2026/07/2026-07-15-fix-checklist-fk-self-heal/design.md similarity index 100% rename from openspec/changes/fix-checklist-fk-self-heal/design.md rename to openspec/changes/archive/2026/07/2026-07-15-fix-checklist-fk-self-heal/design.md diff --git a/openspec/changes/fix-checklist-fk-self-heal/proposal.md b/openspec/changes/archive/2026/07/2026-07-15-fix-checklist-fk-self-heal/proposal.md similarity index 100% rename from openspec/changes/fix-checklist-fk-self-heal/proposal.md rename to openspec/changes/archive/2026/07/2026-07-15-fix-checklist-fk-self-heal/proposal.md diff --git a/openspec/changes/fix-checklist-fk-self-heal/specs/push-poison-pill-protection/spec.md b/openspec/changes/archive/2026/07/2026-07-15-fix-checklist-fk-self-heal/specs/push-poison-pill-protection/spec.md similarity index 100% rename from openspec/changes/fix-checklist-fk-self-heal/specs/push-poison-pill-protection/spec.md rename to openspec/changes/archive/2026/07/2026-07-15-fix-checklist-fk-self-heal/specs/push-poison-pill-protection/spec.md diff --git a/openspec/changes/fix-checklist-fk-self-heal/tasks.md b/openspec/changes/archive/2026/07/2026-07-15-fix-checklist-fk-self-heal/tasks.md similarity index 100% rename from openspec/changes/fix-checklist-fk-self-heal/tasks.md rename to openspec/changes/archive/2026/07/2026-07-15-fix-checklist-fk-self-heal/tasks.md diff --git a/openspec/specs/push-poison-pill-protection/spec.md b/openspec/specs/push-poison-pill-protection/spec.md index 60296453..0fa80578 100644 --- a/openspec/specs/push-poison-pill-protection/spec.md +++ b/openspec/specs/push-poison-pill-protection/spec.md @@ -79,3 +79,23 @@ The push Edge Function SHALL log rejected records via `console.warn` with user I #### Scenario: RPC-rejected records are logged - **WHEN** 1 task is rejected by RPC with FK violation - **THEN** server logs `[push] User : 1 record rejected by RPC` with reason + +### Requirement: Server extracts the fk_violation field name correctly for all tables +The `push_records` RPC SHALL derive the structured rejection reason `fk_violation:` from the violated FK constraint name in a way that is correct even when the **table name contains underscores**. The extracted `` SHALL be the FK reference column (the trailing `_id` segment of the `
__fkey` constraint name), so that client self-healing (`pushRejectionHandler`) receives a field name it recognizes. Implements FR1, FR2, FR3 of fix-checklist-fk-self-heal. + +#### Scenario: Checklist item FK violation yields task_id +- **WHEN** a `checklist_items` insert violates `checklist_items_task_id_fkey` (SQLSTATE 23503) +- **THEN** `push_records` returns `status: "rejected"` with `reason: "fk_violation:task_id"` +- **AND** NOT `reason: "fk_violation:items_task_id"` + +#### Scenario: Task FK violations still yield their reference field +- **WHEN** a `tasks` insert violates `tasks_goal_id_fkey`, `tasks_context_id_fkey`, or `tasks_category_id_fkey` +- **THEN** `push_records` returns `reason: "fk_violation:goal_id"`, `"fk_violation:context_id"`, or `"fk_violation:category_id"` respectively + +#### Scenario: Orphaned checklist item is healable (server backstop) and self-heals client-side +- **WHEN** the client pushes a checklist item whose `task_id` references a task that does not exist on the server (purged or foreign) +- **THEN** the server backstop rejects it with the healable `reason: "fk_violation:task_id"` (not the unhealable `"fk_violation:items_task_id"`) +- **AND** the client recognizes `task_id` as a delete-FK field, sets `is_deleted = true`, `syncStatus: "pending"`, and retries +- **AND** because the composite NOT NULL FK is enforced on INSERT regardless of `is_deleted`, the server never persists the dangling orphan — the item is instead removed client-side (SyncService orphan removal hard-deletes it before the next push, per cascade-checklist-delete FR3), so it is not left permanently `rejected` + +Note: the end-to-end client-side heal is covered by `cascade_checklist_self_healing.feature`, `pushRejectionHandler.test.ts`, and `SyncService.self-healing.test.ts`. This change only corrects the server backstop's reason string, verified by the integration assertions above. From c42d3ce7ff0213b94486242dba60f5cace22fb51 Mon Sep 17 00:00:00 2001 From: oinsio <8233537+oinsio@users.noreply.github.com> Date: Wed, 15 Jul 2026 16:50:00 +0500 Subject: [PATCH 7/9] add collapse-db-versions OpenSpec --- .../collapse-db-versions/.openspec.yaml | 2 + .../changes/collapse-db-versions/design.md | 53 +++++++++++++++++++ .../changes/collapse-db-versions/proposal.md | 28 ++++++++++ .../specs/local-db-schema/spec.md | 37 +++++++++++++ .../changes/collapse-db-versions/tasks.md | 25 +++++++++ 5 files changed, 145 insertions(+) create mode 100644 openspec/changes/collapse-db-versions/.openspec.yaml create mode 100644 openspec/changes/collapse-db-versions/design.md create mode 100644 openspec/changes/collapse-db-versions/proposal.md create mode 100644 openspec/changes/collapse-db-versions/specs/local-db-schema/spec.md create mode 100644 openspec/changes/collapse-db-versions/tasks.md diff --git a/openspec/changes/collapse-db-versions/.openspec.yaml b/openspec/changes/collapse-db-versions/.openspec.yaml new file mode 100644 index 00000000..4f63482c --- /dev/null +++ b/openspec/changes/collapse-db-versions/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-15 diff --git a/openspec/changes/collapse-db-versions/design.md b/openspec/changes/collapse-db-versions/design.md new file mode 100644 index 00000000..50bfbadb --- /dev/null +++ b/openspec/changes/collapse-db-versions/design.md @@ -0,0 +1,53 @@ +## Context + +The Dexie database (`packages/client/src/db/database.ts`) currently declares two schema versions: + +- `version(1).stores(DB_SCHEMA_V1)` — the original schema, indexing a `needsSync` boolean. +- `version(2).stores(DB_SCHEMA_V2).upgrade(...)` — the current schema, indexing `syncStatus`, plus an `.upgrade()` callback (added by `fix-push-poison-pill`, FR6) that walks every entity table and rewrites `record.needsSync → record.syncStatus`. + +`schema.ts` exports `DB_SCHEMA_V1`, `DB_SCHEMA_V2`, and a deprecated `DB_SCHEMA = DB_SCHEMA_V1` alias. The `needsSync` field has no remaining readers in `src/` — it survives only inside the schema/migration code. The app is pre-production: no real user has a persisted database, so the only databases at `version(2)` belong to developers. + +This is driven by the proposal's goal of removing dead migration code before launch, while it is cheap to do so. + +## Goals / Non-Goals + +**Goals:** +- Reduce the database definition to a single `version(1)` baseline using the `syncStatus` schema (proposal FR1, FR4). +- Delete the `version(2)` upgrade migration (FR2). +- Collapse schema exports to a single `DB_SCHEMA` and drop the `needsSync` index (FR3). +- Keep all existing repository/sync behavior and tests green — zero behavioral change. + +**Non-Goals:** +- No data migration for existing local databases — developers wipe and recreate (pre-production trade-off). +- No changes to entity fields, `RECORD_SYNC_STATUS` values, sync push/pull, ports, adapters, or UI. +- No renaming of stores or indexes beyond removing `needsSync`. + +## Decisions + +**D1 — Reuse the V2 schema shape as the single baseline, renamed to `DB_SCHEMA`.** +The current on-disk-correct schema is `DB_SCHEMA_V2`. Rather than invent a new object, rename it to `DB_SCHEMA` (replacing the deprecated alias) and delete `DB_SCHEMA_V1`/`DB_SCHEMA_V2`. `database.ts` then calls `this.version(1).stores(DB_SCHEMA)` with no `.upgrade()`. +- *Alternative considered*: keep `version(2)` as the only declaration (drop `version(1)`). Rejected — a fresh baseline should start at 1; a lone `version(2)` is confusing and still implies a missing predecessor. + +**D2 — Accept the Dexie `VersionError` on down-declaration; document the manual wipe.** +Dexie refuses to open a database whose on-disk version (2) exceeds the max declared version (1), throwing `VersionError`. We will not add code to detect-and-delete the old DB, because it only affects a handful of developer machines and adding self-deleting logic is exactly the kind of throwaway migration code this change removes. +- *Alternative considered*: a one-time `Dexie.delete(DB_NAME)` guard when a `VersionError` is caught. Rejected as unnecessary complexity for a pre-production, developer-only scenario; documented in tasks instead. + +**D3 — Verify via the existing test suite plus a focused schema test.** +Repository tests already run against `fake-indexeddb` with a fresh DB, so they implicitly assert the baseline schema works. Add/keep a small test asserting the DB opens at `verno === 1` and that a `syncStatus` query works, to lock the requirement. No new mutation surface is introduced beyond the schema constant. + +## Risks / Trade-offs + +- **Developer database at version 2 fails to open (`VersionError`)** → Documented one-time fix: delete the `clear-progress` IndexedDB (DevTools → Application → IndexedDB, or `indexedDB.deleteDatabase("clear-progress")`). Only affects local dev machines. +- **A future reader of git history loses the `needsSync → syncStatus` migration context** → The migration remains in the archived `fix-push-poison-pill` change and git history; only the live code path is removed. +- **Hidden reader of `needsSync` re-emerges** → Mitigated: grep confirms zero readers outside `schema.ts`/`database.ts`; the build (`tsc`) will fail if any typed reference remains. + +## Migration Plan + +1. Edit `schema.ts`: rename `DB_SCHEMA_V2 → DB_SCHEMA`, remove `DB_SCHEMA_V1` and the deprecated alias. +2. Edit `database.ts`: single `this.version(1).stores(DB_SCHEMA)`, delete the `version(2)` block and `.upgrade()` callback, update the import. +3. Run typecheck + affected repository tests + the schema test. +4. Rollback: `git revert` — no persisted-data implications since pre-production. + +## Open Questions + + diff --git a/openspec/changes/collapse-db-versions/proposal.md b/openspec/changes/collapse-db-versions/proposal.md new file mode 100644 index 00000000..55ce4364 --- /dev/null +++ b/openspec/changes/collapse-db-versions/proposal.md @@ -0,0 +1,28 @@ +# Collapse IndexedDB Versions + +## Why + +The app is not in production, so there is no real user data to preserve on disk. Yet the Dexie schema still carries the full incremental history: a legacy `version(1)` with the old `needsSync` index, plus a `version(2)` and a runtime `upgrade()` migration that rewrites `needsSync → syncStatus`. This migration only exists to protect data that does not exist, and the dual schema (`DB_SCHEMA_V1` + `DB_SCHEMA_V2` + a deprecated `DB_SCHEMA` alias) adds dead code that degrades clarity and AI context quality. Collapsing to a single clean baseline now — before we ship — is free; doing it after launch would require a real migration. + +## What Changes + +- **REMOVED** the `version(2)` declaration and its `.upgrade()` migration callback that rewrote `needsSync → syncStatus`. +- **REMOVED** `DB_SCHEMA_V1` and the deprecated `DB_SCHEMA` alias, plus the obsolete `needsSync` index that only V1 used. +- **MODIFIED** the database definition to a single `version(1).stores(DB_SCHEMA)` using the current (`syncStatus`-based) schema as the sole baseline. +- **BREAKING** (developers only): a local IndexedDB already at version 2 cannot be down-declared to version 1 — Dexie throws `VersionError`. Existing local databases must be deleted. Acceptable because the app is pre-production and only developer/local databases exist. + +## Capabilities + +### New Capabilities +- `local-db-schema`: Defines how the IndexedDB (Dexie) database schema is declared — a single baseline version with no incremental migration history while the app is pre-production. + +### Modified Capabilities + + +## Impact + +- **Code**: `packages/client/src/db/database.ts` (version declarations + upgrade callback), `packages/client/src/db/schema.ts` (schema exports). +- **Tests**: existing repository tests use `fake-indexeddb` with a fresh DB per run, so they exercise the baseline schema directly and must stay green. No test references `version(2)`, the migration, or `needsSync`. +- **Runtime**: `needsSync` has no remaining readers in `src/` — it lives only in `schema.ts` and `database.ts`. `syncStatus` (`RECORD_SYNC_STATUS`) remains the sole sync-state field. +- **Developers**: must clear their local IndexedDB (`clear-progress`) once, or the app will fail to open the DB after this change. No production users are affected. +- **No changes** to entity fields, sync push/pull semantics, ports/adapters, or UI. diff --git a/openspec/changes/collapse-db-versions/specs/local-db-schema/spec.md b/openspec/changes/collapse-db-versions/specs/local-db-schema/spec.md new file mode 100644 index 00000000..3d866710 --- /dev/null +++ b/openspec/changes/collapse-db-versions/specs/local-db-schema/spec.md @@ -0,0 +1,37 @@ +## ADDED Requirements + +### Requirement: Single baseline schema version +The IndexedDB (Dexie) database SHALL declare its schema through exactly one `version(1).stores(...)` call using the current schema (the `syncStatus`-based schema). No higher version numbers and no incremental version history SHALL exist while the app is pre-production. + + + +#### Scenario: Database opens with version 1 +- **WHEN** the application instantiates the database on a device with no existing IndexedDB +- **THEN** the database is created at schema version 1 +- **AND** all stores (`tasks`, `goals`, `contexts`, `categories`, `checklist_items`, `ideas`, `settings`, `files`, `pending_files`, `attachments`, `sync_meta`) exist with their indexes + +#### Scenario: Sync-state records are queryable by syncStatus +- **WHEN** a record with a `syncStatus` value is stored in any entity table +- **THEN** it can be queried through the `syncStatus` index +- **AND** no `needsSync` index exists on any table + +### Requirement: No migration callbacks +The database definition SHALL NOT contain any `.upgrade()` migration callback. Because the app is pre-production, the schema is treated as a fresh baseline with no data to migrate. + + + +#### Scenario: Opening a fresh database runs no migration +- **WHEN** the database is opened for the first time +- **THEN** no upgrade/migration logic executes +- **AND** records are written with `syncStatus` directly, never derived from a legacy `needsSync` flag + +### Requirement: Single schema export +The schema module SHALL export exactly one schema object (`DB_SCHEMA`) describing the store definitions. Legacy exports (`DB_SCHEMA_V1`, `DB_SCHEMA_V2`, and the deprecated `DB_SCHEMA` alias pointing at V1) SHALL be removed, along with the obsolete `needsSync` index. + + + +#### Scenario: Only the baseline schema is exported +- **WHEN** a module imports the database schema +- **THEN** it imports a single `DB_SCHEMA` object +- **AND** no versioned schema variants (`DB_SCHEMA_V1`, `DB_SCHEMA_V2`) are exported +- **AND** `DB_SCHEMA` contains the `syncStatus` index and does not contain a `needsSync` index diff --git a/openspec/changes/collapse-db-versions/tasks.md b/openspec/changes/collapse-db-versions/tasks.md new file mode 100644 index 00000000..4ef2abe5 --- /dev/null +++ b/openspec/changes/collapse-db-versions/tasks.md @@ -0,0 +1,25 @@ +## 1. Lock baseline behavior with a test + +- [ ] 1.1 Add `src/db/database.test.ts` (using `fake-indexeddb`) asserting: DB opens at `db.verno === 1`; a record written with `syncStatus` is retrievable via the `syncStatus` index; all 11 stores exist. Verifies FR1/FR4 of collapse-db-versions. Run `npx vitest run src/db/database.test.ts` — RED where it references the single `DB_SCHEMA` export not yet renamed (or GREEN on verno if written against current state; then confirm it stays GREEN after step 2/3). + +## 2. Collapse the schema module + +- [ ] 2.1 In `src/db/schema.ts`: rename `DB_SCHEMA_V2` → `DB_SCHEMA` (the sole export). Remove `DB_SCHEMA_V1`, the deprecated `DB_SCHEMA = DB_SCHEMA_V1` alias, and any `needsSync` index. Implements FR3. +- [ ] 2.2 Confirm no remaining references to `DB_SCHEMA_V1` / `DB_SCHEMA_V2` / `needsSync` in `src/` (`grep -rn "DB_SCHEMA_V1\|DB_SCHEMA_V2\|needsSync" src`). + +## 3. Collapse the database definition + +- [ ] 3.1 In `src/db/database.ts`: change the import to the single `DB_SCHEMA`. Replace the two version blocks with one `this.version(1).stores(DB_SCHEMA);`. Delete the `version(2)` declaration and its `.upgrade()` migration callback. Implements FR1, FR2, FR4. +- [ ] 3.2 Remove the now-unused `RECORD_SYNC_STATUS` import from `database.ts` if it was only used by the migration. + +## 4. Verify + +- [ ] 4.1 `get_file_problems` (JetBrains MCP) on `schema.ts`, `database.ts`, `database.test.ts` — fix any errors. +- [ ] 4.2 `pnpm run build` (typecheck must pass; catches any stray typed reader of `needsSync`/`DB_SCHEMA_V*`). +- [ ] 4.3 `npx vitest run src/db/database.test.ts` — GREEN. +- [ ] 4.4 Run the repository test suite touching the DB (`npx vitest run src/db/repositories`) — all GREEN, confirming zero behavioral change. Run STRICTLY one command at a time. +- [ ] 4.5 Mutation-test the changed files: `cd packages/client && npx stryker run --mutate 'src/db/schema.ts,src/db/database.ts'`; read `reports/mutation/mutation-report.json`; add tests to kill survivors (target ≥95%, min ≥90%). + +## 5. Document the developer wipe + +- [ ] 5.1 Note in the change (and PR description at apply time) that developers with an existing local `clear-progress` IndexedDB must run `indexedDB.deleteDatabase("clear-progress")` once, since Dexie throws `VersionError` when the on-disk version (2) exceeds the declared version (1). From 7043a94587548b0caffb14f9ae38ea1b77e5f30a Mon Sep 17 00:00:00 2001 From: oinsio <8233537+oinsio@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:13:30 +0500 Subject: [PATCH 8/9] implement collapse-db-versions OpenSpec --- .../changes/collapse-db-versions/tasks.md | 22 +++---- packages/client/src/db/database.test.ts | 57 +++++++++++++++++++ packages/client/src/db/database.ts | 34 +---------- packages/client/src/db/schema.ts | 25 +------- 4 files changed, 74 insertions(+), 64 deletions(-) create mode 100644 packages/client/src/db/database.test.ts diff --git a/openspec/changes/collapse-db-versions/tasks.md b/openspec/changes/collapse-db-versions/tasks.md index 4ef2abe5..1e718dfe 100644 --- a/openspec/changes/collapse-db-versions/tasks.md +++ b/openspec/changes/collapse-db-versions/tasks.md @@ -1,25 +1,25 @@ ## 1. Lock baseline behavior with a test -- [ ] 1.1 Add `src/db/database.test.ts` (using `fake-indexeddb`) asserting: DB opens at `db.verno === 1`; a record written with `syncStatus` is retrievable via the `syncStatus` index; all 11 stores exist. Verifies FR1/FR4 of collapse-db-versions. Run `npx vitest run src/db/database.test.ts` — RED where it references the single `DB_SCHEMA` export not yet renamed (or GREEN on verno if written against current state; then confirm it stays GREEN after step 2/3). +- [x] 1.1 Add `src/db/database.test.ts` (using `fake-indexeddb`) asserting: DB opens at `db.verno === 1`; a record written with `syncStatus` is retrievable via the `syncStatus` index; all 11 stores exist. Verifies FR1/FR4 of collapse-db-versions. Run `npx vitest run src/db/database.test.ts` — RED where it references the single `DB_SCHEMA` export not yet renamed (or GREEN on verno if written against current state; then confirm it stays GREEN after step 2/3). ## 2. Collapse the schema module -- [ ] 2.1 In `src/db/schema.ts`: rename `DB_SCHEMA_V2` → `DB_SCHEMA` (the sole export). Remove `DB_SCHEMA_V1`, the deprecated `DB_SCHEMA = DB_SCHEMA_V1` alias, and any `needsSync` index. Implements FR3. -- [ ] 2.2 Confirm no remaining references to `DB_SCHEMA_V1` / `DB_SCHEMA_V2` / `needsSync` in `src/` (`grep -rn "DB_SCHEMA_V1\|DB_SCHEMA_V2\|needsSync" src`). +- [x] 2.1 In `src/db/schema.ts`: rename `DB_SCHEMA_V2` → `DB_SCHEMA` (the sole export). Remove `DB_SCHEMA_V1`, the deprecated `DB_SCHEMA = DB_SCHEMA_V1` alias, and any `needsSync` index. Implements FR3. +- [x] 2.2 Confirm no remaining references to `DB_SCHEMA_V1` / `DB_SCHEMA_V2` / `needsSync` in `src/` (`grep -rn "DB_SCHEMA_V1\|DB_SCHEMA_V2\|needsSync" src`). ## 3. Collapse the database definition -- [ ] 3.1 In `src/db/database.ts`: change the import to the single `DB_SCHEMA`. Replace the two version blocks with one `this.version(1).stores(DB_SCHEMA);`. Delete the `version(2)` declaration and its `.upgrade()` migration callback. Implements FR1, FR2, FR4. -- [ ] 3.2 Remove the now-unused `RECORD_SYNC_STATUS` import from `database.ts` if it was only used by the migration. +- [x] 3.1 In `src/db/database.ts`: change the import to the single `DB_SCHEMA`. Replace the two version blocks with one `this.version(1).stores(DB_SCHEMA);`. Delete the `version(2)` declaration and its `.upgrade()` migration callback. Implements FR1, FR2, FR4. +- [x] 3.2 Remove the now-unused `RECORD_SYNC_STATUS` import from `database.ts` if it was only used by the migration. ## 4. Verify -- [ ] 4.1 `get_file_problems` (JetBrains MCP) on `schema.ts`, `database.ts`, `database.test.ts` — fix any errors. -- [ ] 4.2 `pnpm run build` (typecheck must pass; catches any stray typed reader of `needsSync`/`DB_SCHEMA_V*`). -- [ ] 4.3 `npx vitest run src/db/database.test.ts` — GREEN. -- [ ] 4.4 Run the repository test suite touching the DB (`npx vitest run src/db/repositories`) — all GREEN, confirming zero behavioral change. Run STRICTLY one command at a time. -- [ ] 4.5 Mutation-test the changed files: `cd packages/client && npx stryker run --mutate 'src/db/schema.ts,src/db/database.ts'`; read `reports/mutation/mutation-report.json`; add tests to kill survivors (target ≥95%, min ≥90%). +- [x] 4.1 `get_file_problems` (JetBrains MCP) on `schema.ts`, `database.ts`, `database.test.ts` — fix any errors. +- [x] 4.2 `pnpm run build` (typecheck must pass; catches any stray typed reader of `needsSync`/`DB_SCHEMA_V*`). +- [x] 4.3 `npx vitest run src/db/database.test.ts` — GREEN. +- [x] 4.4 Run the repository test suite touching the DB (`npx vitest run src/db/repositories`) — all GREEN, confirming zero behavioral change. Run STRICTLY one command at a time. +- [x] 4.5 Mutation-test the changed files: `cd packages/client && npx stryker run --mutate 'src/db/schema.ts,src/db/database.ts'`; read `reports/mutation/mutation-report.json`; add tests to kill survivors (target ≥95%, min ≥90%). ## 5. Document the developer wipe -- [ ] 5.1 Note in the change (and PR description at apply time) that developers with an existing local `clear-progress` IndexedDB must run `indexedDB.deleteDatabase("clear-progress")` once, since Dexie throws `VersionError` when the on-disk version (2) exceeds the declared version (1). +- [x] 5.1 Note in the change (and PR description at apply time) that developers with an existing local `clear-progress` IndexedDB must run `indexedDB.deleteDatabase("clear-progress")` once, since Dexie throws `VersionError` when the on-disk version (2) exceeds the declared version (1). diff --git a/packages/client/src/db/database.test.ts b/packages/client/src/db/database.test.ts new file mode 100644 index 00000000..577d2f34 --- /dev/null +++ b/packages/client/src/db/database.test.ts @@ -0,0 +1,57 @@ +// Verifies FR1/FR4 of collapse-db-versions +import { beforeEach, describe, expect, it } from "vitest"; +import { RECORD_SYNC_STATUS } from "@/constants"; +import { buildTask } from "@/test/factories/taskFactory"; +import { db } from "./database"; + +const EXPECTED_VERSION_NUMBER = 1; + +const EXPECTED_STORE_NAMES = [ + "tasks", + "goals", + "contexts", + "categories", + "checklist_items", + "ideas", + "settings", + "files", + "pending_files", + "attachments", + "sync_meta", +] as const; + +describe("ClearProgressDatabase", () => { + beforeEach(async () => { + if (!db.isOpen()) { + await db.open(); + } + await db.tasks.clear(); + }); + + it("should open at a single collapsed schema version", () => { + expect(db.verno).toBe(EXPECTED_VERSION_NUMBER); + }); + + it("should expose all eleven expected stores", () => { + const storeNames = db.tables.map((table) => table.name); + + for (const expectedStoreName of EXPECTED_STORE_NAMES) { + expect(storeNames).toContain(expectedStoreName); + } + expect(storeNames).toHaveLength(EXPECTED_STORE_NAMES.length); + }); + + it("should index records by syncStatus", async () => { + const pendingTask = buildTask({ syncStatus: RECORD_SYNC_STATUS.PENDING }); + const syncedTask = buildTask({ syncStatus: RECORD_SYNC_STATUS.SYNCED }); + await db.tasks.bulkAdd([pendingTask, syncedTask]); + + const pendingTasks = await db.tasks + .where("syncStatus") + .equals(RECORD_SYNC_STATUS.PENDING) + .toArray(); + + expect(pendingTasks).toHaveLength(1); + expect(pendingTasks[0].id).toBe(pendingTask.id); + }); +}); diff --git a/packages/client/src/db/database.ts b/packages/client/src/db/database.ts index b8afa345..5f23a36b 100644 --- a/packages/client/src/db/database.ts +++ b/packages/client/src/db/database.ts @@ -1,5 +1,5 @@ import Dexie, { type EntityTable } from "dexie"; -import { DB_NAME, RECORD_SYNC_STATUS } from "@/constants"; +import { DB_NAME } from "@/constants"; import type { Attachment, Category, @@ -13,7 +13,7 @@ import type { SyncMeta, Task, } from "@/types/entities"; -import { DB_SCHEMA_V1, DB_SCHEMA_V2 } from "./schema"; +import { DB_SCHEMA } from "./schema"; export class ClearProgressDatabase extends Dexie { tasks!: EntityTable; @@ -30,35 +30,7 @@ export class ClearProgressDatabase extends Dexie { constructor() { super(DB_NAME); - this.version(1).stores(DB_SCHEMA_V1); - - // implements FR6 of fix-push-poison-pill - this.version(2) - .stores(DB_SCHEMA_V2) - .upgrade(async (transaction) => { - const entityTables = [ - "tasks", - "goals", - "contexts", - "categories", - "checklist_items", - "ideas", - "attachments", - "settings", - ] as const; - - for (const tableName of entityTables) { - await transaction - .table(tableName) - .toCollection() - .modify((record: Record) => { - record.syncStatus = record.needsSync - ? RECORD_SYNC_STATUS.PENDING - : RECORD_SYNC_STATUS.SYNCED; - delete record.needsSync; - }); - } - }); + this.version(1).stores(DB_SCHEMA); } } diff --git a/packages/client/src/db/schema.ts b/packages/client/src/db/schema.ts index de4d6585..1fc8c7dc 100644 --- a/packages/client/src/db/schema.ts +++ b/packages/client/src/db/schema.ts @@ -1,22 +1,6 @@ -export const DB_SCHEMA_V1 = { - tasks: - "id, box, goal_id, context_id, category_id, is_completed, is_hidden, is_deleted, sort_order, revision, needsSync, appear_date, original_task_id, updated_at", - goals: "id, status, is_deleted, sort_order, revision, needsSync, updated_at", - contexts: "id, is_deleted, sort_order, revision, needsSync, updated_at", - categories: "id, is_deleted, sort_order, revision, needsSync, updated_at", - checklist_items: - "id, task_id, is_deleted, sort_order, revision, needsSync, updated_at", - ideas: "id, is_deleted, sort_order, revision, needsSync, updated_at", - settings: "key, needsSync, updated_at", - files: "data_hash", - pending_files: "data_hash", - attachments: - "id, [entity_type+entity_id], data_hash, is_deleted, sort_order, revision, needsSync, updated_at", - sync_meta: "key", -} as const; - -// implements FR6 of fix-push-poison-pill -export const DB_SCHEMA_V2 = { +// implements FR3 of collapse-db-versions +// syncStatus indexes originate from FR6 of fix-push-poison-pill +export const DB_SCHEMA = { tasks: "id, box, goal_id, context_id, category_id, is_completed, is_hidden, is_deleted, sort_order, revision, syncStatus, appear_date, original_task_id, updated_at", goals: "id, status, is_deleted, sort_order, revision, syncStatus, updated_at", @@ -32,6 +16,3 @@ export const DB_SCHEMA_V2 = { "id, [entity_type+entity_id], data_hash, is_deleted, sort_order, revision, syncStatus, updated_at", sync_meta: "key", } as const; - -/** @deprecated Use DB_SCHEMA_V2 instead */ -export const DB_SCHEMA = DB_SCHEMA_V1; From 82e2b5f23ba644024d8003813ba7a7b4d227f9cd Mon Sep 17 00:00:00 2001 From: oinsio <8233537+oinsio@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:44:29 +0500 Subject: [PATCH 9/9] archive collapse-db-versions OpenSpec --- .../.openspec.yaml | 0 .../design.md | 0 .../proposal.md | 0 .../specs/local-db-schema/spec.md | 0 .../2026-07-15-collapse-db-versions}/tasks.md | 0 openspec/specs/local-db-schema/spec.md | 37 +++++++++++++++++++ 6 files changed, 37 insertions(+) rename openspec/changes/{collapse-db-versions => archive/2026/07/2026-07-15-collapse-db-versions}/.openspec.yaml (100%) rename openspec/changes/{collapse-db-versions => archive/2026/07/2026-07-15-collapse-db-versions}/design.md (100%) rename openspec/changes/{collapse-db-versions => archive/2026/07/2026-07-15-collapse-db-versions}/proposal.md (100%) rename openspec/changes/{collapse-db-versions => archive/2026/07/2026-07-15-collapse-db-versions}/specs/local-db-schema/spec.md (100%) rename openspec/changes/{collapse-db-versions => archive/2026/07/2026-07-15-collapse-db-versions}/tasks.md (100%) create mode 100644 openspec/specs/local-db-schema/spec.md diff --git a/openspec/changes/collapse-db-versions/.openspec.yaml b/openspec/changes/archive/2026/07/2026-07-15-collapse-db-versions/.openspec.yaml similarity index 100% rename from openspec/changes/collapse-db-versions/.openspec.yaml rename to openspec/changes/archive/2026/07/2026-07-15-collapse-db-versions/.openspec.yaml diff --git a/openspec/changes/collapse-db-versions/design.md b/openspec/changes/archive/2026/07/2026-07-15-collapse-db-versions/design.md similarity index 100% rename from openspec/changes/collapse-db-versions/design.md rename to openspec/changes/archive/2026/07/2026-07-15-collapse-db-versions/design.md diff --git a/openspec/changes/collapse-db-versions/proposal.md b/openspec/changes/archive/2026/07/2026-07-15-collapse-db-versions/proposal.md similarity index 100% rename from openspec/changes/collapse-db-versions/proposal.md rename to openspec/changes/archive/2026/07/2026-07-15-collapse-db-versions/proposal.md diff --git a/openspec/changes/collapse-db-versions/specs/local-db-schema/spec.md b/openspec/changes/archive/2026/07/2026-07-15-collapse-db-versions/specs/local-db-schema/spec.md similarity index 100% rename from openspec/changes/collapse-db-versions/specs/local-db-schema/spec.md rename to openspec/changes/archive/2026/07/2026-07-15-collapse-db-versions/specs/local-db-schema/spec.md diff --git a/openspec/changes/collapse-db-versions/tasks.md b/openspec/changes/archive/2026/07/2026-07-15-collapse-db-versions/tasks.md similarity index 100% rename from openspec/changes/collapse-db-versions/tasks.md rename to openspec/changes/archive/2026/07/2026-07-15-collapse-db-versions/tasks.md diff --git a/openspec/specs/local-db-schema/spec.md b/openspec/specs/local-db-schema/spec.md new file mode 100644 index 00000000..7c2bca5a --- /dev/null +++ b/openspec/specs/local-db-schema/spec.md @@ -0,0 +1,37 @@ +# local-db-schema spec + +## Purpose + +Defines how the client-side IndexedDB (Dexie) database declares its schema. The database uses a single baseline schema version with no migration history, reflecting the app's pre-production state where no persisted data needs to be migrated. + +## Requirements + +### Requirement: Single baseline schema version +The IndexedDB (Dexie) database SHALL declare its schema through exactly one `version(1).stores(...)` call using the current schema (the `syncStatus`-based schema). No higher version numbers and no incremental version history SHALL exist while the app is pre-production. + +#### Scenario: Database opens with version 1 +- **WHEN** the application instantiates the database on a device with no existing IndexedDB +- **THEN** the database is created at schema version 1 +- **AND** all stores (`tasks`, `goals`, `contexts`, `categories`, `checklist_items`, `ideas`, `settings`, `files`, `pending_files`, `attachments`, `sync_meta`) exist with their indexes + +#### Scenario: Sync-state records are queryable by syncStatus +- **WHEN** a record with a `syncStatus` value is stored in any entity table +- **THEN** it can be queried through the `syncStatus` index +- **AND** no `needsSync` index exists on any table + +### Requirement: No migration callbacks +The database definition SHALL NOT contain any `.upgrade()` migration callback. Because the app is pre-production, the schema is treated as a fresh baseline with no data to migrate. + +#### Scenario: Opening a fresh database runs no migration +- **WHEN** the database is opened for the first time +- **THEN** no upgrade/migration logic executes +- **AND** records are written with `syncStatus` directly, never derived from a legacy `needsSync` flag + +### Requirement: Single schema export +The schema module SHALL export exactly one schema object (`DB_SCHEMA`) describing the store definitions. Legacy exports (`DB_SCHEMA_V1`, `DB_SCHEMA_V2`, and the deprecated `DB_SCHEMA` alias pointing at V1) SHALL be removed, along with the obsolete `needsSync` index. + +#### Scenario: Only the baseline schema is exported +- **WHEN** a module imports the database schema +- **THEN** it imports a single `DB_SCHEMA` object +- **AND** no versioned schema variants (`DB_SCHEMA_V1`, `DB_SCHEMA_V2`) are exported +- **AND** `DB_SCHEMA` contains the `syncStatus` index and does not contain a `needsSync` index