From 0d52858bbd0b7da7763ece636aa23fd16f6fd539 Mon Sep 17 00:00:00 2001 From: varutasu <104105839+varutasu@users.noreply.github.com> Date: Sat, 15 Aug 2026 16:31:37 -0500 Subject: [PATCH] docs: post-hoc B7 closure for reconcile-historical-add-scripts (#163) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ships Brief 7 (documentation + verification runbook) of the reconcile-historical-add-scripts convoy, ~2 months post-hoc. The 6 implementer briefs (B1-B6) landed 2026-06-14 to 2026-07-06 via PRs #148, #149, #150, #151, #152, #153. This PR closes the loop: - Brings the architect's parent convoy file + 6 brief files onto main (they only existed on the stale convoy/reconcile-historical- add-scripts branch, never merged) - Adds § As-shipped to the parent convoy file documenting all 6 squash SHAs + PR numbers + merge dates + the reservation-timestamp rename (1781000000001-006 → 1781442330001-006 in ec9bb2b, except B3 which kept its original) + the B6 shipped-as-tiny-migration deviation from the collapse-to-docs plan - Fixes docs/SCHEMA_MAP.md § user_favorites (was stale (user_id, card_id); actual polymorphic (item_type, item_id) per B4's migration) - Adds docs/MIGRATION_VERIFICATION_RUNBOOK.md — manual fresh-Neon-branch vs prod pg_dump diff runbook per architect D5 - Flips .convoys/ship-readiness.md entries: - reconcile-historical-add-scripts → RESOLVED - retire-graveyard-scripts-after-audit → UNBLOCKED - Adds two new queued follow-ups surfaced by the architect: - unify-user-avatar-column (P3 — dual avatar column smell) - drop-dead-cards-columns (P3 — cards.quantity + cards.favorited) No source-code changes. Docs only. Co-authored-by: Cursor --- .convoys/reconcile-historical-add-scripts.md | 790 ++++++++++++++++++ .../brief-1-reconcile-cards-columns.md | 228 +++++ .../brief-2-reconcile-collections-columns.md | 307 +++++++ .../brief-3-reconcile-collaboration-tables.md | 318 +++++++ .../brief-4-reconcile-favorites-system.md | 241 ++++++ .../brief-5-reconcile-user-profile.md | 425 ++++++++++ .../brief-7-documentation-and-verification.md | 457 ++++++++++ .convoys/ship-readiness.md | 11 +- docs/MIGRATION_VERIFICATION_RUNBOOK.md | 149 ++++ docs/SCHEMA_MAP.md | 9 +- 10 files changed, 2930 insertions(+), 5 deletions(-) create mode 100644 .convoys/reconcile-historical-add-scripts.md create mode 100644 .convoys/reconcile-historical-add-scripts/brief-1-reconcile-cards-columns.md create mode 100644 .convoys/reconcile-historical-add-scripts/brief-2-reconcile-collections-columns.md create mode 100644 .convoys/reconcile-historical-add-scripts/brief-3-reconcile-collaboration-tables.md create mode 100644 .convoys/reconcile-historical-add-scripts/brief-4-reconcile-favorites-system.md create mode 100644 .convoys/reconcile-historical-add-scripts/brief-5-reconcile-user-profile.md create mode 100644 .convoys/reconcile-historical-add-scripts/brief-7-documentation-and-verification.md create mode 100644 docs/MIGRATION_VERIFICATION_RUNBOOK.md diff --git a/.convoys/reconcile-historical-add-scripts.md b/.convoys/reconcile-historical-add-scripts.md new file mode 100644 index 0000000..4de66f4 --- /dev/null +++ b/.convoys/reconcile-historical-add-scripts.md @@ -0,0 +1,790 @@ +--- +name: reconcile-historical-add-scripts +classification: quality +priority: P1 (last open P1 in launch queue) +success_metric: | + A brand-new Neon branch can be onboarded by `npm install` → + `npm run setup-db` alone. After `setup-db` exits 0, the fresh-env + schema (`information_schema.tables` + `.columns` + `.table_constraints` + + `pg_indexes`) is structurally equivalent to prod for every + table/column/index/constraint that runtime code in `pages/api/**`, + `lib/**`, `scripts/**`, and the 4 prior post-backfill migrations + depend on. `.convoys/ship-readiness.md` § "Queued convoys" → + `reconcile-historical-add-scripts` flips from open → RESOLVED; + `retire-graveyard-scripts-after-audit` unblocks as a follow-up. +skip: + - role-design-system-auditor + - role-a11y-auditor + - role-ux-reviewer + - role-ia-architect +status: open +created: 2026-06-14 +parent: migration-tool +addresses: migration-tool § R1 (prod schema drift from setup-neon-db.js DDL) +depends_on: + - migration-tool (PR #32 — provides `node-pg-migrate` + initial backfill) +--- + +# Convoy: reconcile-historical-add-scripts + +Fold the effects of the 13 historical `scripts/add-*.js` / `scripts/fix-*.js` +/ `scripts/seed-*.js` jobs into the migration history so a brand-new Neon +branch can be onboarded by `npm install` → `npm run setup-db` alone, +without manually replaying the historical scripts. Closes the last gap +identified by `migration-tool` § R1 ("Prod schema drift from +`setup-neon-db.js` DDL"). + +## Background — the graveyard residue + +`migration-tool` (PR #32, 2026-05-26) adopted `node-pg-migrate` and +captured `scripts/setup-neon-db.js`'s 7-table bootstrap DDL into +`migrations/1779853647564_initial-schema.js`. Four post-backfill +migrations followed (`add-pg-trgm-card-name-index`, `add-scan-tables`, +`add-user-cards-scan-image-url`, `system-collection-description`). The +migration history's current shape is: + +| Migration | Captures | +| --- | --- | +| `1779853647564_initial-schema` | The 7 bootstrap tables: `users`, `cards`, `user_cards`, `collections`, `collection_cards`, `decks`, `deck_cards` | +| `1779853647565_add-pg-trgm-card-name-index` | `pg_trgm` extension + `idx_cards_name_trgm` GIN | +| `1779853647566_add-scan-tables` | `card_submissions`, `scan_attempts`, 2 indexes | +| `1779908094455_add-user-cards-scan-image-url` | `user_cards.scan_image_url` column | +| `1780378340194_system-collection-description` | `collections.is_system_collection` column + description text backfill | + +`migration-tool` § R1 documented that the bootstrap shape captured by +the initial backfill is **not** the full prod shape. Between +`setup-neon-db.js` and prod-today, 13 historical scripts have added +columns, tables, indexes, constraints, and one-shot data migrations +that are baked into every long-lived environment but are NOT +reproduced by `npm run migrate up` on a brand-new Neon branch. Code +in `pages/api/**`, `lib/**`, and the schema-aware test fixtures all +assume the post-historical shape; a fresh-env onboarding therefore +breaks the first time runtime code touches an uncaptured column or +table (`user_favorites`, `collection_permissions`, `user_settings`, +`user_avatars`, `users.first_name`, `collections.visibility`, etc.). + +This convoy reconstructs the missing migration history by reading +each historical script's SQL, classifying it, and (where the DDL is +not already captured) writing a new idempotent `node-pg-migrate` +migration that brings fresh envs to parity with prod. Historical +scripts themselves are NOT edited — they remain no-go-zones per +`.cursor/rules/no-go-zones.mdc`. After this convoy lands, the queued +`retire-graveyard-scripts-after-audit` (P3) unblocks. + +## Inventory (13 scripts audited) + +Legend: +- **C** = captured (DDL already present in `migrations/`) +- **M** = missed (DDL exists in prod via the script but no migration captures it) +- **DML-only** = no DDL; either pure data migration or dev-fixture seed +- **Mixed** = DDL captured but accompanying DML backfill not captured + +### add-* (9) + +| # | Script | DDL summary | DML summary | Classification | Captured by | +| --- | --- | --- | --- | --- | --- | +| 1 | `add-card-columns.js` | `ALTER TABLE cards ADD COLUMN IF NOT EXISTS quantity INTEGER DEFAULT 0`; `ALTER TABLE cards ADD COLUMN IF NOT EXISTS favorited BOOLEAN DEFAULT false` | — | **M** | none — neither column is in `initial-schema`'s `CREATE TABLE cards`. Both flagged as "unused" smells in `docs/SCHEMA_MAP.md` § "Known schema smells" #3 | +| 2 | `add-collaboration-features.js` | `ALTER collections ADD visibility VARCHAR(20) DEFAULT 'private', tcg VARCHAR(50) DEFAULT 'MTG', tags TEXT`; `CREATE TABLE collection_permissions` (incl. role/status CHECKs + invite_token UNIQUE + `UNIQUE(collection_id, user_id)`); `CREATE TABLE collection_activity` (`details JSONB`); `ALTER users ADD is_pending BOOLEAN DEFAULT false`; 4 indexes | One-time owner-permission backfill: `INSERT INTO collection_permissions ... 'owner', 'active'` for every existing collection lacking one | **M** | none — `initial-schema` only has the 3-column `collections` bootstrap and no `collection_permissions` / `collection_activity` | +| 3 | `add-collection-slugs.js` | `ALTER collections ADD slug VARCHAR(100) UNIQUE`; `CREATE UNIQUE INDEX idx_collections_slug`; `ALTER ... ADD CONSTRAINT check_slug_format CHECK (slug ~ '^[a-z0-9]([a-z0-9-]*[a-z0-9])?$' AND length(slug) <= 50)` | Per-row UPDATE to backfill slugs from `name` via `lib/slug-utils.js::generateUniqueSlug` | **M** | none | +| 4 | `add-favorites-system.js` | `CREATE TABLE user_favorites (id, user_id FK CASCADE, item_type VARCHAR(50), item_id INTEGER, created_at, UNIQUE(user_id, item_type, item_id))`; 4 indexes | — | **M** | none | +| 5 | `add-image-column.js` | `ALTER collections ADD image TEXT` | — | **M** | none | +| 6 | `add-system-collection-column.js` | `ALTER collections ADD is_system_collection BOOLEAN DEFAULT false` | Per-user backfill: creates one `'All My Cards'` system collection + owner permission row for every user lacking one | **Mixed** | `1780378340194_system-collection-description.js` captures the DDL (`ADD COLUMN IF NOT EXISTS is_system_collection`). The per-user backfill DML is genuinely captured by the user-registration hook at `pages/api/auth/register.js:97-119` (`INSERT INTO collections ... is_system_collection=true` on every new user — verified by parent agent post-architect-pass, 2026-06-14). No further work required; see Finding 4 RESOLVED below. | +| 7 | `add-updated-at-column.js` | `ALTER cards ADD updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP` | — | **C** | `1779853647564_initial-schema.js` — `cards` `CREATE TABLE` already includes `updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP` (line 67). Script is now a no-op on fresh envs. No work required. | +| 8 | `add-user-profile-columns.js` | `ALTER users ADD first_name, last_name VARCHAR(255), username VARCHAR(255) UNIQUE, profile_image_url TEXT` | Per-row UPDATE: defaults `first_name='User'`, `last_name=`, `username='_'` | **M** (overlaps #9 — see Drift findings) | +| 9 | `add-user-profile-fields.js` | `ALTER users ADD first_name, last_name VARCHAR(255), username VARCHAR(255) UNIQUE, bio TEXT, avatar_url TEXT`; `ALTER users ADD favorite_games JSONB DEFAULT '["MTG"]', collection_visibility VARCHAR(20) DEFAULT 'private', preferred_currency VARCHAR(3) DEFAULT 'USD', cards_per_page INTEGER DEFAULT 50, default_view VARCHAR(10) DEFAULT 'grid'`; `ALTER users ADD notifications_email BOOLEAN DEFAULT true, notifications_marketing BOOLEAN DEFAULT false, two_factor_enabled BOOLEAN DEFAULT false`; `ALTER users ADD theme VARCHAR(10) DEFAULT 'system', language VARCHAR(5) DEFAULT 'en'`; `CREATE TABLE user_settings (id, user_id FK CASCADE, setting_key VARCHAR(100), setting_value JSONB, UNIQUE(user_id, setting_key))`; `CREATE TABLE user_avatars (id, user_id FK CASCADE, filename, original_name, mime_type, file_size, file_path, is_active BOOLEAN DEFAULT true)`; 6 CHECK constraints (`check_collection_visibility`, `check_preferred_currency`, `check_cards_per_page`, `check_default_view`, `check_theme`, `check_language`); 6 indexes | UPDATE on users to write defaults for any row with NULLs in the new columns | **M** | + +### fix-* (2) + +| # | Script | DDL summary | DML summary | Classification | Captured by | +| --- | --- | --- | --- | --- | --- | +| 10 | `fix-lorcana-images.js` | — | UPDATE on `cards WHERE game='Lorcana' AND image_url LIKE '%-716.webp' OR '%-512.webp'`: rewrite to `-1024.webp` for `image_url`, keep small as `stock_image_url` | **DML-only** | n/a — content-level data fix tied to a specific historical image-CDN payload shape. Re-running it on a fresh env that imports Lorcana via the canonical `import-lorcana` path would do nothing (new imports already use `-1024.webp`). | +| 11 | `fix-user-cards-constraints.js` | `ALTER user_cards ADD CONSTRAINT user_cards_user_card_unique UNIQUE (user_id, card_id)`; `ALTER collection_cards ADD CONSTRAINT collection_cards_collection_card_unique UNIQUE (collection_id, card_id)` | Dedup `user_cards` duplicates by `user_id, card_id`; sync owned cards to each user's `'All My Cards'` system collection | **M** (with conflict — see Drift findings) | DDL is **not** captured; the constraint exists in prod but not on fresh envs. Conflict: `initial-schema` already declares `UNIQUE(user_id, card_id, is_foil)` (3-col) on `user_cards`; this script adds a **stricter** 2-col `UNIQUE(user_id, card_id)` that contradicts the foil-distinguishing semantics encoded in the bootstrap. Halt-and-ask flagged below. | + +### seed-* (2) + +| # | Script | DDL summary | DML summary | Classification | Captured by | +| --- | --- | --- | --- | --- | --- | +| 12 | `seed-collections-alice-bob.js` | — | `DELETE FROM collection_cards / collection_permissions / collections` (destructive!); then INSERT 4 Alice + 5 Bob fixture collections | **DML-only** (dev fixture) | n/a — wires test users for local UI work; not migration material. | +| 13 | `seed-collections-with-cards.js` | — | Same destructive wipe; INSERT 13 sample cards (Black Lotus, Charizard, Elsa, …); INSERT 6 fixture collections with varied empty/thumbnail/card states | **DML-only** (dev fixture) | n/a — UI demo content; not migration material. | + +### Inventory counts + +- **Already captured (no work):** 2 — `add-updated-at-column.js` (#7) fully; `add-system-collection-column.js` (#6) DDL by `1780378340194` + DML backfill by `pages/api/auth/register.js:97-119` (Finding 4 RESOLVED) +- **Missed DDL (needs migration):** 7 — #1, #2, #3, #4, #5, #8∪#9 (deduplicate). #11's DDL is NOT folded into a migration (Finding 1 RESOLVED → Outcome A; see below) +- **DML-only (not migration material):** 3 — #10 fix-lorcana, #12 seed-alice-bob, #13 seed-with-cards +- **Halt-and-ask:** 0 (Finding 1 RESOLVED below) + +## Drift findings + +### Finding 1 — `user_cards` UNIQUE constraint conflict (RESOLVED — Outcome A, 2026-06-14) + +**Resolution:** Operator ratified **Outcome A** post-architect-pass. The +3-col `UNIQUE(user_id, card_id, is_foil)` from +`migrations/1779853647564_initial-schema.js` is canonical and matches +the actual prod state. `scripts/fix-user-cards-constraints.js` was +either never applied to prod OR was applied + reverted at some point; +the canonical current state has the 3-col constraint, foil/non-foil +distinction is a real product invariant, and a separate fixed 2-col +`UNIQUE(user_id, card_id)` is **not present** on the prod `user_cards` +table. + +**Consequence for this convoy:** B6 collapses entirely. No migration +is written for `fix-user-cards-constraints.js`. Per the recommendation +in the operator decision, a no-op documentation-only migration would +add clutter to `pgmigrations` for zero functional benefit; instead, +this Drift Finding entry + a sentence in B7's `docs/SCHEMA_MAP.md` +update + a sentence in the As-shipped section document the reasoning. + +The historical script remains a no-go-zone per +`.cursor/rules/no-go-zones.mdc`; the queued +`retire-graveyard-scripts-after-audit` (P3) will delete or move it +along with the other 12 historical scripts. + +The original audit narrative is preserved below for cross-reference. + +--- + +**Original audit (pre-resolution):** + +`migrations/1779853647564_initial-schema.js` line 82 declares: + +``` +UNIQUE(user_id, card_id, is_foil) +``` + +`scripts/fix-user-cards-constraints.js` lines 50-54 adds (separately, as +a named constraint): + +``` +ALTER TABLE user_cards +ADD CONSTRAINT user_cards_user_card_unique UNIQUE (user_id, card_id) +``` + +These are **semantically incompatible** when more than one foil/non-foil +copy of the same `(user_id, card_id)` exists: + +- The bootstrap 3-column tuple allows `(1, 42, false)` AND `(1, 42, true)` + (user owns one regular + one foil copy of card 42). +- The fix-script's 2-column constraint **forbids** that pair. + +In any prod environment where `fix-user-cards-constraints.js` was run +successfully (the script's dedup-first step would have removed +duplicates pre-constraint), the stricter constraint is in force AND +the looser tuple-implicit one also exists (autogen-named like +`user_cards_user_id_card_id_is_foil_key`). Two coexisting constraints +do not break Postgres semantics — the stricter wins for write +rejection. + +Same situation on `collection_cards`: initial-schema's +`UNIQUE(collection_id, card_id)` (2-column) and the fix script's +`collection_cards_collection_card_unique` are **identical** in column +set, so this half is benign (Postgres rejects the duplicate at ADD +CONSTRAINT time; the script's caught `if (error.message.includes('already exists'))` swallows it). + +**Why this is a halt-and-ask:** the runtime code path that touches +`is_foil` differentiation (see `pages/api/cards/[id]/own.js` and +`pages/api/user-cards.js`) needs to be audited to decide which +constraint matches actual product intent. Possible outcomes: + +- **Outcome A:** Foil/non-foil distinction is a real product invariant + (a user should be able to track foil + non-foil copies of the same + card separately). The new migration should `DROP CONSTRAINT + user_cards_user_card_unique IF EXISTS` on prod, then ensure the + tuple constraint is the only one. Fresh envs already have the + correct tuple constraint from `initial-schema`; no add needed. +- **Outcome B:** Foil/non-foil should be unified at the user_cards row + level (use a separate `foil_quantity` column instead). This is a + product decision that belongs to a separate convoy + (`unify-user-cards-foil-tracking`). +- **Outcome C:** Both are tolerable (current prod state). The new + migration should add `ALTER user_cards ADD CONSTRAINT + user_cards_user_card_unique UNIQUE (user_id, card_id) IF NOT + EXISTS`-equivalent on fresh envs to match prod, AND surface the + smell in `docs/SCHEMA_MAP.md`. + +**Halt point:** before B6 is written, operator picks A / B / C. **Recommended +default** if no decision arrives in 48h: **C** (replicate prod as-is; +surface as smell). Outcome A is the closest to "intent" but adding a +mid-convoy DROP CONSTRAINT on prod data deserves its own scoped +review. + +_— Resolved 2026-06-14 as Outcome A (no DROP needed — the strict +constraint is not actually present on prod). See resolution block at +the top of this Finding._ + +### Finding 2 — `users.username` `profile_image_url` vs `avatar_url` redundancy + +Scripts #8 (`add-user-profile-columns.js`) and #9 (`add-user-profile-fields.js`) +both add `first_name`, `last_name`, `username VARCHAR(255) UNIQUE` — the +overlap is idempotent (both use `ADD COLUMN IF NOT EXISTS`) so prod +ended up with the union. But: + +- #8 adds `profile_image_url TEXT` +- #9 adds `avatar_url TEXT` + +`docs/SCHEMA_MAP.md` § "Known schema smells" #1 already flags this +redundancy ("Pick one"). For this convoy: the new migration MUST add +**both** columns to match prod-as-is (since runtime code may read +either — to be verified). Surface as follow-up +`unify-user-avatar-column`. + +### Finding 3 — `cards.quantity` and `cards.favorited` are dead columns + +`add-card-columns.js` adds `quantity INTEGER DEFAULT 0` and `favorited +BOOLEAN DEFAULT false` to the `cards` table. `docs/SCHEMA_MAP.md` +flags both as **Unused** (smell #3). The actual `quantity` / `favorited` +semantics live on `user_cards` / `user_favorites`. + +For this convoy: add both columns to fresh envs to match prod. Do NOT +drop them in prod (separate convoy). Surface as follow-up +`drop-dead-cards-columns` (deferred until a query-trace audit confirms +zero readers). + +### Finding 4 — `add-system-collection-column.js` DML backfill (RESOLVED — verified, 2026-06-14) + +**Resolution:** Verified by parent agent post-architect-pass. The +`pages/api/auth/register.js` handler at **lines 97-119** creates the +system collection on every new user signup: + +```js +const collectionResult = await sql` + INSERT INTO collections ( + name, description, tcg, is_public, user_id, slug, + is_system_collection, created_at, updated_at + ) + VALUES ( + ${SYSTEM_COLLECTION_DB_NAME}, + ${VOCAB.SYSTEM_COLLECTION_SEED_DESCRIPTION}, + 'All', false, ${user.id}, ${uniqueSlug}, + true, -- is_system_collection + CURRENT_TIMESTAMP, CURRENT_TIMESTAMP + ) + RETURNING id +`; +``` + +The runtime invariant is intact. The historical script's per-user +backfill DML was the **one-time** reconciliation for legacy users +created before the register-hook existed; new envs never need it +because their users are all created post-hook. + +For this convoy: **no migration, no follow-up convoy needed.** The +`add-system-collection-on-register` follow-up originally surfaced in +the prior architect pass is **withdrawn**. B7 will cite this +verification in `docs/SCHEMA_MAP.md` § `collections.is_system_collection` +notes. + +### Finding 5 — Seed scripts wipe collection data + +Both `seed-collections-alice-bob.js` and `seed-collections-with-cards.js` +begin with `DELETE FROM collection_cards / collection_permissions / +collections` — destructive against any environment where real user +data exists. They are clearly tagged as dev fixtures (the test users +`alice@tcgvault.com` / `bob@tcgvault.com` are local-only). Per D4 +below, these stay out of migrations entirely. + +## Decisions (post-IA round; this is a P1 quality convoy that skipped IA/UX/A11y/Design — see frontmatter `skip:`) + +### D1 — Brief grouping: option (b), grouped by logical surface + +**Ratified: 6 implementer briefs grouped by table/feature surface.** +(Originally 7; B6 collapsed post-Finding-1 resolution — see § Brief outline.) + +Missed-DDL count is 8 (above the 1-3 threshold for option (d) and +above the 4-thing threshold that triggered option (b/c) in the +parent prompt). One-brief-per-script (option a, 13 briefs) is +excessive — many scripts touch the same DDL surface and ship one +migration each would create unnecessary `pgmigrations` rows and +make rollback narrative confusing. + +The 6-brief plan groups every missed DDL by the table or feature it +touches: + +- **B1** — Cards table reconciliation (#1, audit #7) +- **B2** — Collections table reconciliation (#2 partial, #3, #5) +- **B3** — Collaboration tables (#2 partial — `collection_permissions`, + `collection_activity`, `users.is_pending`) +- **B4** — Favorites system (#4) +- **B5** — User profile reconciliation (#8 ∪ #9, dedup'd; + + `user_settings`, `user_avatars`; + CHECK constraints; + indexes) +- **B7** — Documentation + verification runbook (`docs/SCHEMA_MAP.md` + refresh, AGENTS.md Gotcha #6 update, operator runbook) + +**B6 was removed post-Finding-1 resolution** (Outcome A — the strict +2-col `UNIQUE(user_id, card_id)` is not present on prod; the canonical +3-col `UNIQUE(user_id, card_id, is_foil)` from `initial-schema` is +what fresh envs already get). No migration is needed. The brief +numbering preserves the original gap (B1-B5 + B7) so cross-references +to "B6" in prior drafts of this convoy file remain unambiguous (they +point to the removed brief). + +Rationale: +1. **Independence.** Each of B1-B6 writes a new file under `migrations/` + with a fresh timestamp; the files don't overlap, so B1-B6 are + trivially parallel (subject to timestamp ordering — see slice + dependencies block below). +2. **Reviewability.** Each PR is one migration + one optional + `docs/SCHEMA_MAP.md` section update; reviewer can verify against + the historical script in a single sitting. +3. **Rollback granularity.** If B5's user-profile migration is found + buggy post-merge, the other 5 migrations are unaffected — they all + `pgmigrations`-row independently. +4. **Estimated LOC per brief stays under 400.** B5 is the largest + (~250 LOC of SQL across 13 ALTERs + 2 CREATE TABLE + 6 CHECKs + 6 + indexes) — comfortably under budget. + +Considered alternatives: + +| Option | Why rejected | +| --- | --- | +| (a) one brief per script (13 briefs) | Splits #2 across `collections`-columns vs `collection_permissions` vs `users.is_pending` artificially. Pads review surface 2x. | +| (c) one DDL + one DML + one verification brief (3 briefs) | A single DDL brief would be a 500+ LOC mega-migration. Hard to review, hard to rollback. | +| (d) single PR | Missed-DDL count >> 1; rejected by spec. | + +### D2 — Idempotency pattern: raw `pgm.sql(...)` with `IF [NOT] EXISTS` guards + +**Ratified: raw `pgm.sql(...)`-style SQL matching `1779853647564_initial-schema.js`'s pattern, every statement guarded with `IF NOT EXISTS` (for CREATE) or `IF EXISTS` (for DROP).** + +Rationale: +1. **Style consistency.** The five existing migrations all use + `pgm.sql()`. Mixing `pgm.createTable()` helpers would introduce a + second pattern for no operational benefit. +2. **Verbatim SQL transparency.** The reviewer can grep the new + migration's SQL string against the historical script's SQL string + and confirm byte-equivalent intent. +3. **Idempotency.** Every CREATE / ALTER ADD COLUMN / CREATE INDEX + statement uses the appropriate `IF NOT EXISTS` guard so re-running + against prod is a documented no-op. CHECK constraints don't accept + `IF NOT EXISTS` directly — wrap in a `DO $$ BEGIN ... EXCEPTION + WHEN duplicate_object THEN NULL; END $$;` block (matching the + historical script's try/catch pattern). + +`node-pg-migrate` helpers (`pgm.createTable`, `pgm.addColumns`) would +work fine technically, but the existing migration corpus is 100% raw +SQL. Optimize for review uniformity. + +### D3 — Drift detection: option (a) — assume change is in prod, add a migration to bring fresh envs to parity + +**Ratified: (a) with a documented "evidence" trail per script in the Inventory section above.** + +For every script in the **M** classification, the assumption is that +the script ran successfully against prod at some point and its DDL is +now baked into the prod schema. Evidence supporting this assumption: + +- `docs/SCHEMA_MAP.md` (last reviewed 2026-05-22) documents all of these + columns/tables as live. +- Runtime code in `pages/api/**` reads/writes `user_favorites`, + `collection_permissions`, `collection_activity`, `user_settings`, + `user_avatars`, `users.first_name`, `users.username`, `users.bio`, + `users.avatar_url`, `users.theme`, `users.language`, + `collections.visibility`, `collections.tcg`, `collections.tags`, + `collections.slug`, `collections.image` — confirmed via grep + (`pages/api/user/settings.js`, `pages/api/user/profile.js`, + `pages/api/favorites.js`, `pages/api/invite/{accept,decline}.js`, + `pages/api/collections/[identifier]/permissions.js`, etc.). +- Each historical script is itself idempotent (`ADD COLUMN IF NOT + EXISTS`), so prod-vs-fresh divergence is the live state. + +For Finding 1 (UNIQUE constraint conflict), Finding 4 (system-coll +backfill), and Finding 5 (seed wipes): per the recommendation in the +parent prompt, halt-and-ask is reserved for destructive changes. (1) +qualifies (DROP CONSTRAINT) → halt-and-ask. (4) is operationally +benign (no DROP) → defer to follow-up. (5) is destructive but the +recommended choice is "don't fold into migrations at all" — D4 below. + +### D4 — Seed scripts: option (a) — leave alone + +**Ratified: dev-fixture seed scripts stay out of the migration history.** + +`seed-collections-alice-bob.js` and `seed-collections-with-cards.js` +are dev-fixture loaders tied to `alice@tcgvault.com` / +`bob@tcgvault.com` (which only exist in `scripts/create-test-users.js`, +gated post-`purge-weak-creds-from-helpers` behind `TEST_USERS_PASSWORD`). +They: + +- Wipe destructive collection data (incompatible with any env with real users). +- Reference Disney/Lorcana sample art URLs (`example.com/elsa.jpg`). +- Are clearly UI-demo fodder. + +They have no place in `setup-db`'s migrate pipeline. They will be +retired (or moved to `scripts/historical/`) by the queued +`retire-graveyard-scripts-after-audit` convoy. + +If multiple developers feel friction managing dev-fixture state in +the future, surface a separate `consolidate-dev-seeds` follow-up +that designs a non-destructive `npm run seed:dev` flow keyed off a +fresh DB. Not surfaced from this convoy because no friction has been +reported yet. + +### D5 — Verification: option (a) — manual operator runbook + +**Ratified: post-merge manual verification by spinning up a fresh +Neon branch + running `npm run setup-db` + diffing schema against +prod.** + +Automated CI verification belongs to the queued `wire-migrate-into-ci` +convoy and is out of scope here. The manual runbook will live in this +convoy file's "Verification plan" section (below) and be promoted to +`docs/operations/RECONCILE-VERIFICATION.md` by B7. + +### D6 — `scripts/migrations/2026-05-24-rename-admin-email.js`: option (b) — leave where it is + +**Ratified: do not move into `migrations/`.** + +The lone pre-tool migration sits at `scripts/migrations/2026-05-24-rename-admin-email.js` +and was applied to every long-lived env at `pick-a-name` time +(2026-05-24). Moving it into `migrations/` would require backfilling a +`pgmigrations` row on every existing env, which is operationally +risky for zero functional benefit (the migration is already applied; +the runtime never re-checks it). + +For fresh envs, the migration is a no-op (the seed admin row is +already created at `admin@deckhearth.com` by post-rename +`scripts/setup-neon-db.js`; there's no `@tcgvault.com` row to rename). + +Document the rationale in B7's `docs/SCHEMA_MAP.md` update + a brief +note in AGENTS.md Gotcha #4's "Rotation script" subsection. If a +future fresh-env onboarding ever tries to roll back to pre-rename +state, surface `fold-rename-admin-email-into-migrations` as a +follow-up. + +### D7 — `pgmigrations` table state on fresh envs: no special handling required + +**Ratified: rely on `node-pg-migrate`'s standard sequential apply.** + +On a fresh Neon branch, `npm run migrate up` will run all migrations +in timestamp order: + +1. `1779853647564_initial-schema` (7 bootstrap tables) +2. `1779853647565_add-pg-trgm-card-name-index` +3. `1779853647566_add-scan-tables` +4. `1779908094455_add-user-cards-scan-image-url` +5. `1780378340194_system-collection-description` +6. **NEW: B1** — cards columns (quantity, favorited) +7. **NEW: B2** — collections columns (visibility, tcg, tags, slug, slug index/constraint, image) +8. **NEW: B3** — collaboration tables (collection_permissions, collection_activity, users.is_pending, 4 indexes) +9. **NEW: B4** — favorites system (user_favorites + 4 indexes) +10. **NEW: B5** — user profile reconciliation (15 ALTER-ADD-COLUMN + 2 CREATE TABLE + 6 CHECK + 6 indexes) + +Each new migration is additive against the post-initial-schema state +that prior migrations leave behind; no inter-migration dependencies +crossed within this convoy. + +Audited ordering risks for the new migrations: + +- B1 depends on `cards` (initial-schema) ✅ +- B2 depends on `collections` (initial-schema) ✅ +- B3 depends on `collections` + `users` (initial-schema) ✅ +- B4 depends on `users` (initial-schema) ✅ +- B5 depends on `users` (initial-schema) ✅ + +For long-lived envs that already have all post-historical columns: +every new migration is a documented no-op due to `IF NOT EXISTS` +guards. The only state change is the `pgmigrations` row insertion. + +## Brief outline + +Six implementer briefs. Each B1-B5 ships exactly one new file under +`migrations/_.js`. B7 updates docs only. Implementer +briefs are drafted under `.convoys/reconcile-historical-add-scripts/brief--.md`. + +| Brief | Title | Files (new) | Depends on | Est. LOC | Notes | +| --- | --- | --- | --- | --- | --- | +| B1 | Reconcile cards columns | `migrations/1781000000001_reconcile-cards-columns.js` | — | ~40 | Captures script #1; script #7 already captured by initial-schema (verify & document in PR body) | +| B2 | Reconcile collections columns | `migrations/1781000000002_reconcile-collections-columns.js` | — | ~120 | Captures #2 (collections half: visibility/tcg/tags), #3 (slug + index + CHECK), #5 (image). Defer slug backfill DML — generating slugs on a fresh env is moot. | +| B3 | Reconcile collaboration tables | `migrations/1781000000003_reconcile-collaboration-tables.js` | — | ~130 | Captures #2 (collaboration half: collection_permissions, collection_activity, users.is_pending, 4 indexes). Defer owner-permission backfill DML — fresh envs have no pre-existing collections needing backfill. | +| B4 | Reconcile favorites system | `migrations/1781000000004_reconcile-favorites-system.js` | — | ~50 | Captures #4 (user_favorites + 4 indexes). | +| B5 | Reconcile user profile | `migrations/1781000000005_reconcile-user-profile.js` | — | ~250 | Captures #8 ∪ #9 (deduplicated; both add columns coexist in prod). Includes user_settings, user_avatars, 6 CHECK constraints (wrapped in `DO $$ EXCEPTION` blocks for idempotency), 6 indexes. Defer defaults-backfill UPDATE — column DEFAULTs handle it. | +| ~~B6~~ | ~~Reconcile user_cards / collection_cards UNIQUE constraints~~ | ~~removed~~ | — | 0 | **REMOVED 2026-06-14 — Finding 1 RESOLVED as Outcome A.** The strict 2-col `UNIQUE(user_id, card_id)` is not present on prod; the canonical 3-col tuple is already on fresh envs via `initial-schema`. Documented in Drift Finding 1 + B7's SCHEMA_MAP update; no migration written. | +| B7 | Documentation + verification | `docs/SCHEMA_MAP.md` (update); `docs/MIGRATION_VERIFICATION_RUNBOOK.md` (new); `AGENTS.md` (Gotcha #6 audit); `.convoys/ship-readiness.md` (flip queued entry) | B1-B5 merged | ~150 | SCHEMA_MAP smell-list updates per Findings 1-3. Verification runbook (see § Verification plan below). AGENTS.md Gotcha #6 already RESOLVED post-migration-tool; verify and leave alone if accurate. ship-readiness.md "Queued convoys" → flip this convoy entry to RESOLVED + unblock `retire-graveyard-scripts-after-audit` + add `unify-user-avatar-column` + `drop-dead-cards-columns` follow-ups. | + +Per-brief acceptance criteria (sketch — formalized in each brief file): + +- Each migration file has a `down()` that **throws** with a clear message + (these are reconciliation migrations; the prod schema state is the + source of truth and rolling back would create inconsistency). +- Each migration's SQL is **byte-equivalent in intent** to the + historical script's SQL (per-statement comments cite the script + and line range). +- Re-running `npm run migrate up` against any current long-lived env + is a no-op (all guards trigger). +- `npm run migrate up` against a fresh Neon branch followed by the + verification runbook (D5) confirms structural parity with prod. + +### Slice dependencies (multitask-ready) + +```yaml +slice_dependencies: + - brief: 1 + depends_on: [] + files: [migrations/1781000000001_reconcile-cards-columns.js] + - brief: 2 + depends_on: [] + files: [migrations/1781000000002_reconcile-collections-columns.js] + - brief: 3 + depends_on: [] + files: [migrations/1781000000003_reconcile-collaboration-tables.js] + - brief: 4 + depends_on: [] + files: [migrations/1781000000004_reconcile-favorites-system.js] + - brief: 5 + depends_on: [] + files: [migrations/1781000000005_reconcile-user-profile.js] + # brief 6 removed — Finding 1 RESOLVED as Outcome A; timestamp 1781000000006 is unused + - brief: 7 + depends_on: [1, 2, 3, 4, 5] + files: + - docs/SCHEMA_MAP.md + - docs/MIGRATION_VERIFICATION_RUNBOOK.md + - AGENTS.md + - .convoys/ship-readiness.md +``` + +**Timestamp coordination.** Each B1-B5 writes a +`migrations/_*.js` file with a pre-assigned timestamp +(reservation token, not literal `Date.now()`). Pre-assigned +timestamps avoid the parallel-implementer collision risk documented +in `scaffold-nextjs-app` retro recommendation #4. The brief frontmatter +in each `.convoys/reconcile-historical-add-scripts/brief--*.md` +file declares the exact path; implementers MUST use that exact +filename (not `npm run migrate create`, which would `Date.now()`). + +## Verification plan (D5 operator runbook) + +To verify the reconstructed migration history produces parity with +prod, after B1-B6 merge: + +```bash +# 1. Snapshot prod's structural shape (run against prod POSTGRES_URL) +# Schema-only dump, no data, no owner/ACL noise +POSTGRES_URL= pg_dump --schema-only --no-owner --no-acl \ + --schema=public > /tmp/prod-schema.sql + +# 2. Create a clean Neon branch from EMPTY (no parent branch) and onboard via setup-db +# (use the Neon dashboard or `neon branches create --empty`) +POSTGRES_URL= \ +ADMIN_INITIAL_PASSWORD=$(openssl rand -base64 24) \ + npm run setup-db + +# 3. Snapshot the fresh branch's structural shape +POSTGRES_URL= pg_dump --schema-only --no-owner --no-acl \ + --schema=public > /tmp/fresh-schema.sql + +# 4. Diff. Expected differences are limited to: +# - constraint/index NAME differences (autogen tuple-UNIQUE names vs explicit names) +# - column-ORDER differences (prod has columns in historical-script-ALTER order; +# fresh envs have them in migration-order) +# Both are semantically irrelevant. Material differences = bug; flag and reopen. +diff <(sort /tmp/prod-schema.sql) <(sort /tmp/fresh-schema.sql) +``` + +Supplementary information-schema spot-checks for the highest-risk surfaces: + +```sql +-- Every column on every table +SELECT table_name, column_name, data_type, is_nullable, column_default +FROM information_schema.columns +WHERE table_schema = 'public' +ORDER BY table_name, ordinal_position; + +-- Every constraint +SELECT table_name, constraint_name, constraint_type +FROM information_schema.table_constraints +WHERE table_schema = 'public' +ORDER BY table_name, constraint_name; + +-- Every index +SELECT tablename, indexname, indexdef +FROM pg_indexes +WHERE schemaname = 'public' +ORDER BY tablename, indexname; +``` + +Run both against prod and fresh-branch; the column-count + constraint-count ++ index-count totals should match exactly. Mismatch = bug. + +B7 will move this runbook to `docs/operations/RECONCILE-VERIFICATION.md` +and link it from `AGENTS.md` Gotcha #6 + `migration-tool` § R1's +"resolved-by" note. + +## Risks + +### R1 — Missed DDL that is NOT actually in prod ("ghost migration") + +The assumption (D3) is that every historical script ran successfully +against every long-lived env. If a script in fact failed silently +mid-execution on prod (e.g. `add-collaboration-features.js`'s +`CREATE INDEX idx_collections_visibility` errored partway through), +prod might not actually have that index even though SCHEMA_MAP says +it does. + +**Mitigation:** the verification plan (D5) catches this. The fresh-env +`pg_dump` would contain the index; prod's `pg_dump` would not; the +diff would surface it. If found, operator decides: (a) the script's +intent was sound, apply the missed DDL to prod manually with +`POSTGRES_URL= psql -c "CREATE INDEX IF NOT EXISTS ..."`; or +(b) the index is unwanted, drop it from the new migration and document. + +### R2 — `pgmigrations` row state on existing prod envs + +After this convoy merges, an operator running `npm run migrate up` +against prod will see 6 new migrations apply (B1-B6) as no-ops (every +guarded statement triggers `IF [NOT] EXISTS`-skip). Six new +`pgmigrations` rows record successful application. + +If for some reason a long-lived prod env genuinely lacks one of the +historical-script columns (Risk R1 above), the corresponding migration +will **add** that column on apply, no-op-ing the others. The +`pgmigrations` row records success; subsequent applies are no-ops. +This is the correct behavior, but the operator should run the +verification plan post-apply to confirm. + +**Mitigation:** the verification plan covers prod-vs-fresh diff after +B1-B6 land. Run it once on each prod-shaped env immediately after +merge. + +### R3 — Ordering: new migration depends on prior schema-state that doesn't exist at its execution point + +Audited in D7. All B1-B6 dependencies on prior tables (`cards`, +`collections`, `users`, `user_cards`, `collection_cards`) are +satisfied by `initial-schema` (timestamp `1779853647564`, runs first +on fresh envs). No B-to-B inter-dependency required. + +### R4 — Seed scripts depend on test users that don't exist on fresh envs + +Out of scope (D4). The two seed scripts are dev fixtures and are not +folded into migrations. Test users (`alice@`, `bob@`) are managed by +`scripts/create-test-users.js` (post-`purge-weak-creds-from-helpers`, +gated behind `TEST_USERS_PASSWORD`). Surface as `consolidate-dev-seeds` +follow-up only if a developer reports friction. + +### R5 — CHECK constraint reapplication on prod is loud + +`add-user-profile-fields.js` wraps each CHECK constraint ADD in a +JS try/catch that swallows `already exists` errors. Postgres doesn't +accept `ADD CONSTRAINT ... IF NOT EXISTS` for CHECK; the SQL has to be +wrapped in `DO $$ ... EXCEPTION WHEN duplicate_object THEN NULL END +$$;`. B5's migration must use the exception-handling form so reapply +against prod is silent. Implementer brief will spell this out. + +### R6 — Mid-convoy timestamp collision when implementers spawn in parallel + +B1-B5 are mutually independent and can dispatch via `/multitask`. +Pre-assigned timestamps (D7 table) avoid the `Date.now()`-collision +risk that bit the `scaffold-nextjs-app` convoy. Each implementer +brief will name its file's exact timestamp; deviations require a +re-plan. + +### R7 — Operator approval lag on Finding 1 (RESOLVED 2026-06-14) + +Originally a risk because B6 was gated on operator decision. Finding 1 +RESOLVED as Outcome A; B6 removed. No lag risk remains. + +## Follow-ups + +Surfaced by this convoy: + +- **`retire-graveyard-scripts-after-audit`** (priority: **P3 polish — now + UNBLOCKED** once this convoy lands). Once B1-B6 capture every + missed historical DDL into the migration history, the 13 historical + scripts can be safely deleted or moved to `scripts/historical/` and + the corresponding no-go-zones rule line can be removed. Documented + in `.convoys/migration-tool.md` § Follow-ups; ship-readiness.md + "Queued convoys" entry; AGENTS.md Gotcha #6 cross-reference. +- **`wire-migrate-into-ci`** (priority: P2 CI infra — pre-existing). + Adds a CI job that runs `npm run migrate up` against a test DB on + every PR. This convoy's verification plan (D5) is the manual + precursor; the CI job is the automation upgrade. Already on the + follow-up list per `migration-tool` § Follow-ups + ship-readiness.md + "Queued convoys". +- **`unify-user-avatar-column`** (priority: P3 hygiene — NEW). Driven + by Finding 2. Two redundant TEXT columns + (`users.profile_image_url` and `users.avatar_url`) coexist; pick one + canonical column, migrate the other's data, drop the loser, update + runtime readers. Requires a query-trace audit first. +- **`drop-dead-cards-columns`** (priority: P3 hygiene — NEW). Driven + by Finding 3. `cards.quantity` and `cards.favorited` are documented + as unused. After a query-trace audit confirms zero readers, ship a + migration that DROPs them (with proper `down()` recreate). +- ~~**`add-system-collection-on-register`**~~ — **WITHDRAWN 2026-06-14**, Finding 4 RESOLVED. Verified the register hook exists at `pages/api/auth/register.js:97-119`. +- **`fold-rename-admin-email-into-migrations`** (priority: P3 polish, + conditional — NEW per D6). Only surface if a future fresh-env + onboarding needs the rename migration applied in order. Until then, + the script-shaped migration at + `scripts/migrations/2026-05-24-rename-admin-email.js` is left in + place per D6. +- ~~**`unify-user-cards-foil-tracking`**~~ — **WITHDRAWN 2026-06-14**, Finding 1 RESOLVED as Outcome A (not Outcome B). The 3-col tuple stays canonical; no foil-tracking redesign needed. + +Not surfaced (no friction yet): + +- `consolidate-dev-seeds` — D4 noted this can wait until multiple + developers report friction with the current per-script dev-fixture + loaders. + +## As-shipped + +Six implementer briefs (B1–B6) landed between 2026-06-14 and 2026-07-06. +Brief 7 (documentation + verification runbook) closed the convoy on +2026-08-15 — a post-hoc docs-only pass delayed by an operator-initiated +pause on a self-hosted runner infra issue that did not affect migration +content correctness. + +### Per-brief delivery + +| Brief | PR | Squash | Merged | Migration file | +| --- | --- | --- | --- | --- | +| B1 cards columns | #148 | `a35ce01` | 2026-06-14 | `migrations/1781442330001_reconcile-cards-columns.js` | +| B2 collections columns | #150 | `e8619c5` | 2026-06-14 | `migrations/1781442330002_reconcile-collections-columns.js` | +| B3 collaboration tables | #151 | `15e02ee` | 2026-07-06 | `migrations/1781000000003_reconcile-collaboration-tables.js` | +| B4 favorites system | #152 | `ef2cfb8` | 2026-06-14 | `migrations/1781442330004_reconcile-favorites-system.js` | +| B5 user profile | #153 | `ee7da9a` | 2026-06-14 | `migrations/1781442330005_reconcile-user-profile.js` | +| B6 user_cards UNIQUE | #149 | `40402eb` | 2026-06-14 | `migrations/1781442330006_reconcile-user-cards-unique.js` | +| B7 docs + runbook | _(this PR)_ | — | 2026-08-15 | _(no migration — docs only)_ | + +All six implementer PRs were green at merge on the (then-flaky) self-hosted +axiom runner pool. + +### Reservation-timestamp rename (`ec9bb2b`) + +The architect pre-assigned timestamps `1781000000001` through +`1781000000006` to avoid `Date.now()` collisions during parallel +implementer dispatch. Commit `ec9bb2b` (`feat(catalog): unified multi-game +bulk sync + schema map update`) renamed all except B3 to +`1781442330001`–`1781442330006` so they run **after** the catalog-sync +migrations that landed in the same period: + +- `1781440700404_add-scryfall-bulk-columns` +- `1781440721350_add-tagger-tables` +- `1781442175729_normalize-lorcana-set-codes` +- `1781442329511_add-catalog-sync-log` + +B3 kept its original `1781000000003` timestamp because it merged after the +rename commit. The rename is filename-only; migration `up()` content is +identical. + +### B6 deviation from architect plan + +The architect's gate-1 plan collapsed Finding 1 (Outcome A) into B7 as a +doc-only note. Implementers shipped B6 anyway as a tiny belt-and-suspenders +migration (`1781442330006_reconcile-user-cards-unique.js`) that: + +- Confirms the canonical 3-column `UNIQUE(user_id, card_id, is_foil)` from + `initial-schema` is present. +- Drops the historical 2-column `user_cards_user_card_unique` constraint if + any env ran `fix-user-cards-constraints.js`. + +The migration is a no-op on prod (Outcome A already holds) but guards +future envs that might have the stricter 2-col variant. + +### Verification + +Manual operator runbook for confirming fresh-env vs prod schema parity: +[`docs/MIGRATION_VERIFICATION_RUNBOOK.md`](../docs/MIGRATION_VERIFICATION_RUNBOOK.md) +(per architect Decision D5). Automated CI verification remains queued as +`wire-migrate-into-ci`. + +### B7 deviation from spec + +B7 shipped ~2 months post-hoc (2026-08-15) rather than immediately after +B1–B6. The delay was operator-initiated (runner-infra pause) and did not +block migration correctness — only documentation closure. diff --git a/.convoys/reconcile-historical-add-scripts/brief-1-reconcile-cards-columns.md b/.convoys/reconcile-historical-add-scripts/brief-1-reconcile-cards-columns.md new file mode 100644 index 0000000..fb33295 --- /dev/null +++ b/.convoys/reconcile-historical-add-scripts/brief-1-reconcile-cards-columns.md @@ -0,0 +1,228 @@ +--- +convoy: reconcile-historical-add-scripts +brief_number: 1 +depends_on: [] +files: + - migrations/1781000000001_reconcile-cards-columns.js +--- + +# Brief 1: Reconcile `cards` columns into migration history + +## Goal (1 sentence) + +Capture the DDL added by `scripts/add-card-columns.js` (the `cards.quantity` + `cards.favorited` columns) into a single new `node-pg-migrate` migration so a brand-new Neon branch ends up with both columns after `npm run setup-db`. + +## Scope (files in scope — do not edit anything else) + +- `migrations/1781000000001_reconcile-cards-columns.js` — **new** + +## Source script (read-only audit reference; DO NOT EDIT — no-go-zone) + +`scripts/add-card-columns.js` lines 22-33 (verbatim): + +```js +await sql` + ALTER TABLE cards + ADD COLUMN IF NOT EXISTS quantity INTEGER DEFAULT 0 +`; +// ... +await sql` + ALTER TABLE cards + ADD COLUMN IF NOT EXISTS favorited BOOLEAN DEFAULT false +`; +``` + +**Sibling script `scripts/add-updated-at-column.js`** is already captured by `migrations/1779853647564_initial-schema.js` (the `cards` `CREATE TABLE` at lines 45-69 already declares `updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP`). Do NOT add an ALTER for `updated_at` — it would be redundant noise on `pgmigrations`. Mention this in the PR body so reviewers don't ask. + +## Target migration file + +Path: `migrations/1781000000001_reconcile-cards-columns.js` + +Contents: + +```js +/** + * Reconcile historical `scripts/add-card-columns.js` into the migration + * history. Adds `cards.quantity` and `cards.favorited` columns so a + * brand-new Neon branch ends up with the same shape that prod has had + * since the historical script's one-shot run. + * + * Both columns are flagged as **unused** in docs/SCHEMA_MAP.md + * § "Known schema smells" #3 — `quantity` lives on `user_cards`, + * `favorited` lives on `user_favorites`. They're added here for + * fresh-env parity with prod. A follow-up `drop-dead-cards-columns` + * convoy (queued, P3 hygiene) will drop both columns once a query-trace + * audit confirms zero runtime readers. + * + * Idempotent: re-running against any long-lived prod env is a no-op + * because both ALTERs use IF NOT EXISTS. + * + * Note: `scripts/add-updated-at-column.js` (the sibling historical + * script in the same convoy) is NOT reconciled here because + * `cards.updated_at` is already declared in + * `migrations/1779853647564_initial-schema.js`'s `CREATE TABLE cards` + * (line 67). No further work needed for that script. + * + * @type {import('node-pg-migrate').ColumnDefinitions | undefined} + */ +export const shorthands = undefined; + +/** + * @param {import('node-pg-migrate').MigrationBuilder} pgm + */ +export const up = (pgm) => { + pgm.sql(` + ALTER TABLE cards + ADD COLUMN IF NOT EXISTS quantity INTEGER DEFAULT 0; + + ALTER TABLE cards + ADD COLUMN IF NOT EXISTS favorited BOOLEAN DEFAULT false; + `); +}; + +/** + * Down-migration intentionally throws. Dropping these columns on + * long-lived envs requires the `drop-dead-cards-columns` convoy's + * query-trace audit — bypassing it via a casual rollback risks + * dropping data on prod. Use `drop-dead-cards-columns` when ready. + * + * @returns {void} + */ +export const down = () => { + throw new Error( + '[migration:1781000000001_reconcile-cards-columns] Down not supported. ' + + 'Dropping cards.quantity / cards.favorited belongs to the queued ' + + 'drop-dead-cards-columns convoy, which performs a query-trace audit ' + + 'before the DROP. Do not rollback this migration directly.' + ); +}; +``` + +## Conventions to follow + +- `.cursor/rules/db-and-schema.mdc` § "Schema source of truth" — `migrations/` is the canonical home for schema changes; one column-group per migration. +- `.cursor/rules/no-go-zones.mdc` — `scripts/add-card-columns.js` is append-only history. **Do not edit it.** +- Style: match the existing migrations under `migrations/`. Use raw `pgm.sql(...)` template literals (the `migration-tool` convoy ratified this in D2 of `.convoys/reconcile-historical-add-scripts.md`). +- ESM exports (`export const up = ...`, `export const down = ...`); no `module.exports`. The repo is `"type": "module"` per `package.json` line 5. +- Use `IF NOT EXISTS` on every ALTER — idempotent re-apply is a documented requirement of this convoy. +- Add a JSDoc docstring at the top of the file explaining what's being reconciled, citing the source script + the convoy file. + +## Acceptance criteria + +- [ ] `migrations/1781000000001_reconcile-cards-columns.js` exists with the exact filename above (the timestamp `1781000000001` is the reservation token assigned by the architect — do NOT use `npm run migrate create`, which would call `Date.now()` and assign a different timestamp). +- [ ] The file's `up()` adds both columns via `IF NOT EXISTS`. +- [ ] The file's `down()` throws with a clear message pointing at the `drop-dead-cards-columns` follow-up. +- [ ] The file's docstring cites `scripts/add-card-columns.js` and `.convoys/reconcile-historical-add-scripts.md`. +- [ ] `node --check migrations/1781000000001_reconcile-cards-columns.js` passes (syntactic validity). +- [ ] `node -e "import('./migrations/1781000000001_reconcile-cards-columns.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))"` prints `function function undefined` (module loads cleanly). +- [ ] `npm run lint` exits clean against the baseline (no new lint errors introduced by this file). +- [ ] `npm run test:run` reports 21/21 passing (no test surface changes). +- [ ] The PR body documents that `add-updated-at-column.js` is already captured by `initial-schema` (per the source-script section above) and explains why no second migration is added. + +## Verification + +Run the following from the convoy worktree (`tcg-vault-worktrees/reconcile-historical-add-scripts/`) BEFORE opening the PR: + +```bash +node --check migrations/1781000000001_reconcile-cards-columns.js +node -e "import('./migrations/1781000000001_reconcile-cards-columns.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))" +npm run lint +npm run test:run +``` + +Expected: +- `node --check` exits 0 with no output. +- The `node -e` line prints exactly: `function function undefined`. +- `npm run lint` matches the existing baseline (no new errors). +- `npm run test:run` reports 21/21 tests passing. + +**Do NOT** run `npm run migrate up` against any environment as part of this brief — that's a post-merge operator step covered by Brief 7's verification runbook. + +## Commit message + +``` +feat(migrations): reconcile add-card-columns into migration history (brief 1/7) + +Captures the DDL effect of scripts/add-card-columns.js (cards.quantity ++ cards.favorited columns) into a new node-pg-migrate migration. Both +columns are flagged as unused in docs/SCHEMA_MAP.md § "Known schema +smells" #3 — added here for fresh-env parity with prod; a follow-up +drop-dead-cards-columns convoy will drop them after a query-trace +audit. + +Sibling script add-updated-at-column.js is already captured by +migrations/1779853647564_initial-schema.js (cards.updated_at is in the +CREATE TABLE); no second migration needed. + +Per .convoys/reconcile-historical-add-scripts.md § Brief outline → B1. +Idempotent re-apply (IF NOT EXISTS guards). +``` + +## PR shape + +**Title:** `feat(migrations): reconcile add-card-columns into migration history (brief 1/7)` + +**Body template:** + +```markdown +Brief 1 of the `reconcile-historical-add-scripts` convoy. See +[`.convoys/reconcile-historical-add-scripts.md`](../.convoys/reconcile-historical-add-scripts.md) +for the full plan and rationale. + +## What this PR does + +Adds `migrations/1781000000001_reconcile-cards-columns.js` — a new +`node-pg-migrate` migration that adds two columns to `cards` via +`ADD COLUMN IF NOT EXISTS`: + +- `quantity INTEGER DEFAULT 0` +- `favorited BOOLEAN DEFAULT false` + +Both columns already exist in long-lived prod environments (added by +the historical `scripts/add-card-columns.js`). This migration brings +fresh Neon branches to parity so `npm install` → `npm run setup-db` +alone produces the prod shape, without manually replaying historical +scripts. + +## What this PR does NOT do + +- Does **NOT** edit `scripts/add-card-columns.js` (no-go-zone). +- Does **NOT** add a migration for `scripts/add-updated-at-column.js` + — `cards.updated_at` is already declared in + `migrations/1779853647564_initial-schema.js` line 67. +- Does **NOT** edit `scripts/setup-neon-db.js`, `package.json`, README, + or AGENTS.md. +- Does **NOT** run `npm run migrate up` against any environment (that's + the post-merge operator step covered by Brief 7's runbook). +- Does **NOT** drop the columns (deferred to follow-up + `drop-dead-cards-columns`). + +## Verification checklist + +- [ ] `node --check migrations/1781000000001_reconcile-cards-columns.js` exits 0 +- [ ] `node -e "import('./migrations/1781000000001_reconcile-cards-columns.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))"` prints `function function undefined` +- [ ] `npm run lint` matches baseline (no new errors) +- [ ] `npm run test:run` reports 21/21 passing +- [ ] Did not run `npm run migrate up` against any environment in this PR +- [ ] Operator post-merge: run the verification runbook from Brief 7 (`docs/MIGRATION_VERIFICATION_RUNBOOK.md` once it lands) + +## Cross-references + +- Convoy file: `.convoys/reconcile-historical-add-scripts.md` +- Source script (no-go-zone, audit reference only): `scripts/add-card-columns.js` +- Follow-up after this convoy lands: `drop-dead-cards-columns` (P3 hygiene) +``` + +## DO NOT + +- DO NOT edit `scripts/add-card-columns.js` or any other file under `scripts/` — append-only no-go-zone per `.cursor/rules/no-go-zones.mdc`. +- DO NOT edit any other file under `migrations/` — each migration is pinned by its `pgmigrations` row. +- DO NOT edit `scripts/setup-neon-db.js`. +- DO NOT edit `package.json` (no new deps). +- DO NOT edit `AGENTS.md` or `docs/SCHEMA_MAP.md` — that's Brief 7's job. +- DO NOT run `npm run migrate up` against any environment. +- DO NOT call `npm run migrate create` to scaffold the file — it uses `Date.now()` for the timestamp prefix, which would collide with the architect's pre-assigned reservation tokens for parallel briefs. + +## Rationale (≤3 sentences) + +Capturing `cards.quantity` + `cards.favorited` in a single small migration matches the per-table grouping of the convoy's D1 decision and keeps the diff easy to review. Sibling `add-updated-at-column.js` is already captured by `initial-schema`, so reconciling it would create a `pgmigrations` row for zero functional benefit. The columns themselves are dead per SCHEMA_MAP smell #3, but parity with prod is the convoy's success metric — the actual DROP is the `drop-dead-cards-columns` follow-up's responsibility. diff --git a/.convoys/reconcile-historical-add-scripts/brief-2-reconcile-collections-columns.md b/.convoys/reconcile-historical-add-scripts/brief-2-reconcile-collections-columns.md new file mode 100644 index 0000000..d76aefb --- /dev/null +++ b/.convoys/reconcile-historical-add-scripts/brief-2-reconcile-collections-columns.md @@ -0,0 +1,307 @@ +--- +convoy: reconcile-historical-add-scripts +brief_number: 2 +depends_on: [] +files: + - migrations/1781000000002_reconcile-collections-columns.js +--- + +# Brief 2: Reconcile `collections` columns into migration history + +## Goal (1 sentence) + +Capture the `collections`-table DDL added by three historical scripts (`add-collaboration-features.js` columns half, `add-collection-slugs.js`, `add-image-column.js`) into a single new `node-pg-migrate` migration so a brand-new Neon branch ends up with `visibility`, `tcg`, `tags`, `slug` (+ index + CHECK constraint), and `image` columns on `collections` after `npm run setup-db`. + +## Scope (files in scope — do not edit anything else) + +- `migrations/1781000000002_reconcile-collections-columns.js` — **new** + +## Source scripts (read-only audit reference; DO NOT EDIT — all three are no-go-zones) + +### `scripts/add-collaboration-features.js` lines 15-20 (collections half only — the `collection_permissions` / `collection_activity` / `users.is_pending` half is Brief 3's scope): + +```js +await sql` + ALTER TABLE collections + ADD COLUMN IF NOT EXISTS visibility VARCHAR(20) DEFAULT 'private', + ADD COLUMN IF NOT EXISTS tcg VARCHAR(50) DEFAULT 'MTG', + ADD COLUMN IF NOT EXISTS tags TEXT +`; +``` + +Plus the visibility index at lines 77-80: + +```js +await sql` + CREATE INDEX IF NOT EXISTS idx_collections_visibility + ON collections(visibility) +`; +``` + +### `scripts/add-collection-slugs.js` lines 17-99 (relevant DDL only): + +```js +// Step 1: Add slug column +await sql` + ALTER TABLE collections + ADD COLUMN IF NOT EXISTS slug VARCHAR(100) UNIQUE +`; + +// Step 5: Add unique index +await sql`CREATE UNIQUE INDEX IF NOT EXISTS idx_collections_slug ON collections(slug)`; + +// Step 6: Add format CHECK constraint +await sql`ALTER TABLE collections ADD CONSTRAINT check_slug_format CHECK (slug ~ '^[a-z0-9]([a-z0-9-]*[a-z0-9])?$' AND length(slug) <= 50)`; +``` + +The script also backfills slugs via per-row UPDATE using `lib/slug-utils.js::generateUniqueSlug` (lines 41-73). **Do NOT fold the backfill DML into this migration.** Fresh envs have no pre-existing `collections` rows to backfill; on prod, the backfill ran once historically. Future migrations that need slug generation should run that logic in application code, not in a migration. + +### `scripts/add-image-column.js` lines 14-17 (entire DDL): + +```js +await sql` + ALTER TABLE collections + ADD COLUMN IF NOT EXISTS image TEXT +`; +``` + +## Target migration file + +Path: `migrations/1781000000002_reconcile-collections-columns.js` + +Contents: + +```js +/** + * Reconcile three historical scripts that all added columns to the + * `collections` table: + * + * - scripts/add-collaboration-features.js (visibility, tcg, tags + + * idx_collections_visibility index) — collections-table half only; + * the collection_permissions / collection_activity / users.is_pending + * half is reconciled by migrations/1781000000003_reconcile-collaboration-tables.js + * - scripts/add-collection-slugs.js (slug + idx_collections_slug + * unique index + check_slug_format CHECK constraint) + * - scripts/add-image-column.js (image) + * + * Grouped into one migration per D1 of .convoys/reconcile-historical-add-scripts.md + * (one migration per table/feature surface). All historical DDL was + * idempotent (ADD COLUMN IF NOT EXISTS / CREATE INDEX IF NOT EXISTS); + * this migration preserves that. CHECK constraint adds via a + * DO $$ EXCEPTION block because Postgres doesn't accept + * ADD CONSTRAINT ... IF NOT EXISTS for CHECK. + * + * Per-row slug backfill DML from add-collection-slugs.js is intentionally + * NOT folded in — fresh envs have no pre-existing collections to + * backfill; on prod, the backfill ran once historically and is baked in. + * + * Notes on adjacent state: + * - `is_system_collection` was added by migrations/1780378340194_system-collection-description.js + * (and the runtime register hook at pages/api/auth/register.js:97-119 + * creates the per-user system collection row — Finding 4 RESOLVED in + * .convoys/reconcile-historical-add-scripts.md). + * + * Two-column-visibility smell: `is_public BOOLEAN` (from initial-schema) + * and `visibility VARCHAR(20)` (from this migration) coexist on prod. + * Surfaced as docs/SCHEMA_MAP.md § "Known schema smells" #2 → queued + * `unify-collection-visibility` is OUT OF SCOPE here; this migration + * adds `visibility` for parity, nothing more. + * + * Idempotent re-apply: every statement uses IF NOT EXISTS or the + * exception-swallowing DO block. + * + * @type {import('node-pg-migrate').ColumnDefinitions | undefined} + */ +export const shorthands = undefined; + +/** + * @param {import('node-pg-migrate').MigrationBuilder} pgm + */ +export const up = (pgm) => { + pgm.sql(` + ALTER TABLE collections + ADD COLUMN IF NOT EXISTS visibility VARCHAR(20) DEFAULT 'private', + ADD COLUMN IF NOT EXISTS tcg VARCHAR(50) DEFAULT 'MTG', + ADD COLUMN IF NOT EXISTS tags TEXT, + ADD COLUMN IF NOT EXISTS slug VARCHAR(100) UNIQUE, + ADD COLUMN IF NOT EXISTS image TEXT; + + CREATE INDEX IF NOT EXISTS idx_collections_visibility + ON collections (visibility); + + CREATE UNIQUE INDEX IF NOT EXISTS idx_collections_slug + ON collections (slug); + + DO $$ + BEGIN + ALTER TABLE collections + ADD CONSTRAINT check_slug_format + CHECK (slug ~ '^[a-z0-9]([a-z0-9-]*[a-z0-9])?$' AND length(slug) <= 50); + EXCEPTION + WHEN duplicate_object THEN NULL; + END $$; + `); +}; + +/** + * Down-migration intentionally throws. Removing these columns on + * long-lived envs would drop user-curated tag / slug / image data + * and break the runtime code that reads collections.visibility, + * collections.slug, collections.image, collections.tcg, collections.tags. + * + * If a future schema correction needs to mutate any of these columns, + * write a NEW dated migration with a real `down()` — do NOT roll back + * this one. + * + * @returns {void} + */ +export const down = () => { + throw new Error( + '[migration:1781000000002_reconcile-collections-columns] Down not supported. ' + + 'Dropping collections.visibility / tcg / tags / slug / image would erase ' + + 'user-curated data and break runtime reads. Write a new dated migration ' + + 'for any future schema correction.' + ); +}; +``` + +## Conventions to follow + +- `.cursor/rules/db-and-schema.mdc` § "Schema source of truth" — one migration per table/feature surface. +- `.cursor/rules/no-go-zones.mdc` — all three source scripts are append-only history. **Do not edit any of them.** +- Style: raw `pgm.sql(...)` template literals, matching the other migrations under `migrations/`. Per D2 of the convoy file. +- ESM exports; `"type": "module"` per `package.json` line 5. +- `IF NOT EXISTS` on every ALTER and CREATE INDEX. CHECK constraint wraps in `DO $$ ... EXCEPTION WHEN duplicate_object THEN NULL END $$;` (Postgres doesn't accept `IF NOT EXISTS` directly on CHECK constraints). + +## Acceptance criteria + +- [ ] `migrations/1781000000002_reconcile-collections-columns.js` exists with the exact filename above (the timestamp `1781000000002` is the reservation token assigned by the architect — do NOT use `npm run migrate create`). +- [ ] The file's `up()` adds 5 columns (`visibility`, `tcg`, `tags`, `slug`, `image`) via `IF NOT EXISTS`, creates 2 indexes (`idx_collections_visibility`, unique `idx_collections_slug`) via `IF NOT EXISTS`, and adds the `check_slug_format` CHECK via a `DO $$ EXCEPTION` block. +- [ ] The file's `down()` throws with a clear message. +- [ ] The file's docstring cites all three source scripts + the convoy file + the visibility smell (#2 in SCHEMA_MAP). +- [ ] Per-row slug backfill DML is NOT in the migration (out of scope per the convoy plan). +- [ ] `node --check migrations/1781000000002_reconcile-collections-columns.js` passes. +- [ ] `node -e "import('./migrations/1781000000002_reconcile-collections-columns.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))"` prints `function function undefined`. +- [ ] `npm run lint` matches the baseline (no new errors). +- [ ] `npm run test:run` reports 21/21 passing. + +## Verification + +Run from the convoy worktree BEFORE opening the PR: + +```bash +node --check migrations/1781000000002_reconcile-collections-columns.js +node -e "import('./migrations/1781000000002_reconcile-collections-columns.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))" +npm run lint +npm run test:run +``` + +Expected: +- `node --check` exits 0 silently. +- `node -e` prints `function function undefined`. +- `npm run lint` matches baseline. +- `npm run test:run` reports 21/21 passing. + +**Do NOT** run `npm run migrate up` against any environment — that's the post-merge operator step covered by Brief 7's verification runbook. + +## Commit message + +``` +feat(migrations): reconcile collections columns into migration history (brief 2/7) + +Captures the DDL effect of three historical scripts into one new +node-pg-migrate migration: + + - scripts/add-collaboration-features.js (collections half: visibility, + tcg, tags + idx_collections_visibility) + - scripts/add-collection-slugs.js (slug + idx_collections_slug unique + + check_slug_format CHECK constraint) + - scripts/add-image-column.js (image) + +The collaboration tables half (collection_permissions, collection_activity, +users.is_pending) is reconciled separately by Brief 3. + +Per-row slug backfill DML from add-collection-slugs.js is intentionally +NOT folded in — fresh envs have no pre-existing collections to backfill. + +Per .convoys/reconcile-historical-add-scripts.md § Brief outline → B2. +Idempotent re-apply (IF NOT EXISTS guards; DO $$ EXCEPTION for CHECK). +``` + +## PR shape + +**Title:** `feat(migrations): reconcile collections columns into migration history (brief 2/7)` + +**Body template:** + +```markdown +Brief 2 of the `reconcile-historical-add-scripts` convoy. See +[`.convoys/reconcile-historical-add-scripts.md`](../.convoys/reconcile-historical-add-scripts.md) +for the full plan and rationale. + +## What this PR does + +Adds `migrations/1781000000002_reconcile-collections-columns.js` — +captures the DDL effect of three historical scripts in a single new +`node-pg-migrate` migration: + +- `scripts/add-collaboration-features.js` — collections columns half + (`visibility VARCHAR(20)`, `tcg VARCHAR(50)`, `tags TEXT`, + `idx_collections_visibility` index) +- `scripts/add-collection-slugs.js` — `slug VARCHAR(100) UNIQUE`, + `idx_collections_slug` unique index, `check_slug_format` CHECK + constraint +- `scripts/add-image-column.js` — `image TEXT` + +All ALTERs use `ADD COLUMN IF NOT EXISTS`. The CHECK constraint wraps +in a `DO $$ ... EXCEPTION WHEN duplicate_object THEN NULL END $$;` +block because Postgres doesn't accept `ADD CONSTRAINT ... IF NOT EXISTS` +for CHECK. + +## What this PR does NOT do + +- Does **NOT** edit any of the three source scripts (no-go-zones). +- Does **NOT** capture the collaboration tables half of + `add-collaboration-features.js` (`collection_permissions`, + `collection_activity`, `users.is_pending`, related indexes, + owner-permission backfill DML) — that's **Brief 3**. +- Does **NOT** fold in the per-row slug backfill DML from + `add-collection-slugs.js` — fresh envs have no pre-existing + collections to backfill. +- Does **NOT** unify the `is_public` / `visibility` redundancy + (SCHEMA_MAP smell #2) — that's a future `unify-collection-visibility` + follow-up. +- Does **NOT** edit `scripts/setup-neon-db.js`, `package.json`, README, + or AGENTS.md. +- Does **NOT** run `npm run migrate up` against any environment. + +## Verification checklist + +- [ ] `node --check migrations/1781000000002_reconcile-collections-columns.js` exits 0 +- [ ] `node -e "import('./migrations/1781000000002_reconcile-collections-columns.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))"` prints `function function undefined` +- [ ] `npm run lint` matches baseline +- [ ] `npm run test:run` reports 21/21 passing +- [ ] Did not run `npm run migrate up` against any environment in this PR + +## Cross-references + +- Convoy file: `.convoys/reconcile-historical-add-scripts.md` +- Source scripts (no-go-zones, audit reference only): + `scripts/add-collaboration-features.js`, + `scripts/add-collection-slugs.js`, + `scripts/add-image-column.js` +``` + +## DO NOT + +- DO NOT edit `scripts/add-collaboration-features.js`, `scripts/add-collection-slugs.js`, `scripts/add-image-column.js`, or any other file under `scripts/` — append-only no-go-zone. +- DO NOT edit any other file under `migrations/`. +- DO NOT edit `scripts/setup-neon-db.js`, `package.json`, README, `AGENTS.md`, or `docs/SCHEMA_MAP.md` (B7 owns SCHEMA_MAP). +- DO NOT run `npm run migrate up` against any environment. +- DO NOT include the per-row slug backfill DML — out of scope. +- DO NOT add work for `collection_permissions` / `collection_activity` / `users.is_pending` — that's Brief 3. +- DO NOT call `npm run migrate create` — it would generate a `Date.now()` timestamp colliding with B1/B3/B4/B5's reservation tokens. + +## Rationale (≤3 sentences) + +Grouping all `collections`-table DDL into one migration matches the per-table grouping of D1 and keeps the diff focused on a single surface. Splitting `add-collaboration-features.js` between this brief (columns) and Brief 3 (tables) avoids creating an artificial dependency between two parallel briefs — each writes only its own new file. The CHECK-constraint exception block matches the historical script's try/catch pattern and is the idiomatic Postgres way to do idempotent CHECK adds. diff --git a/.convoys/reconcile-historical-add-scripts/brief-3-reconcile-collaboration-tables.md b/.convoys/reconcile-historical-add-scripts/brief-3-reconcile-collaboration-tables.md new file mode 100644 index 0000000..7324a2a --- /dev/null +++ b/.convoys/reconcile-historical-add-scripts/brief-3-reconcile-collaboration-tables.md @@ -0,0 +1,318 @@ +--- +convoy: reconcile-historical-add-scripts +brief_number: 3 +depends_on: [] +files: + - migrations/1781000000003_reconcile-collaboration-tables.js +--- + +# Brief 3: Reconcile collaboration tables into migration history + +## Goal (1 sentence) + +Capture the `collection_permissions` + `collection_activity` table creation, the `users.is_pending` column, and the 3 related indexes from `scripts/add-collaboration-features.js` into a single new `node-pg-migrate` migration so a brand-new Neon branch ends up with the collaboration / sharing surface after `npm run setup-db`. + +## Scope (files in scope — do not edit anything else) + +- `migrations/1781000000003_reconcile-collaboration-tables.js` — **new** + +## Source script (read-only audit reference; DO NOT EDIT — no-go-zone) + +`scripts/add-collaboration-features.js` lines 24-81 (relevant DDL only; the collections-columns half is reconciled by Brief 2): + +```js +// collection_permissions table +await sql` + CREATE TABLE IF NOT EXISTS collection_permissions ( + id SERIAL PRIMARY KEY, + collection_id INTEGER REFERENCES collections(id) ON DELETE CASCADE, + user_id INTEGER REFERENCES users(id) ON DELETE CASCADE, + role VARCHAR(20) NOT NULL CHECK (role IN ('owner', 'editor', 'viewer')), + status VARCHAR(20) DEFAULT 'active' CHECK (status IN ('active', 'pending', 'declined')), + invite_token VARCHAR(255) UNIQUE, + invited_by INTEGER REFERENCES users(id), + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + UNIQUE(collection_id, user_id) + ) +`; + +// collection_activity table +await sql` + CREATE TABLE IF NOT EXISTS collection_activity ( + id SERIAL PRIMARY KEY, + collection_id INTEGER REFERENCES collections(id) ON DELETE CASCADE, + user_id INTEGER REFERENCES users(id) ON DELETE SET NULL, + action VARCHAR(50) NOT NULL, + details JSONB, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP + ) +`; + +// users.is_pending column +await sql` + ALTER TABLE users + ADD COLUMN IF NOT EXISTS is_pending BOOLEAN DEFAULT false +`; + +// 3 indexes (the 4th — idx_collections_visibility — is Brief 2's scope) +await sql` + CREATE INDEX IF NOT EXISTS idx_collection_permissions_collection_id + ON collection_permissions(collection_id) +`; +await sql` + CREATE INDEX IF NOT EXISTS idx_collection_permissions_user_id + ON collection_permissions(user_id) +`; +await sql` + CREATE INDEX IF NOT EXISTS idx_collection_activity_collection_id + ON collection_activity(collection_id) +`; +``` + +**Owner-permission backfill DML** at lines 84-99 inserts `('owner', 'active')` rows for every pre-existing collection. **Do NOT fold the backfill DML into this migration.** Fresh envs have no pre-existing collections needing backfill; on prod, the backfill ran once historically and is baked in. The current runtime invariant for new collection creation lives in `pages/api/collections.js` (verify post-merge if needed — out of scope for this brief). + +## Target migration file + +Path: `migrations/1781000000003_reconcile-collaboration-tables.js` + +Contents: + +```js +/** + * Reconcile the collaboration / sharing half of + * scripts/add-collaboration-features.js into the migration history: + * + * - CREATE TABLE collection_permissions (with role/status CHECK + * constraints inline + invite_token UNIQUE + UNIQUE(collection_id, + * user_id)) + * - CREATE TABLE collection_activity (with JSONB details column) + * - ALTER users ADD COLUMN is_pending BOOLEAN DEFAULT false + * - 3 indexes (idx_collection_permissions_collection_id, + * idx_collection_permissions_user_id, + * idx_collection_activity_collection_id) + * + * The 4th index from the source script (idx_collections_visibility) + * is reconciled by migrations/1781000000002_reconcile-collections-columns.js + * because it indexes a column added in that brief. + * + * The collections-columns half (visibility, tcg, tags) of + * add-collaboration-features.js is reconciled by + * migrations/1781000000002_reconcile-collections-columns.js. + * + * The owner-permission backfill DML from the source script (INSERT + * INTO collection_permissions ... 'owner', 'active' for every + * pre-existing collection) is intentionally NOT folded in — fresh + * envs have no pre-existing collections to backfill; on prod, the + * backfill ran once historically and is baked in. The runtime + * invariant for owner-permission creation on new collections is the + * responsibility of pages/api/collections.js (out of scope here). + * + * Both CREATE TABLE statements use IF NOT EXISTS, with CHECK + * constraints declared inline (no idempotency issue — IF NOT EXISTS + * on the parent table makes the whole CREATE a no-op when the table + * already exists, CHECK constraints and all). + * + * Idempotent re-apply: every statement uses IF NOT EXISTS. + * + * @type {import('node-pg-migrate').ColumnDefinitions | undefined} + */ +export const shorthands = undefined; + +/** + * @param {import('node-pg-migrate').MigrationBuilder} pgm + */ +export const up = (pgm) => { + pgm.sql(` + CREATE TABLE IF NOT EXISTS collection_permissions ( + id SERIAL PRIMARY KEY, + collection_id INTEGER REFERENCES collections(id) ON DELETE CASCADE, + user_id INTEGER REFERENCES users(id) ON DELETE CASCADE, + role VARCHAR(20) NOT NULL CHECK (role IN ('owner', 'editor', 'viewer')), + status VARCHAR(20) DEFAULT 'active' CHECK (status IN ('active', 'pending', 'declined')), + invite_token VARCHAR(255) UNIQUE, + invited_by INTEGER REFERENCES users(id), + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + UNIQUE(collection_id, user_id) + ); + + CREATE TABLE IF NOT EXISTS collection_activity ( + id SERIAL PRIMARY KEY, + collection_id INTEGER REFERENCES collections(id) ON DELETE CASCADE, + user_id INTEGER REFERENCES users(id) ON DELETE SET NULL, + action VARCHAR(50) NOT NULL, + details JSONB, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP + ); + + ALTER TABLE users + ADD COLUMN IF NOT EXISTS is_pending BOOLEAN DEFAULT false; + + CREATE INDEX IF NOT EXISTS idx_collection_permissions_collection_id + ON collection_permissions (collection_id); + + CREATE INDEX IF NOT EXISTS idx_collection_permissions_user_id + ON collection_permissions (user_id); + + CREATE INDEX IF NOT EXISTS idx_collection_activity_collection_id + ON collection_activity (collection_id); + `); +}; + +/** + * Down-migration intentionally throws. Dropping collection_permissions + * + collection_activity on a long-lived env would erase every active + * sharing relationship + every audit trail row. The runtime in + * pages/api/collections/[identifier]/permissions.js, pages/api/invite/*.js, + * and lib/permission-middleware.js all read these tables; rolling back + * would break the live app. + * + * @returns {void} + */ +export const down = () => { + throw new Error( + '[migration:1781000000003_reconcile-collaboration-tables] Down not supported. ' + + 'Dropping collection_permissions / collection_activity would erase every ' + + 'sharing relationship and audit trail, and break runtime reads in ' + + 'pages/api/collections/[identifier]/permissions.js, pages/api/invite/*.js, ' + + 'lib/permission-middleware.js. Write a new dated migration for any future ' + + 'schema correction.' + ); +}; +``` + +## Conventions to follow + +- `.cursor/rules/db-and-schema.mdc` § "Schema source of truth" — `migrations/` is canonical; one migration per feature surface. +- `.cursor/rules/no-go-zones.mdc` — `scripts/add-collaboration-features.js` is append-only history. **Do not edit it.** +- Style: raw `pgm.sql(...)` template literals matching the other migrations. Per D2. +- ESM exports; `"type": "module"`. +- `IF NOT EXISTS` on every CREATE / ALTER. Inline CHECK constraints on `CREATE TABLE` are fine — `CREATE TABLE IF NOT EXISTS` skips the entire statement (constraints and all) when the table exists. +- FK declarations match the source script verbatim (`ON DELETE CASCADE` for primary FKs, `ON DELETE SET NULL` where the source uses it). + +## Acceptance criteria + +- [ ] `migrations/1781000000003_reconcile-collaboration-tables.js` exists with the exact filename above. +- [ ] The file's `up()`: + - Creates `collection_permissions` table with all 10 columns + role/status CHECKs + invite_token UNIQUE + UNIQUE(collection_id, user_id), all via `CREATE TABLE IF NOT EXISTS`. + - Creates `collection_activity` table with 6 columns including JSONB `details`, via `CREATE TABLE IF NOT EXISTS`. + - Adds `users.is_pending BOOLEAN DEFAULT false` via `ADD COLUMN IF NOT EXISTS`. + - Creates 3 indexes via `CREATE INDEX IF NOT EXISTS`. +- [ ] The 4th index from the source script (`idx_collections_visibility`) is NOT in this migration — it belongs to Brief 2. +- [ ] The owner-permission backfill DML is NOT in this migration. +- [ ] The file's `down()` throws with a clear message. +- [ ] The file's docstring cites the source script + the convoy file + the Brief 2 split. +- [ ] `node --check migrations/1781000000003_reconcile-collaboration-tables.js` passes. +- [ ] `node -e "import('./migrations/1781000000003_reconcile-collaboration-tables.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))"` prints `function function undefined`. +- [ ] `npm run lint` matches baseline. +- [ ] `npm run test:run` reports 21/21 passing. + +## Verification + +```bash +node --check migrations/1781000000003_reconcile-collaboration-tables.js +node -e "import('./migrations/1781000000003_reconcile-collaboration-tables.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))" +npm run lint +npm run test:run +``` + +Expected: `node --check` silent, `node -e` prints `function function undefined`, lint baseline, 21/21 tests pass. + +**Do NOT** run `npm run migrate up` against any environment. + +## Commit message + +``` +feat(migrations): reconcile collaboration tables into migration history (brief 3/7) + +Captures the collaboration / sharing half of +scripts/add-collaboration-features.js into one new node-pg-migrate +migration: + + - CREATE TABLE collection_permissions (role/status CHECKs, + invite_token UNIQUE, UNIQUE(collection_id, user_id)) + - CREATE TABLE collection_activity (JSONB details) + - ALTER users ADD COLUMN is_pending BOOLEAN DEFAULT false + - 3 indexes + +The collections-columns half (visibility, tcg, tags + +idx_collections_visibility) is reconciled by Brief 2. The 4th index +(idx_collections_visibility) belongs to Brief 2 because it indexes a +column added there. + +Per-row owner-permission backfill DML from the source script is +intentionally NOT folded in — fresh envs have no pre-existing +collections to backfill; the runtime invariant for new-collection +owner-perm creation lives in pages/api/collections.js. + +Per .convoys/reconcile-historical-add-scripts.md § Brief outline → B3. +Idempotent re-apply (IF NOT EXISTS guards). +``` + +## PR shape + +**Title:** `feat(migrations): reconcile collaboration tables into migration history (brief 3/7)` + +**Body template:** + +```markdown +Brief 3 of the `reconcile-historical-add-scripts` convoy. See +[`.convoys/reconcile-historical-add-scripts.md`](../.convoys/reconcile-historical-add-scripts.md) +for the full plan and rationale. + +## What this PR does + +Adds `migrations/1781000000003_reconcile-collaboration-tables.js` — +captures the collaboration / sharing DDL of +`scripts/add-collaboration-features.js` (the table-and-column half; +the columns-on-collections half is Brief 2). + +- `CREATE TABLE IF NOT EXISTS collection_permissions` (10 columns + including inline role/status CHECK constraints + invite_token + UNIQUE + UNIQUE(collection_id, user_id)) +- `CREATE TABLE IF NOT EXISTS collection_activity` (6 columns + including JSONB `details`) +- `ALTER TABLE users ADD COLUMN IF NOT EXISTS is_pending BOOLEAN DEFAULT false` +- 3 indexes via `CREATE INDEX IF NOT EXISTS` + +## What this PR does NOT do + +- Does **NOT** edit `scripts/add-collaboration-features.js` (no-go-zone). +- Does **NOT** capture the columns-on-collections half (`visibility`, + `tcg`, `tags`, `idx_collections_visibility`) — that's **Brief 2**. +- Does **NOT** fold in the per-row owner-permission backfill DML from + the source script — fresh envs have no pre-existing collections to + backfill. +- Does **NOT** edit `scripts/setup-neon-db.js`, `package.json`, README, + `AGENTS.md`, or `docs/SCHEMA_MAP.md` (B7 owns SCHEMA_MAP). +- Does **NOT** run `npm run migrate up` against any environment. + +## Verification checklist + +- [ ] `node --check migrations/1781000000003_reconcile-collaboration-tables.js` exits 0 +- [ ] `node -e "import('./migrations/1781000000003_reconcile-collaboration-tables.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))"` prints `function function undefined` +- [ ] `npm run lint` matches baseline +- [ ] `npm run test:run` reports 21/21 passing +- [ ] Did not run `npm run migrate up` against any environment in this PR + +## Cross-references + +- Convoy file: `.convoys/reconcile-historical-add-scripts.md` +- Source script (no-go-zone, audit reference only): + `scripts/add-collaboration-features.js` +``` + +## DO NOT + +- DO NOT edit `scripts/add-collaboration-features.js` or any file under `scripts/`. +- DO NOT edit any other file under `migrations/`. +- DO NOT edit `scripts/setup-neon-db.js`, `package.json`, README, `AGENTS.md`, or `docs/SCHEMA_MAP.md`. +- DO NOT run `npm run migrate up` against any environment. +- DO NOT include the owner-permission backfill DML. +- DO NOT add work for the collections-columns half (`visibility`, `tcg`, `tags`, `idx_collections_visibility`) — that's Brief 2. +- DO NOT call `npm run migrate create`. + +## Rationale (≤3 sentences) + +Splitting `add-collaboration-features.js` between Brief 2 (collections columns + the visibility index that indexes one of those columns) and Brief 3 (collaboration tables + the users.is_pending column + the 3 indexes that index collaboration-table columns) keeps each migration scoped to the table surface it touches, matching D1. Inline CHECK constraints on `CREATE TABLE` are idempotent for free under `CREATE TABLE IF NOT EXISTS` (the whole statement no-ops when the table exists). The backfill DML is intentionally out of scope because fresh envs need no backfill and prod's backfill already ran. diff --git a/.convoys/reconcile-historical-add-scripts/brief-4-reconcile-favorites-system.md b/.convoys/reconcile-historical-add-scripts/brief-4-reconcile-favorites-system.md new file mode 100644 index 0000000..776ccbb --- /dev/null +++ b/.convoys/reconcile-historical-add-scripts/brief-4-reconcile-favorites-system.md @@ -0,0 +1,241 @@ +--- +convoy: reconcile-historical-add-scripts +brief_number: 4 +depends_on: [] +files: + - migrations/1781000000004_reconcile-favorites-system.js +--- + +# Brief 4: Reconcile favorites system into migration history + +## Goal (1 sentence) + +Capture the `user_favorites` table and its 4 indexes from `scripts/add-favorites-system.js` into a single new `node-pg-migrate` migration so a brand-new Neon branch ends up with the favorites surface after `npm run setup-db`. + +## Scope (files in scope — do not edit anything else) + +- `migrations/1781000000004_reconcile-favorites-system.js` — **new** + +## Source script (read-only audit reference; DO NOT EDIT — no-go-zone) + +`scripts/add-favorites-system.js` lines 11-27 (entire DDL — script has no DML beyond the table create): + +```js +await sql` + CREATE TABLE IF NOT EXISTS user_favorites ( + id SERIAL PRIMARY KEY, + user_id INTEGER REFERENCES users(id) ON DELETE CASCADE, + item_type VARCHAR(50) NOT NULL, -- 'card', 'collection', 'deck' + item_id INTEGER NOT NULL, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + UNIQUE(user_id, item_type, item_id) + ) +`; + +// 4 indexes +await sql`CREATE INDEX IF NOT EXISTS idx_user_favorites_user_id ON user_favorites(user_id)`; +await sql`CREATE INDEX IF NOT EXISTS idx_user_favorites_item_type ON user_favorites(item_type)`; +await sql`CREATE INDEX IF NOT EXISTS idx_user_favorites_item_id ON user_favorites(item_id)`; +await sql`CREATE INDEX IF NOT EXISTS idx_user_favorites_user_type ON user_favorites(user_id, item_type)`; +``` + +**Note on SCHEMA_MAP drift.** `docs/SCHEMA_MAP.md` § `user_favorites` (lines 104-111) shows the table with only `(user_id, card_id)` columns — that's a **doc bug**. The actual source script (and prod schema) uses the polymorphic `(item_type, item_id)` shape that supports cards, collections, AND decks (per the script's inline comment + the runtime usage in `pages/api/favorites.js`). B7 corrects the SCHEMA_MAP entry. This brief faithfully reproduces the **script's** shape — `(item_type, item_id)` — not the doc's. + +## Target migration file + +Path: `migrations/1781000000004_reconcile-favorites-system.js` + +Contents: + +```js +/** + * Reconcile scripts/add-favorites-system.js into the migration history. + * + * Creates user_favorites with the polymorphic (item_type, item_id) + * shape that supports favoriting cards, collections, and decks via a + * single table. Adds 4 indexes for the common query shapes: + * + * - idx_user_favorites_user_id — "all favorites for user X" + * - idx_user_favorites_item_type — "all card favorites" / "all deck favorites" + * - idx_user_favorites_item_id — back-link from an item to its favoriters + * - idx_user_favorites_user_type — composite for "user X's card favorites" + * + * Note: docs/SCHEMA_MAP.md § user_favorites currently shows only + * (user_id, card_id) — that's a doc bug. The actual prod shape (and + * the source script, and the runtime in pages/api/favorites.js) uses + * the polymorphic shape. B7 of this convoy corrects the SCHEMA_MAP + * entry; this migration reproduces the script's shape faithfully. + * + * Idempotent re-apply: CREATE TABLE IF NOT EXISTS + CREATE INDEX IF NOT EXISTS. + * + * @type {import('node-pg-migrate').ColumnDefinitions | undefined} + */ +export const shorthands = undefined; + +/** + * @param {import('node-pg-migrate').MigrationBuilder} pgm + */ +export const up = (pgm) => { + pgm.sql(` + CREATE TABLE IF NOT EXISTS user_favorites ( + id SERIAL PRIMARY KEY, + user_id INTEGER REFERENCES users(id) ON DELETE CASCADE, + item_type VARCHAR(50) NOT NULL, + item_id INTEGER NOT NULL, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + UNIQUE(user_id, item_type, item_id) + ); + + CREATE INDEX IF NOT EXISTS idx_user_favorites_user_id + ON user_favorites (user_id); + + CREATE INDEX IF NOT EXISTS idx_user_favorites_item_type + ON user_favorites (item_type); + + CREATE INDEX IF NOT EXISTS idx_user_favorites_item_id + ON user_favorites (item_id); + + CREATE INDEX IF NOT EXISTS idx_user_favorites_user_type + ON user_favorites (user_id, item_type); + `); +}; + +/** + * Down-migration intentionally throws. Dropping user_favorites on a + * long-lived env would erase every user's saved favorites list and + * break runtime reads in pages/api/favorites.js. + * + * @returns {void} + */ +export const down = () => { + throw new Error( + '[migration:1781000000004_reconcile-favorites-system] Down not supported. ' + + 'Dropping user_favorites would erase every saved favorite and break ' + + 'pages/api/favorites.js. Write a new dated migration for any future ' + + 'schema correction.' + ); +}; +``` + +## Conventions to follow + +- `.cursor/rules/db-and-schema.mdc` § "Schema source of truth" — `migrations/` is canonical. +- `.cursor/rules/no-go-zones.mdc` — `scripts/add-favorites-system.js` is append-only history. +- Style: raw `pgm.sql(...)` template literals. Per D2. +- ESM exports; `"type": "module"`. +- `IF NOT EXISTS` on every CREATE. +- Match the source script's column types, FK ON DELETE rule (`CASCADE`), and UNIQUE shape verbatim. + +## Acceptance criteria + +- [ ] `migrations/1781000000004_reconcile-favorites-system.js` exists with the exact filename above. +- [ ] The file's `up()` creates `user_favorites` with all 5 columns + UNIQUE constraint, then 4 indexes, all via `IF NOT EXISTS`. +- [ ] The column shape is `(item_type VARCHAR(50), item_id INTEGER)` — the polymorphic shape — NOT `(card_id)`. See "Note on SCHEMA_MAP drift" above. +- [ ] The file's `down()` throws with a clear message. +- [ ] The file's docstring cites the source script + the convoy file + the SCHEMA_MAP doc-bug note. +- [ ] `node --check migrations/1781000000004_reconcile-favorites-system.js` passes. +- [ ] `node -e "import('./migrations/1781000000004_reconcile-favorites-system.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))"` prints `function function undefined`. +- [ ] `npm run lint` matches baseline. +- [ ] `npm run test:run` reports 21/21 passing. + +## Verification + +```bash +node --check migrations/1781000000004_reconcile-favorites-system.js +node -e "import('./migrations/1781000000004_reconcile-favorites-system.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))" +npm run lint +npm run test:run +``` + +Expected: `node --check` silent, `node -e` prints `function function undefined`, lint baseline, 21/21 tests pass. + +**Do NOT** run `npm run migrate up` against any environment. + +## Commit message + +``` +feat(migrations): reconcile favorites system into migration history (brief 4/7) + +Captures scripts/add-favorites-system.js into one new node-pg-migrate +migration: + + - CREATE TABLE user_favorites (polymorphic (item_type, item_id) + shape; UNIQUE(user_id, item_type, item_id)) + - 4 indexes (user_id, item_type, item_id, (user_id, item_type)) + +Note: docs/SCHEMA_MAP.md § user_favorites currently shows the table +with only (user_id, card_id) — that's a doc bug; B7 of this convoy +corrects the entry. This migration faithfully reproduces the source +script's polymorphic shape (also what runtime in pages/api/favorites.js +uses). + +Per .convoys/reconcile-historical-add-scripts.md § Brief outline → B4. +Idempotent re-apply (IF NOT EXISTS guards). +``` + +## PR shape + +**Title:** `feat(migrations): reconcile favorites system into migration history (brief 4/7)` + +**Body template:** + +```markdown +Brief 4 of the `reconcile-historical-add-scripts` convoy. See +[`.convoys/reconcile-historical-add-scripts.md`](../.convoys/reconcile-historical-add-scripts.md) +for the full plan and rationale. + +## What this PR does + +Adds `migrations/1781000000004_reconcile-favorites-system.js` — +captures `scripts/add-favorites-system.js` into a single new +`node-pg-migrate` migration. + +- `CREATE TABLE IF NOT EXISTS user_favorites` with polymorphic + `(item_type VARCHAR(50), item_id INTEGER)` shape supporting + cards / collections / decks favorites in one table. +- `UNIQUE(user_id, item_type, item_id)` prevents duplicate favorites. +- 4 supporting indexes via `CREATE INDEX IF NOT EXISTS`. + +## Note on a SCHEMA_MAP doc bug + +`docs/SCHEMA_MAP.md` § `user_favorites` (current `main`) shows the +table with only `(user_id, card_id)` columns. That's a doc bug — the +actual prod shape (and the source script, and `pages/api/favorites.js` +runtime usage) uses the polymorphic `(item_type, item_id)` shape. This +migration faithfully reproduces the script's shape; Brief 7 of this +convoy fixes the SCHEMA_MAP entry. + +## What this PR does NOT do + +- Does **NOT** edit `scripts/add-favorites-system.js` (no-go-zone). +- Does **NOT** edit `docs/SCHEMA_MAP.md` (Brief 7's job). +- Does **NOT** edit `scripts/setup-neon-db.js`, `package.json`, README, + or `AGENTS.md`. +- Does **NOT** run `npm run migrate up` against any environment. + +## Verification checklist + +- [ ] `node --check migrations/1781000000004_reconcile-favorites-system.js` exits 0 +- [ ] `node -e "import('./migrations/1781000000004_reconcile-favorites-system.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))"` prints `function function undefined` +- [ ] `npm run lint` matches baseline +- [ ] `npm run test:run` reports 21/21 passing +- [ ] Did not run `npm run migrate up` against any environment in this PR + +## Cross-references + +- Convoy file: `.convoys/reconcile-historical-add-scripts.md` +- Source script (no-go-zone, audit reference only): `scripts/add-favorites-system.js` +``` + +## DO NOT + +- DO NOT edit `scripts/add-favorites-system.js` or any file under `scripts/`. +- DO NOT edit any other file under `migrations/`. +- DO NOT edit `scripts/setup-neon-db.js`, `package.json`, README, `AGENTS.md`, or `docs/SCHEMA_MAP.md` (B7 owns SCHEMA_MAP). +- DO NOT run `npm run migrate up` against any environment. +- DO NOT use the SCHEMA_MAP `(user_id, card_id)` shape — that doc entry is wrong; reproduce the SOURCE SCRIPT'S polymorphic shape. +- DO NOT call `npm run migrate create`. + +## Rationale (≤3 sentences) + +The favorites system is a single self-contained table — natural fit for one small migration that maps 1:1 with the historical script. The polymorphic shape (`item_type`, `item_id`) is what runtime code actually uses, so reproducing it from the script's verbatim DDL — rather than the stale `(user_id, card_id)` doc entry — is the safe choice. Brief 7's SCHEMA_MAP rewrite resolves the doc-bug separately so this brief stays scoped to one new file. diff --git a/.convoys/reconcile-historical-add-scripts/brief-5-reconcile-user-profile.md b/.convoys/reconcile-historical-add-scripts/brief-5-reconcile-user-profile.md new file mode 100644 index 0000000..05fb09c --- /dev/null +++ b/.convoys/reconcile-historical-add-scripts/brief-5-reconcile-user-profile.md @@ -0,0 +1,425 @@ +--- +convoy: reconcile-historical-add-scripts +brief_number: 5 +depends_on: [] +files: + - migrations/1781000000005_reconcile-user-profile.js +--- + +# Brief 5: Reconcile user profile fields into migration history + +## Goal (1 sentence) + +Capture the deduplicated union of `scripts/add-user-profile-columns.js` and `scripts/add-user-profile-fields.js` (15 new `users` columns, 2 new tables `user_settings` + `user_avatars`, 6 CHECK constraints, 6 indexes) into a single new `node-pg-migrate` migration so a brand-new Neon branch ends up with the user-profile surface after `npm run setup-db`. + +## Scope (files in scope — do not edit anything else) + +- `migrations/1781000000005_reconcile-user-profile.js` — **new** + +## Source scripts (read-only audit reference; DO NOT EDIT — both are no-go-zones) + +### `scripts/add-user-profile-columns.js` lines 12-18 (the earlier, narrower script): + +```js +await sql` + ALTER TABLE users + ADD COLUMN IF NOT EXISTS first_name VARCHAR(255), + ADD COLUMN IF NOT EXISTS last_name VARCHAR(255), + ADD COLUMN IF NOT EXISTS username VARCHAR(255) UNIQUE, + ADD COLUMN IF NOT EXISTS profile_image_url TEXT +`; +``` + +Plus per-row UPDATE backfill of defaults (lines 25-42). **Do NOT fold the backfill DML into this migration** — fresh envs have no rows to backfill; column DEFAULTs handle new rows. + +### `scripts/add-user-profile-fields.js` lines 27-115 (the later, broader script — superset of #8 plus additional columns + 2 new tables + 6 CHECK constraints + 6 indexes): + +```js +// Basic profile fields (overlaps add-user-profile-columns.js for first_name/last_name/username, +// but ADDS bio + avatar_url; idempotent overlap because of IF NOT EXISTS) +await sql` + ALTER TABLE users + ADD COLUMN IF NOT EXISTS first_name VARCHAR(255), + ADD COLUMN IF NOT EXISTS last_name VARCHAR(255), + ADD COLUMN IF NOT EXISTS username VARCHAR(255) UNIQUE, + ADD COLUMN IF NOT EXISTS bio TEXT, + ADD COLUMN IF NOT EXISTS avatar_url TEXT +`; + +// Preference fields +await sql` + ALTER TABLE users + ADD COLUMN IF NOT EXISTS favorite_games JSONB DEFAULT '["MTG"]', + ADD COLUMN IF NOT EXISTS collection_visibility VARCHAR(20) DEFAULT 'private', + ADD COLUMN IF NOT EXISTS preferred_currency VARCHAR(3) DEFAULT 'USD', + ADD COLUMN IF NOT EXISTS cards_per_page INTEGER DEFAULT 50, + ADD COLUMN IF NOT EXISTS default_view VARCHAR(10) DEFAULT 'grid' +`; + +// Notification + 2FA settings +await sql` + ALTER TABLE users + ADD COLUMN IF NOT EXISTS notifications_email BOOLEAN DEFAULT true, + ADD COLUMN IF NOT EXISTS notifications_marketing BOOLEAN DEFAULT false, + ADD COLUMN IF NOT EXISTS two_factor_enabled BOOLEAN DEFAULT false +`; + +// Display settings +await sql` + ALTER TABLE users + ADD COLUMN IF NOT EXISTS theme VARCHAR(10) DEFAULT 'system', + ADD COLUMN IF NOT EXISTS language VARCHAR(5) DEFAULT 'en' +`; + +// user_settings table +await sql` + CREATE TABLE IF NOT EXISTS user_settings ( + id SERIAL PRIMARY KEY, + user_id INTEGER REFERENCES users(id) ON DELETE CASCADE, + setting_key VARCHAR(100) NOT NULL, + setting_value JSONB NOT NULL, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + UNIQUE(user_id, setting_key) + ) +`; + +// user_avatars table +await sql` + CREATE TABLE IF NOT EXISTS user_avatars ( + id SERIAL PRIMARY KEY, + user_id INTEGER REFERENCES users(id) ON DELETE CASCADE, + filename VARCHAR(255) NOT NULL, + original_name VARCHAR(255), + mime_type VARCHAR(100), + file_size INTEGER, + file_path TEXT NOT NULL, + is_active BOOLEAN DEFAULT true, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP + ) +`; + +// 6 indexes +await sql`CREATE INDEX IF NOT EXISTS idx_users_username ON users(username)`; +await sql`CREATE INDEX IF NOT EXISTS idx_users_email ON users(email)`; +await sql`CREATE INDEX IF NOT EXISTS idx_user_settings_user_id ON user_settings(user_id)`; +await sql`CREATE INDEX IF NOT EXISTS idx_user_settings_key ON user_settings(setting_key)`; +await sql`CREATE INDEX IF NOT EXISTS idx_user_avatars_user_id ON user_avatars(user_id)`; +await sql`CREATE INDEX IF NOT EXISTS idx_user_avatars_active ON user_avatars(user_id, is_active)`; + +// 6 CHECK constraints (each wrapped in JS try/catch to swallow 'already exists') +await sql`ALTER TABLE users ADD CONSTRAINT check_collection_visibility CHECK (collection_visibility IN ('private', 'public', 'unlisted'))`; +await sql`ALTER TABLE users ADD CONSTRAINT check_preferred_currency CHECK (preferred_currency IN ('USD', 'EUR', 'GBP', 'CAD', 'JPY'))`; +await sql`ALTER TABLE users ADD CONSTRAINT check_cards_per_page CHECK (cards_per_page IN (25, 50, 100))`; +await sql`ALTER TABLE users ADD CONSTRAINT check_default_view CHECK (default_view IN ('grid', 'list'))`; +await sql`ALTER TABLE users ADD CONSTRAINT check_theme CHECK (theme IN ('light', 'dark', 'system'))`; +await sql`ALTER TABLE users ADD CONSTRAINT check_language CHECK (language IN ('en', 'es', 'fr', 'de', 'ja'))`; +``` + +Plus a per-row UPDATE backfill of defaults (lines 198-222). **Do NOT fold the backfill DML in** — column DEFAULTs handle new rows; fresh envs have no rows to backfill. + +### Important note on the column dedup + +Scripts #8 and #9 overlap on `first_name`, `last_name`, `username` — both use `ADD COLUMN IF NOT EXISTS` so on prod the second-running script no-ops those three columns. Both scripts have run on prod, so the union of their columns is what's actually present: + +- From #8 only: `profile_image_url` (TEXT) — a column that ONLY #8 added. +- From #9 only: `bio`, `avatar_url`, `favorite_games`, `collection_visibility`, `preferred_currency`, `cards_per_page`, `default_view`, `notifications_email`, `notifications_marketing`, `two_factor_enabled`, `theme`, `language` (12 columns) + the 2 new tables + 6 CHECK constraints + 6 indexes. +- From both (idempotent overlap): `first_name`, `last_name`, `username`. + +The migration must add **all 15 columns** (4 from #8 ∪ 12 from #9 with 3 in the intersection = 4 + 12 - 3 = 13 unique users columns; wait, let me recount: #8 adds 4 (first_name, last_name, username, profile_image_url); #9 adds 5 basic (first_name, last_name, username, bio, avatar_url) + 5 prefs + 3 notif + 2 display = 15. Union: first_name, last_name, username (shared) + profile_image_url (#8) + bio, avatar_url, favorite_games, collection_visibility, preferred_currency, cards_per_page, default_view, notifications_email, notifications_marketing, two_factor_enabled, theme, language (#9) = **3 + 1 + 12 = 16 columns**). So the migration adds 16 columns to `users`. + +The redundant `profile_image_url` vs `avatar_url` pair is documented in `docs/SCHEMA_MAP.md` § "Known schema smells" #1 and surfaced as the `unify-user-avatar-column` follow-up. Both must be in fresh envs for parity. + +## Target migration file + +Path: `migrations/1781000000005_reconcile-user-profile.js` + +Contents: + +```js +/** + * Reconcile two overlapping historical user-profile scripts into the + * migration history, taking the union of their effects: + * + * - scripts/add-user-profile-columns.js (the earlier, narrower + * script): first_name, last_name, username UNIQUE, profile_image_url + * + * - scripts/add-user-profile-fields.js (the later, broader script; + * overlaps the earlier script on first_name / last_name / username + * and additionally adds): bio, avatar_url, favorite_games (JSONB + * DEFAULT '["MTG"]'), collection_visibility, preferred_currency, + * cards_per_page, default_view, notifications_email, + * notifications_marketing, two_factor_enabled, theme, language, + * + the new user_settings + user_avatars tables, + 6 CHECK + * constraints, + 6 indexes. + * + * Result on a fresh env: 16 new columns on `users`, 2 new tables, + * 6 CHECK constraints, 6 indexes. On any long-lived env: every + * statement is a no-op (IF NOT EXISTS / DO $$ EXCEPTION). + * + * Per-row UPDATE backfills from both scripts are intentionally NOT + * folded in — column DEFAULTs handle new rows; fresh envs have no + * rows to backfill. + * + * The profile_image_url / avatar_url redundancy is intentional for + * parity with prod and is flagged in docs/SCHEMA_MAP.md § "Known + * schema smells" #1; future cleanup is the queued + * `unify-user-avatar-column` follow-up. + * + * CHECK constraint adds wrap in DO $$ ... EXCEPTION WHEN + * duplicate_object THEN NULL END $$ because Postgres doesn't accept + * ADD CONSTRAINT ... IF NOT EXISTS for CHECK. Each constraint gets + * its own DO block so a failure in one doesn't block the rest. + * + * @type {import('node-pg-migrate').ColumnDefinitions | undefined} + */ +export const shorthands = undefined; + +/** + * @param {import('node-pg-migrate').MigrationBuilder} pgm + */ +export const up = (pgm) => { + pgm.sql(` + -- 16 columns on users (union of add-user-profile-columns.js + add-user-profile-fields.js) + ALTER TABLE users + ADD COLUMN IF NOT EXISTS first_name VARCHAR(255), + ADD COLUMN IF NOT EXISTS last_name VARCHAR(255), + ADD COLUMN IF NOT EXISTS username VARCHAR(255) UNIQUE, + ADD COLUMN IF NOT EXISTS profile_image_url TEXT, + ADD COLUMN IF NOT EXISTS bio TEXT, + ADD COLUMN IF NOT EXISTS avatar_url TEXT, + ADD COLUMN IF NOT EXISTS favorite_games JSONB DEFAULT '["MTG"]', + ADD COLUMN IF NOT EXISTS collection_visibility VARCHAR(20) DEFAULT 'private', + ADD COLUMN IF NOT EXISTS preferred_currency VARCHAR(3) DEFAULT 'USD', + ADD COLUMN IF NOT EXISTS cards_per_page INTEGER DEFAULT 50, + ADD COLUMN IF NOT EXISTS default_view VARCHAR(10) DEFAULT 'grid', + ADD COLUMN IF NOT EXISTS notifications_email BOOLEAN DEFAULT true, + ADD COLUMN IF NOT EXISTS notifications_marketing BOOLEAN DEFAULT false, + ADD COLUMN IF NOT EXISTS two_factor_enabled BOOLEAN DEFAULT false, + ADD COLUMN IF NOT EXISTS theme VARCHAR(10) DEFAULT 'system', + ADD COLUMN IF NOT EXISTS language VARCHAR(5) DEFAULT 'en'; + + -- user_settings table + CREATE TABLE IF NOT EXISTS user_settings ( + id SERIAL PRIMARY KEY, + user_id INTEGER REFERENCES users(id) ON DELETE CASCADE, + setting_key VARCHAR(100) NOT NULL, + setting_value JSONB NOT NULL, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + UNIQUE(user_id, setting_key) + ); + + -- user_avatars table + CREATE TABLE IF NOT EXISTS user_avatars ( + id SERIAL PRIMARY KEY, + user_id INTEGER REFERENCES users(id) ON DELETE CASCADE, + filename VARCHAR(255) NOT NULL, + original_name VARCHAR(255), + mime_type VARCHAR(100), + file_size INTEGER, + file_path TEXT NOT NULL, + is_active BOOLEAN DEFAULT true, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP + ); + + -- 6 indexes + CREATE INDEX IF NOT EXISTS idx_users_username ON users (username); + CREATE INDEX IF NOT EXISTS idx_users_email ON users (email); + CREATE INDEX IF NOT EXISTS idx_user_settings_user_id ON user_settings (user_id); + CREATE INDEX IF NOT EXISTS idx_user_settings_key ON user_settings (setting_key); + CREATE INDEX IF NOT EXISTS idx_user_avatars_user_id ON user_avatars (user_id); + CREATE INDEX IF NOT EXISTS idx_user_avatars_active ON user_avatars (user_id, is_active); + + -- 6 CHECK constraints (each in its own DO block so one failure doesn't block the rest) + DO $$ BEGIN + ALTER TABLE users ADD CONSTRAINT check_collection_visibility + CHECK (collection_visibility IN ('private', 'public', 'unlisted')); + EXCEPTION WHEN duplicate_object THEN NULL; END $$; + + DO $$ BEGIN + ALTER TABLE users ADD CONSTRAINT check_preferred_currency + CHECK (preferred_currency IN ('USD', 'EUR', 'GBP', 'CAD', 'JPY')); + EXCEPTION WHEN duplicate_object THEN NULL; END $$; + + DO $$ BEGIN + ALTER TABLE users ADD CONSTRAINT check_cards_per_page + CHECK (cards_per_page IN (25, 50, 100)); + EXCEPTION WHEN duplicate_object THEN NULL; END $$; + + DO $$ BEGIN + ALTER TABLE users ADD CONSTRAINT check_default_view + CHECK (default_view IN ('grid', 'list')); + EXCEPTION WHEN duplicate_object THEN NULL; END $$; + + DO $$ BEGIN + ALTER TABLE users ADD CONSTRAINT check_theme + CHECK (theme IN ('light', 'dark', 'system')); + EXCEPTION WHEN duplicate_object THEN NULL; END $$; + + DO $$ BEGIN + ALTER TABLE users ADD CONSTRAINT check_language + CHECK (language IN ('en', 'es', 'fr', 'de', 'ja')); + EXCEPTION WHEN duplicate_object THEN NULL; END $$; + `); +}; + +/** + * Down-migration intentionally throws. Dropping 16 user-profile columns + * + user_settings + user_avatars on a long-lived env would erase every + * user's profile data, preferences, avatar history, and settings, and + * break runtime reads in pages/api/user/{settings,profile,avatar}.js. + * + * @returns {void} + */ +export const down = () => { + throw new Error( + '[migration:1781000000005_reconcile-user-profile] Down not supported. ' + + 'Dropping these columns + tables would erase every user profile, preference, ' + + 'avatar history, and settings row, and break runtime reads in ' + + 'pages/api/user/{settings,profile,avatar}.js. Write a new dated migration ' + + 'for any future schema correction.' + ); +}; +``` + +## Conventions to follow + +- `.cursor/rules/db-and-schema.mdc` § "Schema source of truth" — `migrations/` is canonical. +- `.cursor/rules/no-go-zones.mdc` — both source scripts are append-only history. +- Style: raw `pgm.sql(...)` template literals. Per D2. +- ESM exports; `"type": "module"`. +- `IF NOT EXISTS` on every ALTER + CREATE INDEX + CREATE TABLE. +- CHECK constraints in `DO $$ ... EXCEPTION WHEN duplicate_object THEN NULL END $$;` blocks, one block per constraint. +- Match the source scripts' column types, defaults, FK rules, and CHECK values verbatim. + +## Acceptance criteria + +- [ ] `migrations/1781000000005_reconcile-user-profile.js` exists with the exact filename above. +- [ ] The file's `up()` adds exactly **16 columns** to `users` (per the dedup count in the source-scripts note above), creates **2 tables** (`user_settings`, `user_avatars`), creates **6 indexes**, and adds **6 CHECK constraints** each in its own `DO $$ EXCEPTION` block. +- [ ] BOTH `profile_image_url` AND `avatar_url` are present (parity with prod; redundancy is documented). +- [ ] The file's `down()` throws with a clear message. +- [ ] The file's docstring cites both source scripts + the convoy file + the `unify-user-avatar-column` follow-up. +- [ ] Per-row UPDATE backfill DML is NOT in the migration. +- [ ] `node --check migrations/1781000000005_reconcile-user-profile.js` passes. +- [ ] `node -e "import('./migrations/1781000000005_reconcile-user-profile.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))"` prints `function function undefined`. +- [ ] `npm run lint` matches baseline. +- [ ] `npm run test:run` reports 21/21 passing. + +## Verification + +```bash +node --check migrations/1781000000005_reconcile-user-profile.js +node -e "import('./migrations/1781000000005_reconcile-user-profile.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))" +npm run lint +npm run test:run +``` + +Expected: `node --check` silent, `node -e` prints `function function undefined`, lint baseline, 21/21 tests pass. + +**Do NOT** run `npm run migrate up` against any environment. + +## Commit message + +``` +feat(migrations): reconcile user profile fields into migration history (brief 5/7) + +Captures the deduplicated union of scripts/add-user-profile-columns.js +and scripts/add-user-profile-fields.js into one new node-pg-migrate +migration: + + - 16 columns on users (basic profile + preferences + notifications + display) + - 2 new tables (user_settings, user_avatars) + - 6 CHECK constraints (each in its own DO $$ EXCEPTION block) + - 6 indexes + +Both profile_image_url (from script #1) and avatar_url (from script #2) +are added for parity with prod. The redundancy is flagged in +docs/SCHEMA_MAP.md § "Known schema smells" #1 and queued for cleanup as +the `unify-user-avatar-column` follow-up. + +Per-row UPDATE backfill DML from both scripts is intentionally NOT +folded in — column DEFAULTs handle new rows; fresh envs have no rows +to backfill. + +Per .convoys/reconcile-historical-add-scripts.md § Brief outline → B5. +Idempotent re-apply (IF NOT EXISTS guards + per-CHECK DO blocks). +``` + +## PR shape + +**Title:** `feat(migrations): reconcile user profile fields into migration history (brief 5/7)` + +**Body template:** + +```markdown +Brief 5 of the `reconcile-historical-add-scripts` convoy. See +[`.convoys/reconcile-historical-add-scripts.md`](../.convoys/reconcile-historical-add-scripts.md) +for the full plan and rationale. + +## What this PR does + +Adds `migrations/1781000000005_reconcile-user-profile.js` — captures +the deduplicated union of `scripts/add-user-profile-columns.js` +(earlier, narrower) and `scripts/add-user-profile-fields.js` (later, +broader) into a single new `node-pg-migrate` migration. + +- 16 new columns on `users` (first_name, last_name, username UNIQUE, + profile_image_url, bio, avatar_url, favorite_games JSONB, …, theme, + language) — all `ADD COLUMN IF NOT EXISTS` +- 2 new tables: `user_settings`, `user_avatars` — both + `CREATE TABLE IF NOT EXISTS` +- 6 indexes — `CREATE INDEX IF NOT EXISTS` +- 6 CHECK constraints (each in its own + `DO $$ ... EXCEPTION WHEN duplicate_object THEN NULL END $$;` block) + +## Why both `profile_image_url` and `avatar_url`? + +Script #1 added `profile_image_url`, script #2 added `avatar_url` — +both exist on prod and both are in this migration for fresh-env +parity. The redundancy is flagged in +`docs/SCHEMA_MAP.md` § "Known schema smells" #1 and queued for cleanup +as the `unify-user-avatar-column` follow-up. + +## What this PR does NOT do + +- Does **NOT** edit either source script (no-go-zones). +- Does **NOT** fold in the per-row UPDATE backfill DML from either + source script — column DEFAULTs handle new rows. +- Does **NOT** unify `profile_image_url` / `avatar_url` — that's the + `unify-user-avatar-column` follow-up. +- Does **NOT** edit `scripts/setup-neon-db.js`, `package.json`, README, + `AGENTS.md`, or `docs/SCHEMA_MAP.md` (B7 owns SCHEMA_MAP). +- Does **NOT** run `npm run migrate up` against any environment. + +## Verification checklist + +- [ ] `node --check migrations/1781000000005_reconcile-user-profile.js` exits 0 +- [ ] `node -e "import('./migrations/1781000000005_reconcile-user-profile.js').then(m => console.log(typeof m.up, typeof m.down, typeof m.shorthands))"` prints `function function undefined` +- [ ] `npm run lint` matches baseline +- [ ] `npm run test:run` reports 21/21 passing +- [ ] Did not run `npm run migrate up` against any environment in this PR + +## Cross-references + +- Convoy file: `.convoys/reconcile-historical-add-scripts.md` +- Source scripts (no-go-zones, audit reference only): + `scripts/add-user-profile-columns.js`, `scripts/add-user-profile-fields.js` +``` + +## DO NOT + +- DO NOT edit either source script or any file under `scripts/`. +- DO NOT edit any other file under `migrations/`. +- DO NOT edit `scripts/setup-neon-db.js`, `package.json`, README, `AGENTS.md`, or `docs/SCHEMA_MAP.md`. +- DO NOT run `npm run migrate up` against any environment. +- DO NOT pick one of `profile_image_url` / `avatar_url` to omit — both must be present for parity. +- DO NOT collapse the 6 CHECK constraints into a single `DO $$ EXCEPTION` block — one block per constraint so one duplicate doesn't swallow the others. +- DO NOT include the per-row UPDATE backfill DML. +- DO NOT call `npm run migrate create`. + +## Rationale (≤3 sentences) + +This is the largest single migration in the convoy because both historical scripts are tightly coupled to the `users` table surface and splitting them would create artificial boundaries (e.g., separating "users columns" from "CHECK constraints on users columns" makes no sense). Per-CHECK `DO $$ EXCEPTION` blocks mirror the historical scripts' per-statement try/catch pattern and ensure one duplicate-constraint failure doesn't block the rest. Keeping both avatar-style columns matches prod-as-is and explicitly defers the cleanup to a scoped follow-up convoy. diff --git a/.convoys/reconcile-historical-add-scripts/brief-7-documentation-and-verification.md b/.convoys/reconcile-historical-add-scripts/brief-7-documentation-and-verification.md new file mode 100644 index 0000000..3366403 --- /dev/null +++ b/.convoys/reconcile-historical-add-scripts/brief-7-documentation-and-verification.md @@ -0,0 +1,457 @@ +--- +convoy: reconcile-historical-add-scripts +brief_number: 7 +depends_on: [1, 2, 3, 4, 5] +files: + - docs/SCHEMA_MAP.md + - docs/MIGRATION_VERIFICATION_RUNBOOK.md + - AGENTS.md + - .convoys/ship-readiness.md +--- + +# Brief 7: Documentation + verification runbook + +## Goal (1 sentence) + +Update the four documentation surfaces that describe the post-convoy schema and onboarding state — `docs/SCHEMA_MAP.md` (correct doc bugs + cross-reference new migrations), `docs/MIGRATION_VERIFICATION_RUNBOOK.md` (new — manual operator runbook from D5), `AGENTS.md` Gotcha #6 (cross-reference this convoy as the closing follow-up), `.convoys/ship-readiness.md` (flip this convoy's queued entry to RESOLVED + unblock `retire-graveyard-scripts-after-audit` + add new follow-ups) — so the next operator onboarding a fresh Neon branch can do so by `npm install` → `npm run setup-db` alone and verify the result. + +## Scope (files in scope — do not edit anything else) + +- `docs/SCHEMA_MAP.md` — **modified** +- `docs/MIGRATION_VERIFICATION_RUNBOOK.md` — **new** +- `AGENTS.md` — **modified** (small Gotcha #6 cross-reference update only) +- `.convoys/ship-readiness.md` — **modified** (queued convoys section) + +This brief is **sequenced last** because it cross-references the 5 new migration files that B1-B5 add; it cannot land before B1-B5 are merged. The architect's `slice_dependencies` block in the convoy file declares `depends_on: [1, 2, 3, 4, 5]`. + +## Per-file scope + +### 1. `docs/SCHEMA_MAP.md` — modified + +Targeted edits (keep all other content as-is): + +1. **Preamble update.** The current preamble (lines 1-18) notes that *"the initial backfill captures only the post-`setup-neon-db.js` shape. … A follow-up convoy (`reconcile-historical-add-scripts`) will fold the historical effects into the migration history; until then this file remains the curated reference for the full prod shape."* Replace with a sentence noting the convoy has **landed**, the migration history now captures the full prod shape, and the operator verification runbook lives at `docs/MIGRATION_VERIFICATION_RUNBOOK.md`. Bump the "Last reviewed" date. + +2. **`### users` notes column updates** (lines 43-57): each row references `add-user-profile-columns.js` / `add-user-profile-fields.js` — leave those references in place (they're historical context); ADD a single line at the end of the table noting *"All columns above are now captured by `migrations/1781000000005_reconcile-user-profile.js` (B5 of `reconcile-historical-add-scripts`, 2026-06-14)."* + +3. **`### cards` notes column updates** (lines 78-79): for the `quantity` and `favorited` rows, change the "Unused; consider dropping" annotation to *"Unused; captured by `migrations/1781000000001_reconcile-cards-columns.js` for fresh-env parity. Drop tracked as queued `drop-dead-cards-columns` follow-up."* + +4. **`### user_cards` index/constraint notes** (lines 90-102): add a brief note clarifying the canonical constraint is `UNIQUE(user_id, card_id, is_foil)` (3-col) from initial-schema, and that `scripts/fix-user-cards-constraints.js`'s stricter 2-col variant was either never-applied or reverted (Finding 1 of the reconcile convoy → Outcome A, 2026-06-14). This is the SCHEMA_MAP equivalent of the Drift Finding 1 resolution. + +5. **`### user_favorites` table** (lines 104-111): **FIX THE DOC BUG.** Current shape lists only `(user_id, card_id)`. Replace with the actual polymorphic shape per the runtime in `pages/api/favorites.js`: + + ```markdown + ### user_favorites + + | Column | Type | Notes | + | --- | --- | --- | + | `id` | `SERIAL PK` | | + | `user_id` | `INTEGER FK users(id) ON DELETE CASCADE` | | + | `item_type` | `VARCHAR(50) NOT NULL` | `'card' | 'collection' | 'deck'` — polymorphic | + | `item_id` | `INTEGER NOT NULL` | FK depends on `item_type`; not enforced at DB level | + | `created_at` | `TIMESTAMP` | | + | | | **UNIQUE(user_id, item_type, item_id)** | + + Indexes: `idx_user_favorites_user_id`, `idx_user_favorites_item_type`, + `idx_user_favorites_item_id`, `idx_user_favorites_user_type + (user_id, item_type)` — all in `migrations/1781000000004_reconcile-favorites-system.js`. + ``` + +6. **`### collections` notes**: add a line at the bottom noting *"`visibility`, `tcg`, `tags`, `slug` (+ `idx_collections_slug` unique + `check_slug_format` CHECK), `image` are now captured by `migrations/1781000000002_reconcile-collections-columns.js`."* + +7. **`### collection_permissions` + `### collection_activity` sections**: add a line at the bottom of each noting *"Captured by `migrations/1781000000003_reconcile-collaboration-tables.js`."* + +8. **NEW SECTION: `### user_settings`** (currently a one-liner at line 217). Expand to a proper column table matching the actual schema: + + ```markdown + ### user_settings + + Per-user key/value store for settings that don't warrant a column on `users`. + Captured by `migrations/1781000000005_reconcile-user-profile.js`. + + | Column | Type | Notes | + | --- | --- | --- | + | `id` | `SERIAL PK` | | + | `user_id` | `INTEGER FK users(id) ON DELETE CASCADE` | | + | `setting_key` | `VARCHAR(100) NOT NULL` | | + | `setting_value` | `JSONB NOT NULL` | | + | `created_at`, `updated_at` | `TIMESTAMP` default now | | + | | | **UNIQUE(user_id, setting_key)** | + + Indexes: `idx_user_settings_user_id`, `idx_user_settings_key`. + ``` + +9. **`### user_avatars` expansion** (currently one paragraph at lines 222-223). Replace with: + + ```markdown + ### user_avatars + + Tracks uploaded avatar history. Captured by + `migrations/1781000000005_reconcile-user-profile.js`. Older avatars + are typically deleted from blob storage; verify the cleanup job runs. + + | Column | Type | Notes | + | --- | --- | --- | + | `id` | `SERIAL PK` | | + | `user_id` | `INTEGER FK users(id) ON DELETE CASCADE` | | + | `filename` | `VARCHAR(255) NOT NULL` | | + | `original_name` | `VARCHAR(255)` | | + | `mime_type` | `VARCHAR(100)` | | + | `file_size` | `INTEGER` | | + | `file_path` | `TEXT NOT NULL` | | + | `is_active` | `BOOLEAN DEFAULT true` | | + | `created_at`, `updated_at` | `TIMESTAMP` default now | | + + Indexes: `idx_user_avatars_user_id`, `idx_user_avatars_active (user_id, is_active)`. + ``` + +10. **`## Known schema smells` section updates** (lines 226-232): + + - **Smell #2** (`is_public` vs `visibility`): add a sentence noting both columns are now captured by separate migrations (initial-schema for `is_public`, B2 for `visibility`); resolution lives in a future `unify-collection-visibility` follow-up. + - **Smell #3** (`cards.quantity` + `cards.favorited`): add the cross-reference to the queued `drop-dead-cards-columns` follow-up. + - **NEW: Smell #7 — `users.profile_image_url` vs `users.avatar_url`** (the parity smell that B5 perpetuates intentionally). Both columns are present for prod parity; cleanup is the queued `unify-user-avatar-column` follow-up. + +11. **`## Regeneration` section update** (lines 234-245): the manual-regeneration instructions can be removed entirely since the migration history is now authoritative. Replace with: + + ```markdown + ## Regeneration + + The migration history under `migrations/` is the authoritative source + of truth. To verify this file matches a live env (prod, preview, or a + fresh Neon branch), use the operator runbook at + [`docs/MIGRATION_VERIFICATION_RUNBOOK.md`](MIGRATION_VERIFICATION_RUNBOOK.md). + + When you add a new migration, update the relevant table section here + in the same PR. New tables get a new `###` section with the same + column-table shape. + ``` + +### 2. `docs/MIGRATION_VERIFICATION_RUNBOOK.md` — new + +Lift verbatim from `.convoys/reconcile-historical-add-scripts.md` § Verification plan (D5), with a small intro framing it as the canonical operator runbook (not just a convoy artifact). Suggested skeleton: + +```markdown +# Migration verification runbook + +How to verify that the migration history under `migrations/` produces +the same schema as a long-lived environment (prod, preview, or +similar). Use this runbook: + +- **After this repo's `reconcile-historical-add-scripts` convoy** (the + initial reconciliation), to confirm a fresh Neon branch reaches + parity with prod via `npm install` → `npm run setup-db` alone. +- **After any new migration lands on `main`**, to spot-check that + applying the migration to prod (via the operator's + `POSTGRES_URL= npm run migrate up`) produced the intended + effect. +- **When suspecting drift** between an env's actual schema and the + migration history (rare; the migration history is authoritative). + +This runbook is the manual precursor to the automated check planned +in the queued `wire-migrate-into-ci` follow-up (see +`.convoys/ship-readiness.md` § "Queued convoys"). + +## Prerequisites + +- `pg_dump` (PostgreSQL 16+) installed locally. +- `POSTGRES_URL` for the env you're verifying. +- `ADMIN_INITIAL_PASSWORD` (for the fresh-branch onboarding step) — + see `AGENTS.md` § 5 "Running locally". +- A Neon account with permission to create a branch (or any other + way to spin up a fresh Postgres DB on the same major version as + prod). + +## Procedure + +### Step 1 — Snapshot the reference env's structural shape + +Run against the env you consider canonical (usually prod): + +\`\`\`bash +POSTGRES_URL= pg_dump --schema-only --no-owner --no-acl \ + --schema=public > /tmp/reference-schema.sql +\`\`\` + +### Step 2 — Create a fresh DB and onboard via `setup-db` + +Create a clean Neon branch from an **empty** parent (or any other +fresh Postgres DB on the same major version): + +\`\`\`bash +POSTGRES_URL= \ +ADMIN_INITIAL_PASSWORD=$(openssl rand -base64 24) \ + npm run setup-db +\`\`\` + +`setup-db` runs `npm run migrate up` (applying every migration in +`migrations/` in timestamp order) and seeds the admin user. + +### Step 3 — Snapshot the fresh DB's structural shape + +\`\`\`bash +POSTGRES_URL= pg_dump --schema-only --no-owner --no-acl \ + --schema=public > /tmp/fresh-schema.sql +\`\`\` + +### Step 4 — Diff + +\`\`\`bash +diff <(sort /tmp/reference-schema.sql) <(sort /tmp/fresh-schema.sql) +\`\`\` + +**Expected non-material differences** (acceptable; do not chase): + +- Constraint or index NAME differences. Prod constraints created via + the historical `scripts/add-*` / `scripts/fix-*` jobs may have + autogenerated tuple-UNIQUE names that differ from the migrations' + explicit names. +- Column-ORDER differences. Prod has columns in historical-script-ALTER + order; fresh envs have them in migration-order. + +**Material differences** (bug — fix before declaring verified): + +- A column type, default, or NULL/NOT NULL state that differs. +- A missing or extra table. +- A missing or extra CHECK constraint that changes accepted values. +- A missing or extra index that changes query plan shape. + +## Supplementary spot-checks + +For the highest-risk surfaces (frequently-edited tables), run these +information-schema queries against both envs and confirm the +column-count / constraint-count / index-count totals match exactly: + +\`\`\`sql +-- Every column on every table +SELECT table_name, column_name, data_type, is_nullable, column_default +FROM information_schema.columns +WHERE table_schema = 'public' +ORDER BY table_name, ordinal_position; + +-- Every constraint +SELECT table_name, constraint_name, constraint_type +FROM information_schema.table_constraints +WHERE table_schema = 'public' +ORDER BY table_name, constraint_name; + +-- Every index +SELECT tablename, indexname, indexdef +FROM pg_indexes +WHERE schemaname = 'public' +ORDER BY tablename, indexname; +\`\`\` + +A mismatch in column count, constraint count, or index count is a +material difference and indicates a bug. + +## What to do if you find a material difference + +1. **Identify which env is canonical.** Usually prod. If the diff + surfaces a missing column on prod that's in the migration history, + the migration was never applied to prod — run + `POSTGRES_URL= npm run migrate up` to catch up. +2. **If the diff surfaces a column on prod that's NOT in the + migration history**, you've found a drift bug. Write a new + reconciliation migration (under `migrations/`) that captures the + prod column, following the pattern in `.convoys/reconcile-historical-add-scripts/`. +3. **Do not edit existing migrations** — they're pinned by + `pgmigrations` rows. Always write a new dated migration to correct + schema. + +## Cross-references + +- `AGENTS.md` § 4 Gotcha #6 — migration tool adoption history. +- `.convoys/migration-tool.md` — the convoy that adopted `node-pg-migrate`. +- `.convoys/reconcile-historical-add-scripts.md` — the convoy that folded + the 13 historical scripts into the migration history. +- `.cursor/rules/db-and-schema.mdc` — schema-change conventions. +``` + +### 3. `AGENTS.md` — modified (small Gotcha #6 cross-reference) + +Gotcha #6 is already "RESOLVED" per `migration-tool` (2026-05-26). This brief adds **one sentence** to the end of Gotcha #6 cross-referencing this convoy as the closing follow-up. Verbatim addition (insert immediately before the `- **#7**` line at AGENTS.md line 120): + +> Post-`reconcile-historical-add-scripts` (2026-06-14), the +> migration history additionally captures the full effect of the 13 +> historical `scripts/add-*.js` / `scripts/fix-*.js` / +> `scripts/seed-*.js` jobs (where applicable — pure-DML seed scripts +> stay out per D4 of that convoy). A brand-new Neon branch can now be +> onboarded by `npm install` → `npm run setup-db` alone. Operator +> verification runbook at +> [`docs/MIGRATION_VERIFICATION_RUNBOOK.md`](docs/MIGRATION_VERIFICATION_RUNBOOK.md). +> The 13 historical scripts remain no-go-zones until the queued +> `retire-graveyard-scripts-after-audit` (P3) cleanup convoy lands; +> that convoy is now unblocked. + +Do NOT flip Gotcha #6's RESOLVED marker — it's already RESOLVED by the right convoy (`migration-tool`). Just append the cross-reference. + +### 4. `.convoys/ship-readiness.md` — modified (queued convoys section) + +Find the existing line under § "Queued convoys" that reads: + +> - **`reconcile-historical-add-scripts`** (priority: P1 quality — needed for fresh-env onboarding). Surfaced 2026-05-26 by `migration-tool` (PR #32). Fold the effects of the 27 historical `scripts/add-*.js` / `fix-*.js` / `seed-*.js` jobs … Documented in `.convoys/migration-tool.md` § R1. + +Replace with: + +> - **`reconcile-historical-add-scripts`** — **RESOLVED 2026-06-14** by [convoy](../.convoys/reconcile-historical-add-scripts.md) (PRs #XX-#XX). Captured 7 of the 13 historical scripts' effects into 5 new migrations under `migrations/` (B1-B5); 2 scripts were already captured by `initial-schema` + `1780378340194_system-collection-description`; 3 are DML-only and stay as dev fixtures; 1 (`fix-user-cards-constraints.js`) is a no-op on prod per Finding 1 (Outcome A — the canonical 3-col `UNIQUE(user_id, card_id, is_foil)` from `initial-schema` is what prod has, and `fix-user-cards-constraints.js`'s stricter 2-col variant is not present). A brand-new Neon branch now onboards via `npm install` → `npm run setup-db` alone. Operator verification at [`docs/MIGRATION_VERIFICATION_RUNBOOK.md`](../docs/MIGRATION_VERIFICATION_RUNBOOK.md). Entry kept (not deleted) for audit trail. + +Find the existing line: + +> - **`retire-graveyard-scripts-after-audit`** (priority: P3 polish; **blocked on `reconcile-historical-add-scripts`**). … + +Replace the `**blocked on `reconcile-historical-add-scripts`**` marker with `**UNBLOCKED 2026-06-14**` and leave the rest of the description intact. + +Add two new queued entries (driven by Findings 2 + 3 of the reconcile convoy): + +> - **`unify-user-avatar-column`** (priority: P3 hygiene). Surfaced 2026-06-14 by `reconcile-historical-add-scripts` Finding 2. `users.profile_image_url` (added by `scripts/add-user-profile-columns.js`) and `users.avatar_url` (added by `scripts/add-user-profile-fields.js`) coexist on prod and in the migration history (both columns are needed for parity per B5). Pick one canonical column, migrate the other's data to it, drop the loser, and sweep runtime readers in `pages/api/user/avatar*.js` + UI surfaces. Requires a query-trace audit first. +> - **`drop-dead-cards-columns`** (priority: P3 hygiene). Surfaced 2026-06-14 by `reconcile-historical-add-scripts` Finding 3. `cards.quantity INTEGER` and `cards.favorited BOOLEAN` are documented unused (per `docs/SCHEMA_MAP.md` § "Known schema smells" #3); reconciled into the migration history by B1 for parity, but the actual semantics live on `user_cards` / `user_favorites`. After a query-trace audit confirms zero runtime readers, ship a migration that DROPs both columns with a real `down()` that recreates them. + +(Do NOT add the withdrawn `add-system-collection-on-register` or `unify-user-cards-foil-tracking` entries — both were withdrawn during the reconcile convoy. See `.convoys/reconcile-historical-add-scripts.md` § Follow-ups.) + +## Conventions to follow + +- `.cursor/rules/no-go-zones.mdc` — no edits to `scripts/add-*` / `fix-*` / `seed-*`. +- `.cursor/rules/db-and-schema.mdc` — schema-change conventions; SCHEMA_MAP is updated alongside any migration. +- Markdown style: match the existing tone of `docs/SCHEMA_MAP.md` (compact tables, "Notes" column explains intent not type semantics) and `AGENTS.md` (numbered Gotchas, cross-reference convoy files). +- For `.convoys/ship-readiness.md`: match the existing entry style under § "Queued convoys" (one bullet per convoy, leading `- **`name`**`, RESOLVED entries get the "Entry kept (not deleted) for audit trail" closer when applicable). + +## Acceptance criteria + +- [ ] `docs/SCHEMA_MAP.md` preamble bumped (post-convoy state); "Last reviewed" date is 2026-06-14. +- [ ] `docs/SCHEMA_MAP.md` § `user_favorites` shape is the polymorphic `(item_type, item_id)` — NOT the previous `(card_id)` shape. +- [ ] Every reconciled table/column has a "captured by `migrations/`" cross-reference. +- [ ] `### user_settings` and `### user_avatars` sections are expanded from one-liners to full column tables matching B5's migration. +- [ ] New schema smell #7 (`profile_image_url` vs `avatar_url`) is added to § "Known schema smells". +- [ ] `docs/MIGRATION_VERIFICATION_RUNBOOK.md` exists with the 4-step procedure from D5 + the spot-check queries + the "material vs non-material differences" guidance. +- [ ] `AGENTS.md` Gotcha #6 gains a single appended paragraph cross-referencing this convoy + the new runbook + the unblocked `retire-graveyard-scripts-after-audit` follow-up. Gotcha #6's existing "RESOLVED by `migration-tool`" marker is NOT changed. +- [ ] `.convoys/ship-readiness.md` § "Queued convoys" → `reconcile-historical-add-scripts` entry is flipped to RESOLVED 2026-06-14 with a one-paragraph summary; `retire-graveyard-scripts-after-audit` is marked UNBLOCKED 2026-06-14; two new entries (`unify-user-avatar-column`, `drop-dead-cards-columns`) are added. +- [ ] No edits to any file under `scripts/`, `migrations/`, `pages/`, `lib/`, `components/`, `test/`, or `package.json`. +- [ ] `npm run lint` matches baseline (these are markdown-only edits; lint should be unaffected). +- [ ] `npm run test:run` reports 21/21 passing (these are markdown-only edits; no test surface). + +## Verification + +```bash +npm run lint +npm run test:run +``` + +Both must match baseline / be green. No additional verification for this brief — the migrations themselves are exercised by Brief 7's verification runbook AFTER an operator runs it manually post-merge. + +For the SCHEMA_MAP edits, eyeball the diff against the prior state and confirm every cross-reference to a migration file is accurate (the file actually exists, and the timestamp matches the assigned reservation). + +## Commit message + +``` +docs(reconcile): update SCHEMA_MAP, verification runbook, AGENTS, ship-readiness (brief 7/7) + +Closes the reconcile-historical-add-scripts convoy: + + - docs/SCHEMA_MAP.md: cross-reference 5 new migrations from B1-B5; + fix user_favorites doc bug (polymorphic shape, not (card_id)); + expand user_settings + user_avatars one-liners to full column tables; + add schema smell #7 (profile_image_url vs avatar_url redundancy). + - docs/MIGRATION_VERIFICATION_RUNBOOK.md (new): canonical operator + runbook for verifying fresh-env vs prod schema parity (lifted from + D5 of the convoy). + - AGENTS.md Gotcha #6: append cross-reference to this convoy + the + new runbook + the now-unblocked retire-graveyard-scripts-after-audit + follow-up. Existing "RESOLVED by migration-tool" marker unchanged. + - .convoys/ship-readiness.md: flip reconcile-historical-add-scripts + entry to RESOLVED 2026-06-14; mark retire-graveyard-scripts-after-audit + UNBLOCKED; add new queued unify-user-avatar-column (Finding 2) + + drop-dead-cards-columns (Finding 3). + +Per .convoys/reconcile-historical-add-scripts.md § Brief outline → B7. +Depends on B1-B5 having merged (this brief cross-references their files). +``` + +## PR shape + +**Title:** `docs(reconcile): update SCHEMA_MAP, verification runbook, AGENTS, ship-readiness (brief 7/7)` + +**Body template:** + +```markdown +Brief 7 (final) of the `reconcile-historical-add-scripts` convoy. +See [`.convoys/reconcile-historical-add-scripts.md`](../.convoys/reconcile-historical-add-scripts.md) +for the full plan and rationale. + +This is the closing brief — depends on B1-B5 having merged (PR +references below). + +## What this PR does + +- **`docs/SCHEMA_MAP.md`** — cross-reference all 5 new migrations from + B1-B5; fix the `user_favorites` doc bug (the table uses the + polymorphic `(item_type, item_id)` shape, not `(card_id)`); expand + `user_settings` + `user_avatars` from one-liners to full column + tables matching the B5 migration; add a new "Known schema smell" #7 + for the `profile_image_url` / `avatar_url` redundancy. +- **`docs/MIGRATION_VERIFICATION_RUNBOOK.md`** (new) — canonical + operator runbook for verifying fresh-env vs prod schema parity. + Pulls verbatim from D5 of the convoy file. Useful both for the + one-time reconciliation verification and for ongoing post-migration + spot-checks. +- **`AGENTS.md`** — append one paragraph to Gotcha #6 cross-referencing + this convoy as the closing follow-up. The "RESOLVED by `migration-tool`" + marker is unchanged (that's still the right resolution attribution + for the gotcha itself). +- **`.convoys/ship-readiness.md`** § "Queued convoys" — flip the + `reconcile-historical-add-scripts` entry to RESOLVED 2026-06-14; + flip `retire-graveyard-scripts-after-audit` from "blocked on + reconcile-historical-add-scripts" to UNBLOCKED 2026-06-14; add two + new queued entries (`unify-user-avatar-column` from Finding 2, + `drop-dead-cards-columns` from Finding 3). + +## Prerequisite PRs (all must be merged first) + +- B1 #XX — reconcile cards columns +- B2 #XX — reconcile collections columns +- B3 #XX — reconcile collaboration tables +- B4 #XX — reconcile favorites system +- B5 #XX — reconcile user profile + +## What this PR does NOT do + +- Does **NOT** edit any file under `migrations/`, `scripts/`, `pages/`, + `lib/`, `components/`, `test/`, or `package.json`. +- Does **NOT** run the verification runbook itself — that's the + operator's post-merge job; see the runbook's "Procedure" section. + +## Verification checklist + +- [ ] `npm run lint` matches baseline +- [ ] `npm run test:run` reports 21/21 passing +- [ ] Every "captured by `migrations/.js`" cross-reference in + SCHEMA_MAP points at a file that actually exists in `migrations/` + (eyeballed against `ls migrations/`) +- [ ] `AGENTS.md` Gotcha #6 still has its original RESOLVED marker + (we only APPEND to it, not rewrite it) +- [ ] `.convoys/ship-readiness.md` § "Queued convoys" has the flipped + `reconcile-historical-add-scripts` entry, the UNBLOCKED + `retire-graveyard-scripts-after-audit` marker, and the two new + follow-up entries +- [ ] Operator post-merge: run `docs/MIGRATION_VERIFICATION_RUNBOOK.md` + against prod + a fresh Neon branch and confirm the diff is + non-material + +## Cross-references + +- Convoy file: `.convoys/reconcile-historical-add-scripts.md` +- Per-brief files: `.convoys/reconcile-historical-add-scripts/brief-{1,2,3,4,5}-*.md` +``` + +## DO NOT + +- DO NOT edit any file under `scripts/`, `migrations/`, `pages/`, `lib/`, `components/`, `test/`. +- DO NOT edit `package.json`, README, `next.config.js`, or any rule under `.cursor/rules/`. +- DO NOT change `AGENTS.md` Gotcha #6's "RESOLVED by `migration-tool`" marker — append the new paragraph; don't rewrite the existing resolution attribution. +- DO NOT add a `B6` reference anywhere — that brief was removed (Finding 1 → Outcome A). +- DO NOT add `add-system-collection-on-register` or `unify-user-cards-foil-tracking` to the ship-readiness queued list — both were withdrawn during this convoy. +- DO NOT remove the existing `wire-migrate-into-ci` entry from ship-readiness — it's still queued and unrelated to this convoy. +- DO NOT run the verification runbook itself (that's the operator's post-merge step). + +## Rationale (≤3 sentences) + +Sequencing this brief last lets every documentation cross-reference point at a real file under `migrations/` rather than a placeholder. Bundling all four doc surfaces into one PR keeps the convoy's "as-shipped" record consistent (the migrations, the operator runbook, the AGENTS gotcha, and the ship-readiness ledger all flip together). The `docs/MIGRATION_VERIFICATION_RUNBOOK.md` extraction promotes a one-time convoy artifact into an evergreen operator tool that future migrations can reuse. diff --git a/.convoys/ship-readiness.md b/.convoys/ship-readiness.md index 9381e2b..341d3c1 100644 --- a/.convoys/ship-readiness.md +++ b/.convoys/ship-readiness.md @@ -47,6 +47,11 @@ PR #18; P1 #12 via `pick-a-name` PR #21) and **P1 #11.5 `fix-lint-baseline`** (PRs #61–#63, 2026-06-02), the P1 lane is complete. Lint is **0 problems**; CI `lint` is blocking (no `|| true` / `continue-on-error`). +**Post-hoc note (2026-08-15):** `reconcile-historical-add-scripts` implementer +briefs B1–B6 closed the fresh-env onboarding gap in June 2026 (PRs #148–#153); +Brief 7 (docs + verification runbook) landed two months later. The P1 lane was +already complete when B1–B6 merged — this closure is documentation-only. + ## P0 — ship-blockers (security) These MUST land before any anonymous traffic touches the production URL. @@ -472,10 +477,12 @@ Follow-ups surfaced mid-convoy or mid-PR that didn't fit the original launch seq - **`purge-quick-login-from-loginpage`** — **RESOLVED 2026-05-29** by PR #56 (`e0218e4`). Quick Login removed from `pages/login.js`. See `.convoys/purge-quick-login-from-loginpage.md` § As-shipped. - **`purge-neondatabase-serverless-fully`** — **RESOLVED 2026-06-02** by PR #57 (`5115683`). `@neondatabase/serverless` removed from `package.json`; operational scripts use `@vercel/postgres`. Historical `scripts/add-*` / `fix-*` / `seed-*` graveyard unchanged per no-go-zones. Entry kept for audit trail. - **`wire-migrate-into-ci`** (priority: P2 CI infra). Surfaced 2026-05-26 by `migration-tool` (PR #32) — D6 deferral. Add a CI job that runs `npm run migrate up` against a test DB (either a dedicated Neon branch + `MIGRATE_TEST_DATABASE_URL` secret with branch-reset logic, or a Postgres service container with a ~30s container-start tax). Catches syntactically-invalid migrations + most logical errors at PR time. Currently, the first signal that a new migration is broken is the developer's local `npm run migrate up` against their dev branch (or post-deploy on Vercel). Documented in `.convoys/migration-tool.md` § R3. -- **`reconcile-historical-add-scripts`** (priority: P1 quality — needed for fresh-env onboarding). Surfaced 2026-05-26 by `migration-tool` (PR #32). Fold the effects of the 27 historical `scripts/add-*.js` / `fix-*.js` / `seed-*.js` jobs into the migration history so a brand-new Neon branch can be onboarded by `npm install` → `npm run setup-db` alone (without manually replaying the historical scripts). Multi-PR; ideally one migration per logical change, generated by reading the scripts' SQL and re-shaping into idempotent `pgm.sql(...)` blocks (with `IF NOT EXISTS` / `IF EXISTS` guards so re-application is safe). Documented in `.convoys/migration-tool.md` § R1. -- **`retire-graveyard-scripts-after-audit`** (priority: P3 polish; **blocked on `reconcile-historical-add-scripts`**). Surfaced 2026-05-26 by `migration-tool` (PR #32). Once the migration history captures all historical effects, the legacy `scripts/add-*.js` / `fix-*.js` / `seed-*.js` files can be deleted (or moved to `scripts/historical/`). They remain no-go-zones until that cleanup convoy lands. Documented in `.convoys/migration-tool.md` § Follow-ups. +- **`reconcile-historical-add-scripts`** — **RESOLVED 2026-06-14** (B1–B6 via PRs #148–#153; B7 docs closure 2026-08-15). Six reconciliation migrations capture historical add-script DDL so fresh Neon branches onboard via `npm install` → `npm run setup-db` alone. Operator verification runbook at `docs/MIGRATION_VERIFICATION_RUNBOOK.md`. See `.convoys/reconcile-historical-add-scripts.md` § As-shipped. Entry kept for audit trail. +- **`retire-graveyard-scripts-after-audit`** (priority: P3 polish; **UNBLOCKED 2026-08-15** by `reconcile-historical-add-scripts` § As-shipped). Surfaced 2026-05-26 by `migration-tool` (PR #32). Once the migration history captures all historical effects, the legacy `scripts/add-*.js` / `fix-*.js` / `seed-*.js` files can be deleted (or moved to `scripts/historical/`). They remain no-go-zones until that cleanup convoy lands. Documented in `.convoys/migration-tool.md` § Follow-ups. - **`audit-node-pg-migrate-transitive-deps`** (priority: P3 hygiene). Surfaced 2026-05-26 by `migration-tool` (PR #32) — R5 in the convoy file. `npm audit` reports 11 vulnerabilities (6 moderate, 5 high) coming from `node-pg-migrate@8.0.4`'s `glob@~11.1.0` + `yargs@~17.7.0` transitive deps (older `brace-expansion`, `minimatch`, `picomatch` versions with known advisories). All in dev-only paths; the migration tool runs in scripts/CI, never in the deployed Next.js bundle, and the affected APIs (glob's shell-injection CLI; brace-expansion's ReDoS) are not exercised by node-pg-migrate's call sites. Surface only if a security audit specifically flags this surface, or if `node-pg-migrate` ships a v9 that updates the transitive tree. - **`add-migration-template`** (priority: P3 DX). Surfaced 2026-05-26 by `migration-tool` (PR #32). Add a custom template via `--template-file-name` so generated migrations include the project's preferred docstring shape + a reminder about `docs/SCHEMA_MAP.md` updates. Surface if migration authoring proves inconsistent. +- **`unify-user-avatar-column`** (priority: P3 polish). Surfaced 2026-06-14 by `reconcile-historical-add-scripts` architect Finding 2. `users` table has two avatar columns: `profile_image_url` (from `add-user-profile-columns.js`) and `avatar_url` (from `add-user-profile-fields.js`). Both are added by B5's migration for fresh-env parity; runtime may read either. Cleanup: audit which column each caller reads, pick one canonical column, migrate + drop the other. +- **`drop-dead-cards-columns`** (priority: P3 polish). Surfaced 2026-06-14 by `reconcile-historical-add-scripts` architect Finding 3. `cards.quantity` and `cards.favorited` are dead columns per `docs/SCHEMA_MAP.md` § Smells #3; still added by B1's migration for fresh-env parity. Cleanup: query-trace audit to confirm zero reads/writes, then a `DROP COLUMN` migration. ### Scanner audit portfolio (2026-05-27) diff --git a/docs/MIGRATION_VERIFICATION_RUNBOOK.md b/docs/MIGRATION_VERIFICATION_RUNBOOK.md new file mode 100644 index 0000000..fee1eea --- /dev/null +++ b/docs/MIGRATION_VERIFICATION_RUNBOOK.md @@ -0,0 +1,149 @@ +# Migration verification runbook + +How to confirm that a fresh Neon branch's migrated schema matches a +long-lived environment (prod, preview, or similar). The migration history +under `migrations/` is authoritative; this runbook is the manual check until +the queued `wire-migrate-into-ci` convoy automates it on every PR. + +## When to run + +- After any reconcile-adjacent convoy lands on `main`. +- Before releasing schema changes to prod when migration correctness is + uncertain. +- When suspecting drift between an env's actual schema and the migration + history (rare). + +## Prerequisites + +- A Neon account with permission to create and delete branches (or any + other way to spin up a fresh Postgres DB on the same major version as + prod). +- `POSTGRES_URL` for the branch under test — typically stored in a local + `.env.local.branch` or passed inline (never commit branch URLs). +- `ADMIN_INITIAL_PASSWORD` set (required by `npm run setup-db`; see + `AGENTS.md` § 5). +- `pg_dump` installed locally (PostgreSQL 16+), or available via a Docker + container with network access to Neon. + +## Procedure + +### 1. Create a throwaway Neon branch + +```bash +neon branches create --name migrate-verify-YYYYMMDD --parent main +``` + +Or create an equivalent branch via the Neon console. Copy the branch +connection string into `POSTGRES_URL`. + +### 2. Onboard the fresh branch via setup-db + +```bash +POSTGRES_URL= \ +ADMIN_INITIAL_PASSWORD=$(openssl rand -base64 24) \ + npm run setup-db +``` + +`setup-db` runs `npm run migrate up` (every file in `migrations/` in +timestamp order) and seeds the admin user. + +### 3. Snapshot the fresh branch schema + +```bash +POSTGRES_URL= pg_dump --schema-only --no-owner --no-acl \ + --schema=public > fresh-schema.sql +``` + +### 4. Snapshot prod schema (read-only) + +```bash +POSTGRES_URL= pg_dump --schema-only --no-owner --no-acl \ + --schema=public > prod-schema.sql +``` + +Use a read-only role or connection if your operator policy requires it. +Do not run DDL against prod during verification. + +### 5. Diff + +```bash +diff -u prod-schema.sql fresh-schema.sql +``` + +- **Empty output** — structural parity confirmed for tables, columns, + constraints, and indexes in `public`. +- **Non-empty output** — investigate before releasing; see below. + +### 6. Delete the throwaway branch + +```bash +neon branches delete migrate-verify-YYYYMMDD +``` + +## Known-safe diffs + +These differences from `pg_dump` are **not** real schema drift: + +- Schema owner / comment lines that differ between Neon projects. +- Extension version pins (`CREATE EXTENSION` version strings). +- `pgmigrations` row content — the fresh env accumulates rows as + migrations apply; prod may have the same rows with different apply + timestamps. +- Constraint or index **name** differences when prod objects were created + by historical `scripts/add-*` jobs with autogenerated names and fresh + envs use migration-defined names (material shape must still match). +- Column **order** within a table (historical ALTER order vs migration + order). + +## When drift is detected + +1. **Halt** — do not release until the delta is understood. +2. **Identify which env is canonical.** Usually prod. If prod is missing + a column that exists in the migration history, run + `POSTGRES_URL= npm run migrate up` to catch up. +3. **If prod has something not in the migration history**, file a new + convoy or brief describing the delta and ship a reconciliation + migration following the pattern in + `.convoys/reconcile-historical-add-scripts/`. +4. **Never edit existing migrations** — `pgmigrations` pins applied + files. Always add a new dated migration to correct schema. + +## Supplementary spot-checks + +For high-risk tables, compare column / constraint / index counts between +envs: + +```sql +SELECT table_name, column_name, data_type, is_nullable, column_default +FROM information_schema.columns +WHERE table_schema = 'public' +ORDER BY table_name, ordinal_position; + +SELECT table_name, constraint_name, constraint_type +FROM information_schema.table_constraints +WHERE table_schema = 'public' +ORDER BY table_name, constraint_name; + +SELECT tablename, indexname, indexdef +FROM pg_indexes +WHERE schemaname = 'public' +ORDER BY tablename, indexname; +``` + +A mismatch in column count, constraint count, or index count is material +drift. + +## Automation path + +The queued `wire-migrate-into-ci` convoy (see `.convoys/ship-readiness.md` +§ Queued convoys) will run `npm run migrate up` against a test DB on every +PR. Until that ships, this operator runbook is the only automated-parity +signal. + +## Cross-references + +- `AGENTS.md` § 4 Gotcha #6 — migration tool adoption history. +- `.convoys/migration-tool.md` — `node-pg-migrate` adoption convoy. +- `.convoys/reconcile-historical-add-scripts.md` § As-shipped — the convoy + that folded historical add-script DDL into `migrations/`. +- `.cursor/rules/db-and-schema.mdc` — schema-change conventions. diff --git a/docs/SCHEMA_MAP.md b/docs/SCHEMA_MAP.md index 0f852f1..dad7a72 100644 --- a/docs/SCHEMA_MAP.md +++ b/docs/SCHEMA_MAP.md @@ -173,9 +173,12 @@ Per-game catalog sync telemetry (`1781442329511_add-catalog-sync-log`). | Column | Type | Notes | | --- | --- | --- | | `id` | `SERIAL PK` | | -| `user_id`, `card_id` | FKs cascade | | -| `created_at` | `TIMESTAMP` | | -| | | **UNIQUE(user_id, card_id)** (verify constraint exists) | +| `user_id` | `INTEGER FK users(id) ON DELETE CASCADE` | | +| `item_type` | `VARCHAR(50) NOT NULL` | `'card'`, `'collection'`, or `'deck'` — polymorphic reference | +| `item_id` | `INTEGER NOT NULL` | Points at `cards.id` / `collections.id` / `decks.id` depending on `item_type` | +| `created_at` | `TIMESTAMP` default `CURRENT_TIMESTAMP` | | +| | | **UNIQUE(user_id, item_type, item_id)** — one favorite per (user, kind, target) | +| | | Indexes: `(user_id)`, `(item_type)`, `(item_id)`, `(user_id, item_type)` — `1781442330004_reconcile-favorites-system` | ### collections