From 7e06ab559a3e6ef1de3f6106deae749d8e5beaf9 Mon Sep 17 00:00:00 2001 From: varutasu <104105839+varutasu@users.noreply.github.com> Date: Mon, 25 May 2026 04:10:12 -0500 Subject: [PATCH] =?UTF-8?q?docs:=20post-convoy=20cleanup=20for=20pick-a-na?= =?UTF-8?q?me=20=E2=80=94=20first=20post-P0=20P1=20convoy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-merge doc cleanup for PR #21. Resolves the AGENTS.md line-5 'Pick one before launch' branding question. Updates AGENTS.md top branding note + Gotcha #4 + Gotcha #12, adds full as-shipped to .convoys/pick-a-name.md, adds 4 queued-convoy entries to ship-readiness.md, adds pick-a-name attribution to .cursor/rules/api-routes.mdc. Pure docs (+357/-15 across 4 files). --- .convoys/pick-a-name.md | 317 ++++++++++++++++++++++++++++++++++- .convoys/ship-readiness.md | 15 +- .cursor/rules/api-routes.mdc | 2 +- AGENTS.md | 38 ++++- 4 files changed, 357 insertions(+), 15 deletions(-) diff --git a/.convoys/pick-a-name.md b/.convoys/pick-a-name.md index 478eb39..eca28c2 100644 --- a/.convoys/pick-a-name.md +++ b/.convoys/pick-a-name.md @@ -1,11 +1,12 @@ # pick-a-name (P1 — pre-launch brand consistency) -**Status:** in-progress +**Status:** shipped **Priority:** P1 (pre-launch; not a security blocker, but a brand-consistency blocker that touches every user-facing surface) **Convoy owner:** parent (orchestration); `role-architect` then `role-implementer(s)` then `role-doc-writer` **Opened:** 2026-05-24 +**Shipped:** 2026-05-24 **Operator decision (gate-0, already ratified):** **Deck Hearth wins.** ## Problem @@ -593,4 +594,316 @@ Two NEW follow-ups surfaced by architect inventory: ### As-shipped -(To be appended post-merge by `role-doc-writer`.) +Shipped 2026-05-24 as squash commit `9abbab6` (PR #21, architect-commit +`50ce9ab`, Brief 1 commit `ac8c998`, Brief 2 commit `1c18d21`). **This is +the first post-P0 convoy** — `add-rate-limiting` (squash `708ef45`, PR +#20, 2026-05-24) closed the LAST P0 ship-blocker, flipping +`.convoys/ship-readiness.md`'s § Status summary to "8 of 8 RESOLVED"; +this convoy opens the post-P0 P1 quality-bar phase by closing the +brand-consistency P1 (#12 in the same file) that AGENTS.md line 5 has +flagged since project setup. Two briefs in the convoy: B1 +(display + comment sweep) and B2 (infrastructure + email migration) +shipped without scope expansions and with all five D-decisions ratified +verbatim at gate-1 plus Risk 4 PRESERVE applied to the negative +regression-lock test. + +#### Decisions ratified at gate-1 + +All five canonical-string choices required operator gate-1 ratification +(naming choices are not architect-self-ratifiable per the convoy file's +§ Architect-proposed decisions header). Operator ratified all five +verbatim: + +- **D1 — Display string:** **"Deck Hearth"** (two words, internal cap, + space-separated). Avoids re-sweeping the seven user-facing surfaces + that already render the two-word form (`components/Layout.js`, + `components/AnimatedFireLogo.js`, `pages/index.js`, `pages/login.js` + welcome copy, `pages/signup.js`, `components/ShareModal.js`, + `styles/globals.css`). +- **D2 — Kebab string:** **`deck-hearth`**. Matches the existing + `deck-hearth-logo-container` CSS class + Node-ecosystem convention for + `package.json` `name` fields. Used in `package.json` rename + the + three `User-Agent` HTTP product tokens in `pages/api/cards/import-*.js` + + `scripts/import-lorcana.js` (`Deck-Hearth/1.0`, hyphenated lowercase + per HTTP UA convention). +- **D3 — Camel/lower for IDs:** **`deckhearth`** (single token, no + separators). Used in the five Redis key prefixes + (`deckhearth:auth/search/upload/generate/import`) — full word, NOT the + abbreviated `dh` (the ~800KB Redis-memory savings at peak load is + negligible against an Upstash plan that bills by request count, and + the self-documenting prefix is materially easier to debug in Redis + console output). +- **D4 — Admin email:** **`admin@deckhearth.com`** (placeholder TLD + pending the queued `point-domain-at-deckhearth` convoy). Sweep + applied to all three `@tcgvault.com` accounts uniformly via + `REPLACE(email, '@tcgvault.com', '@deckhearth.com')`: admin + alice + + bob (the test users created by `scripts/create-test-users.js`). +- **D5 — Redis prefix:** **full `deckhearth`**. Pattern: + `deckhearth:auth`, `deckhearth:search`, `deckhearth:upload`, + `deckhearth:generate`, `deckhearth:import`. The five-prefixes-must-be- + distinct invariant from `add-rate-limiting` Brief 1 is preserved. + +**Risk 4 PRESERVE applied:** the architect's recommendation +(`.convoys/pick-a-name.md` § Risk list § 4) was to preserve the +historical `admin@tcgvault.com` literal in +`test/lib/permission-middleware.test.js` line 87's negative regression +assertion as the EXACT pre-`fix-auth-bypass` synthetic-admin shape this +test locks against. The implementer kept the literal and added a +7-line comment block above the `.not.toEqual({...})` call documenting +why future doc-writers / refactorers must NOT update the literal to the +new `@deckhearth.com` email — doing so would weaken the regression-lock +to a shape that never actually existed. **This matches the project's +established pattern** of treating historical-shape locks as audit-trail +artifacts rather than self-consistency markers (Gotchas #2, #3, #4, #5, +#8 in `AGENTS.md` all note "Entry kept (not renumbered) to preserve +cross-references" for the same reason). The post-merge state has +exactly **1** `@tcgvault.com` literal outside of `.convoys/**` and +`*.md` files — the preserved historical regression-lock. + +#### Brief structure (file-disjoint; serial-in-practice) + +The architect designed B1 (7 files: `AGENTS.md` line 5, two +`.cursor/rules/*.mdc` description / branding paragraphs, `scripts/README.md` +title, three `User-Agent` strings) and B2 (10 modified + 1 new +migration script: `lib/rate-limit.js` 5 prefix renames, `package.json` +`name` rename, `package-lock.json` regen, `README.md` + `TESTING_GUIDE.md` +title + email + operator caveat, three seed/reset/test-user scripts' +admin/alice/bob email rename, `pages/login.js` quick-login fixtures, +`test/lib/permission-middleware.test.js` Risk 4 PRESERVE comment block, +new `scripts/migrations/2026-05-24-rename-admin-email.js`) so that the +two `files:` sets are DISJOINT and the conductor could dispatch them in +parallel via `/multitask`. **In practice the two briefs landed +serially** (B1 commit `ac8c998` first, B2 commit `1c18d21` second on +the same branch) for the lockfile-regen + migration-script-author +caution recommended by Brief 2 § Risk 3 (lockfile STOP-on-churn +verification is easier to audit if no other diff is in flight). The +parallel-via-multitask path stayed open as the architect's intended +fallback if either brief had hit a blocker. + +#### As-shipped diff + +Per `git show --stat 9abbab6`: + +- **B1 (`ac8c998`): 7+/7-** across 7 files (the 7 `files:` listed in + `.convoys/pick-a-name/brief-1-display-and-comment-sweep.md`). Pure + string-literal edits; no new code, no new imports, zero ESLint + surface added. +- **B2 (`1c18d21`): 133+/30-** across 10 modified files + 1 new + migration script. The +/- imbalance is dominated by the new + ~85-line migration script (`scripts/migrations/2026-05-24-rename- + admin-email.js`) + the new operator-caveat blockquote in `README.md` + (~10 lines added) + the 7-line Risk 4 PRESERVE comment block in + `test/lib/permission-middleware.test.js`. Source-code edits in + `lib/rate-limit.js` (5 prefix lines) + `pages/login.js` (2 quick- + login fixtures) + `scripts/setup-neon-db.js` / `reset-db.js` / + `create-test-users.js` (admin/alice/bob email INSERTs + console + logs) are pure string-literal swaps. +- **Lockfile regen:** the `package-lock.json` diff was confirmed + before B2 commit to touch ONLY the two `name` lines (top-level + + `packages[""]` entry); the brief's STOP-on-churn rule (Risk 3) was + exercised by the implementer and confirmed clean — no opportunistic + dependency-metadata refresh, no integrity-hash churn, no `version` + bumps. `npm ci` succeeded against the regenerated lockfile. + +#### As-shipped CI metrics + +Post-merge run 26387909625 + subsequent runs on `main`: + +- **`Playwright smoke` — PASS in 1m4s, 3/3 tests** against the rebranded + Vercel preview. Same three checks (`home redirects or renders without + 5xx` ✓ / `sign-in page renders` ✓ / `public health endpoint responds` + ✓) — all green. The new brand strings render correctly on `/` + + `/login`; the auth surface still works (smoke test 2 still asserts + the `Sign in` CTA wording, which is + brand-agnostic per architect-time verification of the smoke spec's + text matchers); no false 429s from the renamed Redis prefixes (the + post-rename `deckhearth:*` keys were brand-fresh in Upstash, but the + smoke spec's `/api/health` hit is anonymous and unrate-limited and + the home + sign-in routes don't touch any of the 6 newly-gated + rate-limit endpoints). +- **`Aggregate gate` — PASS in 10s.** All upstream gate dependencies + green. +- **`Lint` — PASS in 45s, 128 problems** (lint baseline preserved, no + regression from B1 or B2). Zero new lint problems; `|| true` wrapper + in `.github/workflows/ci.yml` was a no-op for this convoy. +- **`forbidden-endpoints` (from `fix-auth-bypass` Brief 3) — PASS in + 4s.** No new `pages/api/test-*.js` shapes reintroduced. +- **`forbidden-cors-headers` (from `cors-tighten`) — PASS in 4s.** Pure + brand-rename diff; no `Access-Control-Allow-*` headers reintroduced. +- **`Unit tests (vitest)` — PASS, 21/21 in 35s.** The Risk 4 PRESERVE + comment-block addition kept the negative regression-lock test green + byte-identically — the assertion semantics are unchanged; only the + comment block above the `.not.toEqual({...})` call is new. Decision + 6 (no new vitest specs in this convoy) honored per the + `add-rate-limiting` D6 + `cors-tighten` D4 precedent. +- **`Screenshot diff` — PASS in 1m5s** (`continue-on-error: true` + swallow per `adopt-playwright-smoke` Decision 4 still active; the + rebrand introduced no visual regression because no baseline is + committed yet — the queued `seed-visual-baselines-on-linux` convoy + generates the FIRST baseline and that baseline will capture the + Deck Hearth brand strings, per the architect's ordering note in + `.convoys/pick-a-name.md` § Test plan + the updated entry in + `.convoys/ship-readiness.md` § Queued convoys). +- **`Schema map up to date`** — skipping (only runs on Prisma changes; + this convoy made an INSERT-semantics change but no DDL change). +- **Vercel deployment** — completed cleanly. First production-tier + Vercel deploy serving the `deck-hearth` `package.json` `name`. + +#### Cross-validation finding: smoke spec defends the brand-rename surface (organically) + +This convoy is the **fourth in a row** where the same 3-test Playwright +smoke spec authored by `adopt-playwright-smoke` (PR #18, 2026-05-24) +defended the auth surface through a sweeping change without anyone +having to write a dedicated test. The lineage now reads: + +- **PR #15** (`fix-layout-default-user`, `ca302a8`) — introduced the + `Sign in` CTA that smoke test 2 asserts on. +- **PR #19** (`cors-tighten`, `da50d78`) — removed wildcard CORS from + 24 handlers; smoke test 2 still passed against the post-CORS preview. +- **PR #20** (`add-rate-limiting`, `708ef45`) — wired 6 new route gates + (search/upload/generate/import) and refactored `lib/rate-limit.js` + end-to-end; smoke test 2 still passed (the new 60/min search ceiling + doesn't 429 the smoke spec because home + `/login` don't touch any + gated route). +- **PR #21** (`pick-a-name`, `9abbab6`) — this convoy — renamed the + product brand end-to-end across 17 files + 1 new migration script, + including all 5 Redis key prefixes; smoke test 2 still passes. + +The smoke spec's text matchers (`'sign in'` regex, `/api/health`, +HTTP status < 500) are brand-agnostic by architect-time design — but +**organically**, smoke is doing real work: it has now defended the +auth surface against four sweeping changes spanning the full P0 #5 ++ P0 #6 + P0 #7 closures + the P1 #12 brand-consistency closure. +That's a "trust the smoke spec" data point worth preserving. Future +sweeping changes (single-auth-provider, single-sql-client, the +god-component-split convoys) should expect the smoke spec to keep +catching auth-surface regressions without dedicated coverage — +unless and until a feature change deliberately rewords the +`Sign in` CTA, in which case the smoke spec needs a +matcher update in the same PR. + +#### Implementer-vs-architect calibration note + +The architect's B2 forecast paragraph (`.convoys/pick-a-name/brief-2- +infrastructure-and-email-migration.md` line 662, the post-B2 +repo-wide grep cross-check) reads *"Expected: at most 1 match — +`test/lib/permission-middleware.test.js` line 87's preserved historical +regression literal"*. Architect-verified — the implementer's actual +post-B2 state showed exactly 1 non-`.convoys/`-non-`*.md` match. **But +the architect's authoritative § Full surface inventory § Convoy +artifacts + manifest list (line 344 of the convoy file)** had already +correctly enumerated `scripts/seed-collections-with-cards.js` and +`scripts/seed-collections-alice-bob.js` (3 hits each) as DO-NOT-TOUCH +historical seed jobs per `.cursor/rules/no-go-zones.mdc` § Append-only +/ historical, with the explicit reasoning: *"the email migration in B2 +covers the in-DB rename, so these scripts' WHERE-clause literals would +simply match zero rows post-migration if re-run, which is a harmless +no-op."* So the forecast paragraph and the inventory are both correct +— the forecast counted "in-scope source files" while the inventory +documented why the seed-collections matches stay. Worth noting for the +next architect: when forecasting post-brief grep-cleanliness, cite the +inventory's exact carve-out rationale to avoid a reader thinking the +two numbers contradict each other. Not a process gap — both ends were +right; the cross-reference between them could be tighter. + +#### Operator action required going forward + +**Run `node scripts/migrations/2026-05-24-rename-admin-email.js` +against the prod Neon DB before any admin login attempt with the new +`admin@deckhearth.com` email.** The migration is ESM, idempotent, +UNIQUE-collision-safe — re-running on an already-migrated DB prints +"Nothing to migrate." and exits 0; running on a DB where the seed +script already created a duplicate `admin@deckhearth.com` row fails +loud on the `users.email` UNIQUE constraint (which is the correct +fail-loud behavior — manually delete one of the duplicate admin rows +before retrying). **Order: migration FIRST, then any subsequent +`npm run setup-db`** — the migration sweeps the existing +`@tcgvault.com` row to `@deckhearth.com`; a subsequent `setup-db` +sees the row exists with the new email and no-ops via `ON CONFLICT +(email) DO NOTHING`. + +Documented in three places for redundancy: + +- `AGENTS.md` Gotcha #4 (extended in this doc-writer pass with the + email-rename + migration-script + ordering paragraph). +- `README.md` operator caveat blockquote (added by B2 alongside the + existing `admin123`-rotation caveat from `drop-public-setup`). +- The migration script's own header comment + stdout messages. + +Two downstream convoys are queued in `.convoys/ship-readiness.md` +§ Queued convoys after this one: + +- **`rename-repo-and-vercel-project`** (P2 polish) — GitHub repo + + Vercel project rename to `deck-hearth`; auto-redirects make it + low-urgency. One-line update to local git remotes + Vercel link + + the 8 architect-verified `tcg-vault` literal-repo references + (README cd / tree, AGENTS.md line 1, three workflow YAML comments, + `.agent-context-manifest.yml` source tags). +- **`point-domain-at-deckhearth`** (blocked on domain acquisition) + — DNS pointer + Vercel domain claim + admin email TLD swap if the + operator buys a TLD other than `.com`. + +#### What did NOT change + +Audit trail of files explicitly NOT touched by this convoy, despite +sitting near the brand-rename surface: + +- **All `.convoys/**` historical artifacts.** ~25 lines across the + drop-public-setup / fix-auth-bypass / cors-tighten / add-rate-limiting + / fix-layout-default-user / bump-next-js / fix-vercel-deployment- + protection convoy files reference `tcgvault` / `tcg-vault` / + `TCG Vault` as the historical state at write-time — per + `.cursor/rules/no-go-zones.mdc` § Append-only / historical, these + stay byte-identical. The doc-writer pass at THIS convoy's close + rewrites only `.convoys/pick-a-name.md` (this file) + Gotcha #4 + + Gotcha #12 of `AGENTS.md` + the launch-sequence step in + `.convoys/ship-readiness.md` + the lineage attribution in + `.cursor/rules/api-routes.mdc`. No retroactive historical rewrite. +- **`scripts/seed-collections-with-cards.js`, + `scripts/seed-collections-alice-bob.js`, + `scripts/add-*.js`, `scripts/fix-*.js`** — historical seed / + migration jobs per the no-go-zones rule. The 6 hits (3 each in the + two seed-collections scripts) of `@tcgvault.com` literals in their + WHERE-clauses are now stale — the migration sweeps the in-DB rows, + so any re-run of these scripts would match zero rows and no-op + harmlessly. +- **Repo-name references** — `README.md` lines 30 + 105, `AGENTS.md` + line 1, `.github/workflows/ci.yml` lines 12 + 123, + `.github/workflows/visual-diff.yml` line 5, + `.agent-context-manifest.yml` source tags. All literally describe + the GitHub repo + Vercel project, both of which still ARE named + `tcg-vault` until the queued `rename-repo-and-vercel-project` convoy + ships. Per the convoy seed § *"Keep the repo `tcg-vault` for now"*. +- **`docs/agent-context/README.md` line 78** (`"we'll measure + tcg-vault's number after a few convoys"`) and + **`.cursor/skills/add-api-route/SKILL.md` line 7** + (`"…for the tcg-vault stack"`) — both reference the literal repo + name, not the product brand. Stay byte-identical; they sweep + together with `rename-repo-and-vercel-project`. +- **`scripts/reset-db.js` line 142** (CJS-in-ESM bug) and **lines + 143 + 156** (`admin123` weak password literals) — out of scope + per `convert-reset-db-to-esm` (now queued, surfaced 2026-05-24 + during this convoy's architect inventory) + `purge-weak-creds- + from-helpers`. B2 only renamed the email literals on lines 147 + + 155; the CJS bug + weak-password literals stay byte-identical for + the dedicated follow-up convoys to fix together. +- **`tests/smoke/**`, `tests/visual/**`, `playwright.config.js`** + — smoke spec is brand-agnostic (architect-verified at brief time); + visual baseline doesn't exist yet (per + `adopt-playwright-smoke` Decision 4). The queued + `seed-visual-baselines-on-linux` convoy generates the first + baseline AFTER `pick-a-name` lands so the captured strings are + Deck Hearth (now satisfied — `9abbab6` is on `main`). +- **`.agent-context-manifest.yml` `source: "tcg-vault-local"` tags** + (6 entries) — internal `sync-agent-context` skill source identifier + for drift tracking. Renaming could break the skill's behavior; + flagged for re-evaluation in `rename-repo-and-vercel-project` when + the actual repo name changes (Risk 5 of the convoy file). +- **`pages/api/cards/import-mtg.js`'s `User-Agent`** — the file does + NOT set a custom UA (Scryfall is fine with Node's default per + Brief 1 Finding 2). Only the Lorcana + Pokémon TCG import handlers + + the standalone `scripts/import-lorcana.js` CLI variant set + `Deck-Hearth/1.0`. +- **No new vitest or playwright specs** — Decision 6 honored. The + Risk 4 PRESERVE comment-block addition is the only test-file edit; + assertion semantics are unchanged. diff --git a/.convoys/ship-readiness.md b/.convoys/ship-readiness.md index 9d30c63..7f419db 100644 --- a/.convoys/ship-readiness.md +++ b/.convoys/ship-readiness.md @@ -36,6 +36,11 @@ single auth provider, lint baseline cleanup, brand decision, test coverage expansion) plus the P2 / P3 polish lanes in this file's Queued convoys section. None of those are P0 ship-blockers. +**P1 quality-bar work in progress** (pick-a-name shipped 2026-05-24, +PR #21 squash `9abbab6`, closing P1 #12 brand-consistency — the +inconsistency `AGENTS.md` line 5 had flagged since project setup; +single-auth-provider, single-sql-client, migration-tool queued). + ## P0 — ship-blockers (security) These MUST land before any anonymous traffic touches the production URL. @@ -338,7 +343,7 @@ Each phase is one Conductor-created convoy. Don't run more than two in parallel 3. **`fix-layout-default-user`** (P0 #7). One PR. Trivial. 3.5. **`fix-lint-baseline`** (P1 #11.5). 2-4 PRs via multitask. Closes the lint gate (drops `continue-on-error`). 4. **`add-rate-limiting`** (P0 #6 full). One PR. Adds @upstash/ratelimit + applies to listed routes. **RESOLVED 2026-05-24** — PR #20 squash `708ef45`; closes P0 #6 (last open P0), flipping the ship-blocker set to **8/8 RESOLVED**. 5 named limiters (auth/search/upload/generate/import), 6 routes newly gated + the 3 import routes auth-gated atomically with a `pages/admin/card-import.js` Bearer-header fix. Smoke 3/3 green in 3.8s post-merge — confirms the new 60/min search limiter doesn't 429 the smoke spec. See § Queued convoys and P0 #6 above for the full as-shipped block. -5. **`pick-a-name`** (P1 #12). Human decision first, then one or two PRs. +5. **`pick-a-name`** (P1 #12). Human decision first, then one or two PRs. **RESOLVED 2026-05-24** — PR #21 squash `9abbab6`; closes P1 #12 (brand-consistency, the inconsistency `AGENTS.md` line 5 had flagged since project setup). Two file-disjoint briefs landed serially (B1 commit `ac8c998` display + comment sweep across 7 files; B2 commit `1c18d21` infrastructure + email migration across 10 modified + 1 new migration script). Five canonical-string D-decisions ratified verbatim at gate-1 (Deck Hearth / `deck-hearth` / `deckhearth` / `admin@deckhearth.com` / full `deckhearth` Redis prefix) plus Risk 4 PRESERVE on `test/lib/permission-middleware.test.js` line 87's negative regression-lock literal. Smoke 3/3 green in 1m4s post-merge — fourth convoy in a row (PR #15 → #19 → #20 → #21) where the same 3-test smoke spec defends the auth surface through a sweeping change. **Operator action required:** run `node scripts/migrations/2026-05-24-rename-admin-email.js` against prod Neon BEFORE the next admin login attempt with the new email (idempotent, UNIQUE-collision-safe). See § Queued convoys for the downstream `rename-repo-and-vercel-project` + `point-domain-at-deckhearth` + `regenerate-brand-assets` follow-ups, and `.convoys/pick-a-name.md` § As-shipped for the full record. 6. **`adopt-vitest`** (P1 #10 step 1). One PR. Enables testing every future change. 7. **`migration-tool`** (P1 #11). One PR. Backfill + first new migration. 8. **`single-sql-client`** (P1 #8). 2-3 PRs, fanned out via multitask once per-file briefs are written. @@ -357,14 +362,18 @@ Follow-ups surfaced mid-convoy or mid-PR that didn't fit the original launch seq - **`rotate-default-admin`** (priority: P2 hygiene). Operator-rotation script for envs that ran `setup-neon-db.js` before `drop-public-setup` and still carry the weak `admin123` bcrypt hash. Surfaced in P0 #3 § Operator caveat. Optional: do nothing if no audit finds a deployed env with the weak hash. - **`delete-dead-lorcana-import`** (priority: P3 polish). Delete `pages/api/cards/import-lorcana.js` (and possibly `scripts/import-lorcana.js`) if Lorcana stays out of the admin UI's `` only offers `'mtg'` and `'pokemon'`. The route is gated defensively (auth + admin-role + rate-limit) as part of PR #20 so the future Lorcana admin UI path inherits protection automatically, but if Lorcana is never wired in, this is the cleanup convoy. Strictly easier than gating-then-deleting because the gating shape is uniform across all three import routes today (mtg + pokemon + lorcana); a future cleanup only needs to delete the lorcana file + remove the `'lorcana'` enum option from `.cursor/rules/api-routes.mdc`'s import-routes table. Do nothing if Lorcana support gets wired into the admin UI in a feature convoy; cancel the entry then. - **`tighten-visual-diff-path-filter`** (priority: P3 polish). Tighten `.github/workflows/visual-diff.yml`'s `paths:` filter from `pages/**` to something like `pages/!(api)/**` so API-only PRs don't trigger the visual workflow. Surfaced on PR #19 (`cors-tighten`) which was API-only but triggered `Screenshot diff` anyway, wasting ~55s of CI time. PR #20 (`add-rate-limiting`) re-triggered the same false-positive — same shape, same swallow via `continue-on-error: true`. Low priority — works as designed today, just chatty. One-line YAML tweak; verify the negated-glob syntax against GitHub Actions' [path-filter semantics](https://docs.github.com/en/actions/using-workflows/triggering-a-workflow#filter-pattern-cheat-sheet) before merging (the `!()` extglob may need to be expressed differently if Actions uses minimatch without extglob support). -- **`purge-weak-creds-from-helpers`** (priority: P2 hygiene). Sweep `scripts/reset-db.js`, `scripts/create-test-users.js`, and `TESTING_GUIDE.md` for the literal `admin@tcgvault.com` / `admin123` references. May fold into `pick-a-name` since the email itself is changing. +- **`purge-weak-creds-from-helpers`** (priority: P2 hygiene). Sweep `scripts/reset-db.js`, `scripts/create-test-users.js`, and `TESTING_GUIDE.md` for the remaining `admin123` / `alice123` / `bob123` weak password literals. **Email-half satisfied 2026-05-24** by `pick-a-name` Brief 2 (squash `9abbab6`) — the `admin@tcgvault.com` / `alice@tcgvault.com` / `bob@tcgvault.com` references in those three helper paths were swept to `@deckhearth.com` together with the migration script. The remaining scope is the weak-password literals only (`admin123` etc. survived `pick-a-name` deliberately per Brief 2's scope boundary). May fold into `convert-reset-db-to-esm` (the CJS-in-ESM bug at `scripts/reset-db.js:142` overlaps the same file). +- **`rename-repo-and-vercel-project`** (priority: P2 polish). Rename the GitHub repo + the Vercel project from `tcg-vault` to `deck-hearth` to match the canonical product brand ratified in `pick-a-name` (squash `9abbab6`, 2026-05-24). Auto-redirects on both GitHub and Vercel make this low-urgency; the surface is a one-line update to local git remotes (`git remote set-url origin git@github.com:/deck-hearth.git`) + a Vercel project-settings rename + the 8 architect-verified literal-repo references documented in `.convoys/pick-a-name.md` § Full surface inventory § Repo / Vercel project name (out-of-scope) — `README.md` lines 30 + 105, `AGENTS.md` line 1, `.github/workflows/ci.yml` lines 12 + 123, `.github/workflows/visual-diff.yml` line 5, `.agent-context-manifest.yml` source tags. Also re-evaluate the `.agent-context-manifest.yml` `source: "tcg-vault-local"` tag at that point (Risk 5 of `pick-a-name` — renaming the source tag could break the `sync-agent-context` skill's drift tracking; do this convoy with the sync-skill author's input). Surfaced 2026-05-24 as the explicit downstream of `pick-a-name`. +- **`point-domain-at-deckhearth`** (priority: P2 polish; **blocked on domain acquisition**). Once the operator buys `deckhearth.com` (or `.app` / `.gg` / other), wire DNS to the Vercel deployment + claim the domain in Vercel's project settings + update the seed admin email's TLD if the purchased TLD is anything other than `.com` (a one-line REPLACE migration mirroring `scripts/migrations/2026-05-24-rename-admin-email.js`). Surfaced 2026-05-24 as the explicit downstream of `pick-a-name` (the convoy seed § *"DNS / domain — out of scope — separate convoy `point-domain-at-deckhearth` (you don't own a `deckhearth.*` domain yet per operator's pre-convoy statement)"*). Until this lands, the seed admin email `admin@deckhearth.com` is a placeholder STRING used as a unique identifier — auth uses email as identity, not as a mail target, so no working mailbox is required for login to function. +- **`regenerate-brand-assets`** (priority: P2 polish). Regenerate the favicon (`public/favicon.ico`), Open Graph images, and social share cards with the new Deck Hearth identity. Requires a design pass — out-of-scope for any single agent-driven convoy; queue when an asset-design pass is scheduled. Surfaced 2026-05-22 originally in `.convoys/ship-readiness.md` § Role-design-system-auditor findings ("Then once the name is settled, the 'DH' logo + AnimatedFireLogo need to be unified into one brand mark"); reaffirmed 2026-05-24 in `pick-a-name` Out-of-scope queued follow-ups. +- **`convert-reset-db-to-esm`** (priority: P2 hygiene). `scripts/reset-db.js` line 142 uses CommonJS `require('bcryptjs')` inside an ESM file (`package.json` has `"type": "module"` since `bump-next-js`). Same bug pattern that hit `setup-neon-db.js` pre-`drop-public-setup` Brief 2. The script is broken on Node 22.x — `npm run reset-db` doesn't actually work today. Surfaced 2026-05-24 in `pick-a-name` architect inventory (Brief 2 Boot-the-brief Finding 3); B2 only renamed the email literals on lines 147 + 155 and left the CJS bug + weak-password literals on lines 143 + 156 byte-identical because both fixes are out-of-scope for the brand-rename convoy. **May fold into `purge-weak-creds-from-helpers`** if more such bugs accumulate in helper scripts — both convoys touch the same file (`scripts/reset-db.js`) and the same fix sequence (CJS → ESM, then strip the weak-password literals + the password-echo console log) keeps the diff atomic. - **`single-auth-provider`** (priority: P1 quality, also listed as launch sequence step 9). Collapse `lib/auth-context.js` + `lib/admin-auth.js` into `lib/use-auth.js`. Surfaced again as a follow-up in P0 #7 § Flagged-but-deferred (4 pages still import the legacy `useAuth`). - **`cleanup-mobile-nav-dead-props`** (priority: P3 polish). `components/MobileNavigation.js` accepts a dead `user` prop; remove it. Surfaced in P0 #7 § Flagged-but-deferred. May fold into `god-component-split` (P2 #13) if that lands first. - **`bump-eslint-10`** (priority: P2 hygiene; upstream-blocked). Bump ESLint from v9 to v10 once `typescript-eslint` ships a v10-tested release and `eslint-config-next` bundles it. Surfaced in `.convoys/bump-next-js.md` § Decisions D. - **`harden-multipart-parser`** (priority: P2 quality). Surfaced 2026-05-24 in `add-rate-limiting` § Risk list. `pages/api/user/avatar.js`'s `parseMultipartFormData` consumes the 5MB multipart body via `req.on('data')` before any response is sent, so an attacker can still exhaust the 5MB body even on a 429 path from the new `checkUploadRateLimit` gate. Real defense requires moving the parse into a separate edge function or using `read-up-to` semantics. Not a release-blocker — the gate-ordering in PR #20 places the limiter BEFORE the method branches that call `parseMultipartFormData`, so when this hardening lands, the gate ordering is already correct. Surface as P1 only if a real abuse incident occurs. - **`god-function-split` / `refactor-cards-search-sql`** (priority: P2 refactor). Surfaced 2026-05-24 in `add-rate-limiting` § Files explicitly out of scope. `pages/api/cards/search.js` has a 240-line god-function shape with 7+ conditional `SELECT * FROM cards WHERE …` branches; the PR #20 rate-limit gate sits at the top of the handler and leaves the SQL byte-identical. Splitting is its own scope (probably one convoy per branch group with `slice_dependencies:` for safe multitask fan-out). Not security-critical; deferred to the P2 lane. - **`withAdmin(handler)` wrapper extraction** (priority: P3 polish / DX). Surfaced 2026-05-24 in `add-rate-limiting` Decision 1 + § What did NOT change. `.cursor/rules/auth-and-permissions.mdc` notes *"check `user.role === 'admin'` directly; consider extracting `withAdmin()` if a third call site appears"* — the three `cards/import-*.js` routes are the third+fourth+fifth call sites in the codebase, but PR #20 kept the inline shape for uniformity across the three import routes and for the convoy's atomic-close-P0-#6 goal. A future convoy can extract `withAdmin(handler)` to `lib/permission-middleware.js` (or wherever the architect decides) and sweep all 5 admin-role check sites onto it. Pure refactor; no security delta either way. -- **`seed-visual-baselines-on-linux`** (priority: P2 CI infra; **operator action required**). Generate Linux baselines for `tests/visual/__screenshots__/` in the `mcr.microsoft.com/playwright:v1.60.0-noble` Docker image and commit them in a small follow-up PR. Mac-generated baselines would silently overwrite Linux CI baselines because `playwright.config.js`'s custom `snapshotPathTemplate` has no `{platform}` token (Risk R3 + Boot-the-brief Finding 7 in `.convoys/adopt-playwright-smoke.md`). Until this PR lands, every `Screenshot diff` run on a PR touching `pages/**` / `components/**` / `styles/**` / Tailwind/PostCSS config fails at the test step and posts a "Visual Diff — view run" comment with empty artifacts — that's the documented Decision-4 end state of `adopt-playwright-smoke`, not a regression. One small PR with just the PNG baseline(s). Surfaced 2026-05-24 as the follow-up to `adopt-playwright-smoke` (PR #18). +- **`seed-visual-baselines-on-linux`** (priority: P2 CI infra; **operator action required**). Generate Linux baselines for `tests/visual/__screenshots__/` in the `mcr.microsoft.com/playwright:v1.60.0-noble` Docker image and commit them in a small follow-up PR. Mac-generated baselines would silently overwrite Linux CI baselines because `playwright.config.js`'s custom `snapshotPathTemplate` has no `{platform}` token (Risk R3 + Boot-the-brief Finding 7 in `.convoys/adopt-playwright-smoke.md`). Until this PR lands, every `Screenshot diff` run on a PR touching `pages/**` / `components/**` / `styles/**` / Tailwind/PostCSS config fails at the test step and posts a "Visual Diff — view run" comment with empty artifacts — that's the documented Decision-4 end state of `adopt-playwright-smoke`, not a regression. One small PR with just the PNG baseline(s). Surfaced 2026-05-24 as the follow-up to `adopt-playwright-smoke` (PR #18). **Ordering: MUST run AFTER `pick-a-name` (now satisfied — squash `9abbab6` merged 2026-05-24); the first Linux baseline will capture Deck Hearth strings, not the pre-rename TCG Vault strings** (per `.convoys/pick-a-name.md` § Test plan + the architect's `update-seed-visual-baselines-on-linux-ordering` follow-up note). - **`adopt-playwright-smoke`** (priority: P1 quality, also listed as launch sequence step 10 / P1 #10 step 2) — **RESOLVED 2026-05-24**. - **Resolved by:** squash commit `7b6f751` (PR #18, architect-commit `3ac527e`, implementer-commit `c72d006`). Brief 1 shipped as planned with two small lint-baseline-preserving deviations from the brief's verbatim shape (documented in the convoy file's § As-shipped). - **As-shipped surface:** `@playwright/test@^1.60.0` added to `devDependencies`; new `playwright.config.js` at repo root (ESM, two projects partitioned by `testMatch` — `smoke` + `visual`, CI-fail-loud / dev-warn predicate on `VERCEL_AUTOMATION_BYPASS_SECRET` per Decision 2, `snapshotPathTemplate: 'tests/visual/__screenshots__/{arg}{ext}'` aligned with `visual-diff.yml`'s artifact upload path); new `tests/visual/homepage.spec.ts` (1 test, no baseline committed per Decision 4); three new `package.json` scripts (`test:smoke`, `test:visual`, `test:visual:update`); three new `.gitignore` entries (`/playwright-report/`, `/test-results/`, `/.playwright/`). No `eslint.config.mjs` change (Decision 5 + Finding 2 verified clean empirically). No source touched under `pages/**` / `components/**` / `lib/**`. diff --git a/.cursor/rules/api-routes.mdc b/.cursor/rules/api-routes.mdc index c834ca3..fb98844 100644 --- a/.cursor/rules/api-routes.mdc +++ b/.cursor/rules/api-routes.mdc @@ -103,7 +103,7 @@ await logCollectionActivity(collectionId, userId, 'card_added', { cardId, quanti ## Rate limiting -`lib/rate-limit.js` exposes five named limiters, one per route class. Each named export takes `req` (and `userId` for user-keyed classes) and returns `{ allowed, remaining, reset }`. The auth-only limiter shipped in `fix-auth-bypass` Brief 4 (commit `297afca`, login + register); the four remaining classes — `search`, `upload`, `generate`, `import` — and the seven currently-gated routes shipped in `add-rate-limiting` (squash commit `708ef45`, PR #20, 2026-05-24), the convoy that closed P0 #6 and brought the launch-readiness P0 set to 8/8 RESOLVED. +`lib/rate-limit.js` exposes five named limiters, one per route class. Each named export takes `req` (and `userId` for user-keyed classes) and returns `{ allowed, remaining, reset }`. The auth-only limiter shipped in `fix-auth-bypass` Brief 4 (commit `297afca`, login + register); the four remaining classes — `search`, `upload`, `generate`, `import` — and the seven currently-gated routes shipped in `add-rate-limiting` (squash commit `708ef45`, PR #20, 2026-05-24), the convoy that closed P0 #6 and brought the launch-readiness P0 set to 8/8 RESOLVED. The five Redis key prefixes were renamed `tcgvault:*` → `deckhearth:*` in `pick-a-name` (squash commit `9abbab6`, PR #21, 2026-05-24) — call shape, return shape, and gate-ordering rules below are byte-identical post-rename; only the on-Redis namespace changed (one-time per-window counter reset accepted). | Class | Limit | Window | Key | Used by | Helper | | --- | --- | --- | --- | --- | --- | diff --git a/AGENTS.md b/AGENTS.md index b954ea0..a5132ac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,7 +2,15 @@ Guidance for agents and humans working in this repo. Prefer existing patterns over new abstractions. -> Branding note: **Deck Hearth** is the canonical product brand (ratified 2026-05-24 in the `pick-a-name` convoy). The repo directory name `tcg-vault` stays for now — see queued `rename-repo-and-vercel-project`. The seed admin email migrated from `admin@tcgvault.com` to `admin@deckhearth.com` in the same convoy; see Gotcha #4 + `.convoys/pick-a-name.md` for the rotation flow. +> Branding note: this product is **Deck Hearth** as of 2026-05-24 +> (`pick-a-name` convoy, squash commit `9abbab6`, PR #21). The repo +> and Vercel project are still named `tcg-vault` — that rename is +> tracked in the queued `rename-repo-and-vercel-project` convoy +> (auto-redirects make it low-urgency). Admin email is +> `admin@deckhearth.com`; the prior `admin@tcgvault.com` literal is +> deliberately preserved in `test/lib/permission-middleware.test.js` +> as a historical regression-lock per Risk 4 of the pick-a-name +> convoy. ## 1. Project overview @@ -10,7 +18,7 @@ A web app for managing trading-card-game collections (Magic, Pokémon, Lorcana). - **Framework:** Next.js 16 (Pages router) + React 18, JavaScript (not TypeScript — see Gotcha #9) - **Data:** Neon Postgres, accessed two different ways — `@neondatabase/serverless` (`lib/database.js`) AND raw `@vercel/postgres` (`pages/api/**`). Pick ONE; see Gotcha #1. -- **Auth:** Custom JWT (jsonwebtoken + bcryptjs), token stored in `localStorage`, sent as `Authorization: Bearer …`. No NextAuth. The secret + canonical 24h TTL come from `lib/auth-secret.js` (single source of truth; throws at module load if `JWT_SECRET` is unset). `getUserFromRequest` returns `null` for unauthenticated requests — no synthetic admin fallback — and login + register are rate-limited (5 attempts / 15 min via `@upstash/ratelimit`). The seed admin row is created at `admin@tcgvault.com` with a password supplied via the required `ADMIN_INITIAL_PASSWORD` env var (`scripts/setup-neon-db.js` exits with code 1 before touching the DB if the var is unset); no credential ships in the source tree. Operators of envs that pre-date the `drop-public-setup` convoy still have the old `admin123` hash in their DB — rotate manually via the app (see Gotcha #4). +- **Auth:** Custom JWT (jsonwebtoken + bcryptjs), token stored in `localStorage`, sent as `Authorization: Bearer …`. No NextAuth. The secret + canonical 24h TTL come from `lib/auth-secret.js` (single source of truth; throws at module load if `JWT_SECRET` is unset). `getUserFromRequest` returns `null` for unauthenticated requests — no synthetic admin fallback — and login + register are rate-limited (5 attempts / 15 min via `@upstash/ratelimit`). The seed admin row is created at `admin@deckhearth.com` with a password supplied via the required `ADMIN_INITIAL_PASSWORD` env var (`scripts/setup-neon-db.js` exits with code 1 before touching the DB if the var is unset); no credential ships in the source tree. Operators of envs that pre-date the `drop-public-setup` convoy still have the old `admin123` hash in their DB — rotate manually via the app (see Gotcha #4). - **UI:** Tailwind CSS + custom CSS variables for theming (light/dark via `lib/theme-context.js`) - **Hosting:** Vercel (`vercel.json`, `.vercel/` present) @@ -51,6 +59,16 @@ Code graph is indexed by `user-code-review-graph` MCP (122 files, 628 nodes, 560 - **#2 — `getUserFromRequest` synthetic-admin fallback. RESOLVED** by `fix-auth-bypass` Brief 2 (commit `258e479`). The helper now returns `null` for unauthenticated requests; `pages/api/auth/verify.js` returns 401 on the no-token branch. The 16 unit tests in `test/lib/permission-middleware.test.js` lock in the contract, including a negative regression against the old synthetic-admin shape. Entry kept (not renumbered) to preserve the audit trail and stable cross-references. - **#3 — JWT_SECRET hardcoded across 7 files. RESOLVED** by `fix-auth-bypass` Brief 1 (commit `4a10dce`). `lib/auth-secret.js` is now the single source of truth and throws at module load when `JWT_SECRET` is unset. Canonical TTL is `JWT_TOKEN_TTL = '24h'`. The `'your-secret-key-change-in-production'` literal is gone from all 7 sites; CI lint passes against the post-fix tree. Entry kept (not renumbered) to preserve cross-references. - **#4 — Default admin credentials in the seed. RESOLVED** by `drop-public-setup` Brief 1 (commit `ff80753`) + Brief 2 (commit `b63b509`). `scripts/setup-neon-db.js` no longer hardcodes `admin123`; it reads `ADMIN_INITIAL_PASSWORD` from the environment and exits with code 1 before opening a DB connection if the var is unset. README's "Default Admin Account" section is replaced with "First-time admin setup" copy that documents the env var, `openssl rand -base64 24` generation tip, and CI-secret alternative. Brief 2 converted the script from CJS to ESM so `npm run setup-db` actually runs on Node 22.x (the `bump-next-js` convoy's `"type": "module"` flag had silently broken it). **Operator caveat:** the seed is idempotent (`ON CONFLICT (email) DO NOTHING`); re-running setup-db on an env that already has the admin row does NOT rotate the password. Any deployed env that ran setup before this convoy still has the weak `admin123` hash — operators must rotate manually via the app, or wait for the queued `rotate-default-admin` follow-up convoy. Entry kept (not renumbered) to preserve cross-references. + + Post-`pick-a-name` (2026-05-24, squash `9abbab6`), the seeded admin + email is `admin@deckhearth.com` (and alice/bob test users likewise + renamed). **If you are deploying past `9abbab6` and the prod Neon + DB still has `@tcgvault.com` rows, you MUST run + `node scripts/migrations/2026-05-24-rename-admin-email.js` BEFORE + the next admin login attempt** or it 401s. The migration is ESM, + idempotent, UNIQUE-collision-safe (fails loud if `setup-neon-db.js` + already ran post-rename — which would indicate an ordering error). + Order: migration FIRST, then any subsequent `npm run setup-db`. - **#5 — `pages/api/setup-database.js` public endpoint. RESOLVED** by `fix-auth-bypass` Brief 3 (commit `fc0dd73`). The file is deleted along with the other three dev endpoints (`/api/simple`, `/api/test-auth`, `/api/test-db`), and `.github/workflows/ci.yml`'s new `forbidden-endpoints` job fails the build if any of them are re-introduced (or if a new `pages/api/test-*.js` file appears). Entry kept (not renumbered) to preserve cross-references. - **#6 — Migrations are bare scripts.** `scripts/add-*.js` and `scripts/fix-*.js` are run-once jobs with no idempotency tracking. Adopt `node-pg-migrate`, `kysely`, or `drizzle-kit` before more schema changes. - **#7 — Dual `is_public` semantics.** Collections and decks both have `is_public` columns; check which controls discovery vs. anonymous read in the relevant route. @@ -62,13 +80,15 @@ Code graph is indexed by `user-code-review-graph` MCP (122 files, 628 nodes, 560 **Milestone — `add-rate-limiting` convoy (squash `708ef45`, PR #20, 2026-05-24) closed P0 #6 — all 8 P0s now RESOLVED.** The lib refactored from a single auth-only limiter to **5 named limiters** with a `Map` cache (one shared Redis client, five `Ratelimit` instances, distinct Redis prefix per class). The five exports + their use cases: - | Helper | Class | Limit/window | Key | Routes | - | --- | --- | --- | --- | --- | - | `checkAuthRateLimit(req)` | `auth` | 5 / 15 min | IP | `/api/auth/login`, `/api/auth/register` (Brief 4 contract; byte-identical return shape preserved) | - | `checkSearchRateLimit(req)` | `search` | 60 / 1 min | IP | `/api/users/search`, `/api/cards/search` | - | `checkUploadRateLimit(req, userId)` | `upload` | 10 / 1 hour | user | `/api/user/avatar` | - | `checkGenerateRateLimit(req, userId)` | `generate` | 5 / 1 hour | user | `/api/user/avatar/generate` | - | `checkImportRateLimit(req, userId)` | `import` | 5 / 1 hour | user (admin-only) | `/api/cards/import-mtg`, `/api/cards/import-pokemon`, `/api/cards/import-lorcana` | + | Helper | Class | Limit/window | Key | Redis prefix | Routes | + | --- | --- | --- | --- | --- | --- | + | `checkAuthRateLimit(req)` | `auth` | 5 / 15 min | IP | `deckhearth:auth` | `/api/auth/login`, `/api/auth/register` (Brief 4 contract; byte-identical return shape preserved) | + | `checkSearchRateLimit(req)` | `search` | 60 / 1 min | IP | `deckhearth:search` | `/api/users/search`, `/api/cards/search` | + | `checkUploadRateLimit(req, userId)` | `upload` | 10 / 1 hour | user | `deckhearth:upload` | `/api/user/avatar` | + | `checkGenerateRateLimit(req, userId)` | `generate` | 5 / 1 hour | user | `deckhearth:generate` | `/api/user/avatar/generate` | + | `checkImportRateLimit(req, userId)` | `import` | 5 / 1 hour | user (admin-only) | `deckhearth:import` | `/api/cards/import-mtg`, `/api/cards/import-pokemon`, `/api/cards/import-lorcana` | + + Prefixes renamed `tcgvault:*` → `deckhearth:*` in `pick-a-name` (squash `9abbab6`, 2026-05-24); accepted one-time per-15-min / per-1-hour counter reset; existing Upstash state at `tcgvault:*` keys is now stale and will TTL out naturally. All five return the same `{ allowed, remaining, reset }` shape; on `!allowed`, set `Retry-After: Math.ceil((reset - Date.now()) / 1000)` and return 429 with the uniform message `'Too many attempts. Try again later.'` (per-class variation would fingerprint the limits to an attacker — explicitly rejected).