From 22ebef24d71a185118800d22db60f73fd804fd5e Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Sun, 14 Jun 2026 07:38:59 -0500 Subject: [PATCH] =?UTF-8?q?convoy(architect):=20reconcile-historical-add-s?= =?UTF-8?q?cripts=20=E2=80=94=20inventory=20+=20decisions=20+=20brief=20ou?= =?UTF-8?q?tline?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Cursor --- .convoys/reconcile-historical-add-scripts.md | 704 +++++++++++++++++++ 1 file changed, 704 insertions(+) create mode 100644 .convoys/reconcile-historical-add-scripts.md diff --git a/.convoys/reconcile-historical-add-scripts.md b/.convoys/reconcile-historical-add-scripts.md new file mode 100644 index 0000000..e2a8c8c --- /dev/null +++ b/.convoys/reconcile-historical-add-scripts.md @@ -0,0 +1,704 @@ +--- +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`) but NOT the per-user backfill DML. Backfill is an application-runtime invariant — every user-registration code path is expected to create the system collection — so leaving the historical backfill out of migrations is intentional. | +| 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 only +- **Missed DDL (needs migration):** 8 — #1, #2, #3, #4, #5, #8∪#9 (deduplicate), #11 (with conflict resolution) +- **DML-only (not migration material):** 3 — #10 fix-lorcana, #12 seed-alice-bob, #13 seed-with-cards +- **Halt-and-ask:** 1 — #11's `UNIQUE(user_id, card_id)` vs `initial-schema`'s `UNIQUE(user_id, card_id, is_foil)` + +## Drift findings + +### Finding 1 — `user_cards` UNIQUE constraint conflict (HALT-AND-ASK) + +`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. + +### 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 is not captured + +The script's DDL (column add) is in `1780378340194_system-collection-description`. +The DML backfill (per-user `INSERT INTO collections ... 'All My Cards' +... is_system_collection=true` + matching `collection_permissions` +owner row) is NOT captured anywhere in the migration history. + +For prod: the backfill was run once and the rows exist. For fresh +envs: per-user system-collection creation is the responsibility of the +user-registration code path +(`pages/api/auth/register.js` — verify the create-on-register hook +exists). If it doesn't, fresh envs will have users without system +collections — a runtime bug that's separate from this convoy's scope. + +For this convoy: **no migration**. Note the runtime invariant in +`docs/SCHEMA_MAP.md` § "Notes" alongside `is_system_collection`. If the +register hook is missing, surface as `add-system-collection-on-register` +follow-up. + +### 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: 7 implementer briefs grouped by table/feature surface.** + +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 7-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) +- **B6** — `user_cards` / `collection_cards` named UNIQUE constraints + (#11 — gated on Drift Finding 1 halt-and-ask outcome) +- **B7** — Documentation + verification runbook (`docs/SCHEMA_MAP.md` + refresh, AGENTS.md Gotcha #6 update, operator runbook) + +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) +11. **NEW: B6** — user_cards/collection_cards named UNIQUE constraints (pending Finding 1 outcome) + +Each new migration is additive against the post-initial-schema state +that prior migrations leave behind; **no inter-migration dependencies +crossed within this convoy except B6's dependency on user_cards +existing (provided by B0 / initial-schema) — already satisfied.** + +Audited ordering risks for the new migrations: + +- B1 depends on `cards` (initial-schema #1) ✅ +- B2 depends on `collections` (initial-schema #4) ✅ +- B3 depends on `collections` (#4) + `users` (#1) ✅ +- B4 depends on `users` (#1) ✅ +- B5 depends on `users` (#1) ✅ +- B6 depends on `user_cards` (#3) + `collection_cards` (#5) ✅ + +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 + +Seven implementer briefs. Each B1-B6 ships exactly one new file +under `migrations/_.js`. B7 updates docs only. +Implementer briefs are **not** drafted in this convoy — they will be +created after Human Gate 1 approves this plan. + +| Brief | Title | Files (new) | Depends on | Est. LOC | Notes | +| --- | --- | --- | --- | --- | --- | +| B1 | Reconcile cards columns | `migrations/_add-cards-quantity-favorited.js` | — | ~40 | Captures script #1; script #7 already captured by initial-schema (verify & document) | +| B2 | Reconcile collections columns | `migrations/_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/_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/_reconcile-favorites-system.js` | — | ~50 | Captures #4 (user_favorites + 4 indexes). | +| B5 | Reconcile user profile | `migrations/_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 | `migrations/_reconcile-user-cards-constraints.js` | **HALT — gated on Finding 1 outcome** | ~30-80 (depending on outcome) | If Outcome A: DROP `user_cards_user_card_unique` on prod (real `down()` deferred — this is one-way). If Outcome C: ADD constraint on fresh envs idempotently. Outcome B: defer entire brief to `unify-user-cards-foil-tracking` convoy. | +| B7 | Documentation + verification | `docs/SCHEMA_MAP.md` (update); `docs/operations/RECONCILE-VERIFICATION.md` (new); `AGENTS.md` (Gotcha #6 update flipping to RESOLVED) | B1-B6 merged | ~150 | SCHEMA_MAP smell-list updates per Findings 1-3. Verification runbook (see § Verification plan below). AGENTS.md Gotcha #6 RESOLVED note. ship-readiness.md "Queued convoys" → flip entry to RESOLVED + unblock `retire-graveyard-scripts-after-audit`. | + +Per-brief acceptance criteria (sketch — implementer briefs will +formalize): + +- 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/_add-cards-quantity-favorited.js] + - brief: 2 + depends_on: [] + files: [migrations/_reconcile-collections-columns.js] + - brief: 3 + depends_on: [] + files: [migrations/_reconcile-collaboration-tables.js] + - brief: 4 + depends_on: [] + files: [migrations/_reconcile-favorites-system.js] + - brief: 5 + depends_on: [] + files: [migrations/_reconcile-user-profile.js] + - brief: 6 + depends_on: [] # gated by operator decision on Finding 1, not by another brief + files: [migrations/_reconcile-user-cards-constraints.js] + - brief: 7 + depends_on: [1, 2, 3, 4, 5, 6] + files: + - docs/SCHEMA_MAP.md + - docs/operations/RECONCILE-VERIFICATION.md + - AGENTS.md + - .convoys/ship-readiness.md +``` + +**Timestamp coordination.** Each B1-B6 writes a `migrations/_*.js` +file. Conductor assigns non-overlapping timestamps at dispatch (e.g. +`npm run migrate create` rounds to millisecond, so concurrent +generation would collide). Recommended pre-assignment: + +| Brief | Pre-assigned timestamp prefix | +| --- | --- | +| B1 | 1781000000001 | +| B2 | 1781000000002 | +| B3 | 1781000000003 | +| B4 | 1781000000004 | +| B5 | 1781000000005 | +| B6 | 1781000000006 | + +Implementer briefs will spell out the exact filename; the timestamps +are reservation tokens, not literal `Date.now()` values. (Pre-assigned +timestamps avoid the parallel-implementer collision risk documented +in `scaffold-nextjs-app` retro recommendation #4.) + +## 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-B6 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 (halt-and-ask) + +B6 cannot land without operator picking Outcome A / B / C from +Finding 1. If approval is delayed, B1-B5 + B7 can still land (B7's +SCHEMA_MAP update can flag B6 as "deferred pending operator decision" +and resolve in a follow-up PR once the decision arrives). Conductor +should not block B7 on B6 specifically; B7's `depends_on: [1, 2, 3, +4, 5, 6]` allows B6 to be deferred if necessary, with a re-PR for +B7 once B6 lands. + +## 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`** (priority: P1 correctness if + the runtime hook is missing; P3 hygiene if it exists but is + undocumented — NEW, conditional). Driven by Finding 4. Verify + `pages/api/auth/register.js` creates an + `is_system_collection = true` row on user registration. If not, + fresh envs have users without system collections and the runtime + behavior is broken. +- **`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`** (priority: P2 correctness, NEW, + conditional on Finding 1 Outcome B). If operator picks B, this + convoy designs a non-tuple foil-tracking shape on `user_cards`. + +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 + +_Stub for role-doc-writer to fill once the convoy ships. Per-brief +PR numbers, squash SHAs, ship dates, verification-plan run results, +and any deviations from this plan land here._