Reflects the merged drop-public-setup convoy (PR #13) in repo documentation. Small focused pass; no behavior changes. AGENTS.md: - § 1 auth bullet: replace "seed admin row still ships in setup-neon-db.js" claim with the new env-var-gated reality and the R1 operator-rotation caveat. - § 4 Gotcha #4: mark RESOLVED with commit refs (ff80753+b63b509), document the as-shipped behavior, the Brief 2 CJS→ESM Node 22.x fix, and the R1 operator caveat. Entry kept (not renumbered) per the same convention used for resolved gotchas #2, #3, #5. - § 5 Running locally: add ADMIN_INITIAL_PASSWORD to the env-var template list with a note that setup-db exits 1 if it's unset. .convoys/ship-readiness.md: - P0 #3: mark RESOLVED 2026-05-23 with commit refs, document the full as-shipped behavior including Brief 2's CJS→ESM bonus, the R1 operator caveat (Decision A — going-forward only), and the deferred sibling weak-cred references queued for purge-weak-creds-from-helpers. .cursor/rules/no-go-zones.mdc: - Editing rules of thumb: clarify the schema-vs-operational distinction for scripts/setup-neon-db.js. drop-public-setup set the precedent that operational changes (env-var gating, pre-flight validation, module-system fixes) are allowed in place, while DDL changes still need a separate migration script. Documented so future agents don't have the same confusion the drop-public-setup architect did (see Decision B in .convoys/drop-public-setup.md). No changes to: package.json, lib/**, pages/**, components/**, scripts/**, .github/**, README.md (already updated in PR #13). Co-authored-by: Cursor <cursoragent@cursor.com>
272 lines
27 KiB
Markdown
272 lines
27 KiB
Markdown
---
|
||
name: ship-readiness
|
||
classification: epic
|
||
success_metric: tcg-vault is safe to expose to anonymous internet traffic with a documented launch checklist green
|
||
skip: []
|
||
status: open
|
||
created: 2026-05-22
|
||
---
|
||
|
||
# Ship-readiness convoy
|
||
|
||
Umbrella convoy capturing the full agent-pipeline review of tcg-vault as of 2026-05-22. Findings are grouped by L2 role lens (Reviewer / Architect / Design-system / A11y / IA / Doc-writer) and severity. Each item points to the convoy that will execute the fix.
|
||
|
||
Code graph: 122 files, 628 nodes, 5602 edges, 11 communities. Indexed by `user-code-review-graph` MCP.
|
||
|
||
## P0 — ship-blockers (security)
|
||
|
||
These MUST land before any anonymous traffic touches the production URL.
|
||
|
||
### 1. `getUserFromRequest` returns a hardcoded admin when no Bearer token is present — **RESOLVED 2026-05-23**
|
||
|
||
- **Resolved by:** `fix-auth-bypass` Brief 2, commit `258e479` (PR #8). Follow-up Brief 6 hotfix `1fca3aa` added explicit 401 guards to the cards-collection POST/PUT/DELETE branches that previously masked the bug as 500s.
|
||
- **File:** `lib/permission-middleware.js` lines 13-17.
|
||
- **Impact:** Every API route that calls `getUserFromRequest` (30+ handlers — see `user-code-review-graph` cross-community edges from `api-handler` → `lib-admin`) accepts unauthenticated requests as admin user 1.
|
||
- **Repro:** `curl https://<host>/api/collections` with no `Authorization` header returns admin's collections.
|
||
- **Fix:** Delete lines 13-17. Return `null` when no Bearer token. Update every caller to handle `null` properly (most already do; the broken fallback was masking the right path).
|
||
- **As-shipped:** The helper now returns `null` for any unauthenticated request. 16 unit tests in `test/lib/permission-middleware.test.js` lock in the contract (including a negative regression against the old synthetic-admin shape). `pages/api/auth/verify.js` returns 401 on the no-token branch instead of fetching the seed admin row.
|
||
- **Owns:** `role-architect` + `role-implementer` (one PR; small surface area in the helper, callers already check `!user`).
|
||
|
||
### 2. JWT_SECRET hardcoded fallback in 7 files — **RESOLVED 2026-05-23**
|
||
|
||
- **Resolved by:** `fix-auth-bypass` Brief 1, commit `4a10dce` (PR #7).
|
||
- **Files:**
|
||
- `pages/api/auth-utils.js` (`'your-secret-key'`)
|
||
- `pages/api/auth/login.js`, `pages/api/auth/register.js`, `pages/api/auth/verify.js`
|
||
- `pages/api/favorites.js`, `pages/api/users/search.js`
|
||
- `lib/permission-middleware.js`
|
||
- **Impact:** If `JWT_SECRET` env var is unset (e.g. preview/staging misconfig), tokens are signed with `'your-secret-key-change-in-production'` — an attacker can sign their own admin token in 5 seconds.
|
||
- **Fix:** Centralize JWT_SECRET access in one helper that `throw`s at module load if `process.env.JWT_SECRET` is unset. Every other file imports from there.
|
||
- **Bonus:** Token expiry is inconsistent (`/api/auth/login.js` uses 24h, `pages/api/auth-utils.js` uses 7d). Pick one.
|
||
- **As-shipped:** `lib/auth-secret.js` is the single source of truth and throws at module load if `JWT_SECRET` is unset. Canonical TTL is `JWT_TOKEN_TTL = '24h'`. All 7 literal fallback sites are converted to import-and-throw. `test/lib/auth-secret.test.js` (3 tests) covers the fail-loud path.
|
||
- **Owns:** `role-architect` + `role-implementer`.
|
||
|
||
### 3. Default admin credentials in seed + README — **RESOLVED 2026-05-23**
|
||
|
||
- **Resolved by:** `drop-public-setup` Brief 1 (commit `ff80753`) + Brief 2 (commit `b63b509`). PR #13.
|
||
- **Files:**
|
||
- `scripts/setup-neon-db.js` lines 130-138 — creates `admin@tcgvault.com` / `admin123`
|
||
- `README.md` documents the credentials
|
||
- `pages/api/setup-database.js` — duplicates the setup AND is an UNAUTHENTICATED public POST endpoint with `Access-Control-Allow-Origin: *`
|
||
- **Impact:** Anyone who hits `/api/setup-database` can re-trigger DDL. The `admin123` password is one Google away from public knowledge.
|
||
- **Fix:**
|
||
1. Delete `pages/api/setup-database.js`. Schema setup is a one-time job; it should not be a route.
|
||
2. Change `setup-neon-db.js` to require a `ADMIN_INITIAL_PASSWORD` env var (no default).
|
||
3. Strip the admin password from README — replace with "run `npm run setup-db` and follow the prompt".
|
||
- **As-shipped:**
|
||
1. `pages/api/setup-database.js` already deleted by `fix-auth-bypass` Brief 3 (commit `fc0dd73`); the `forbidden-endpoints` CI job blocks re-introduction.
|
||
2. `scripts/setup-neon-db.js` now reads `ADMIN_INITIAL_PASSWORD` from `process.env`; if unset or empty, the script writes an actionable error (names the env var, points at `.env.local`, suggests `openssl rand -base64 24`, mentions CI-secret alternative, references README) and exits with code 1 **before** opening any DB connection. The bcrypt input is the env-var value, not the literal `admin123`. The two `console.log` lines that previously echoed `Admin User: admin@tcgvault.com` + `Admin Password: admin123` are deleted (R3 — stdout-leak prevention into CI logs); replaced with a single `Admin user ready (email: admin@tcgvault.com)` line that does NOT echo the password.
|
||
3. `README.md`'s "Default Admin Account" section replaced with "First-time admin setup" copy that documents the env-var requirement, the `openssl rand -base64 24` generation tip, the CI-secret alternative, and an operator-rotation note for envs that pre-date this convoy.
|
||
4. **Bonus (Decision D, Brief 2):** `scripts/setup-neon-db.js` converted from CommonJS to ESM so `npm run setup-db` actually executes on Node 22.x. The `bump-next-js` convoy added `"type": "module"` to `package.json` for ESLint v9 flat config; the seed script's `require()` calls were silently broken since that landed. Without Brief 2, Brief 1's env-var gate would have been theatrical (script throws `ReferenceError` before reaching the gate).
|
||
- **Operator caveat (R1, Decision A — going-forward only):** the seed is idempotent (`ON CONFLICT (email) DO NOTHING`); re-running `npm run 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 in its DB — operators must rotate manually via the app's profile settings, or wait for the queued `rotate-default-admin` follow-up convoy. Documented in `AGENTS.md` Gotcha #4 and the README's First-time admin setup blockquote.
|
||
- **Sibling weak-cred references deferred:** `scripts/reset-db.js`, `scripts/create-test-users.js`, and `TESTING_GUIDE.md` still hardcode `admin@tcgvault.com` / `admin123` — out of scope here per the no-go-zones rule (historical scripts) and the convoy spec. Queued for `purge-weak-creds-from-helpers` follow-up (or fold into `pick-a-name` since the email is also changing).
|
||
- **Owns:** `role-implementer`.
|
||
|
||
### 4. Dev-only test endpoints shipped to production — **RESOLVED 2026-05-23**
|
||
|
||
- **Resolved by:** `fix-auth-bypass` Brief 3, commit `fc0dd73` (PR #6).
|
||
- **Files:** `pages/api/simple.js`, `pages/api/test-auth.js`, `pages/api/test-db.js`, `pages/api/setup-database.js`.
|
||
- **Impact:** Unknown — depends on what they expose. `/api/test-db` likely returns the DB connection string; `/api/test-auth` may leak token-handling details.
|
||
- **Fix:** Delete all four. Add a CI grep that fails the build if any file matching `pages/api/(test-|simple|setup-)*.js` exists.
|
||
- **As-shipped:** All four files deleted. `.github/workflows/ci.yml` has a new `forbidden-endpoints` job (blocking) that fails the build if any of the four paths reappear OR if a new `pages/api/test-*.js` file is added. Local simulation in the implementer PR confirmed clean → OK, with `test-fake.js` → FAIL, post-cleanup → OK.
|
||
- **Owns:** `role-implementer`.
|
||
|
||
### 5. CORS `Access-Control-Allow-Origin: *` on auth endpoints — **PARTIAL 2026-05-23**
|
||
|
||
- **Partially resolved by:** `fix-auth-bypass` Brief 4, commit `297afca` (PR #9). Login + register only; `pages/api/auth/verify.js` is **deferred** to the queued `cors-tighten` follow-up convoy.
|
||
- **Files:** at minimum `pages/api/auth/login.js`, `pages/api/auth/register.js`, `pages/api/setup-database.js` (verify others).
|
||
- **Impact:** Any origin can submit credentials. Combined with the no-rate-limit problem below, credential stuffing is wide open.
|
||
- **Fix:** Set `Access-Control-Allow-Origin` to the literal frontend origin (`https://tcgvault.com` / preview domain), or remove the header entirely if the API and the frontend are same-origin (they are, on Vercel).
|
||
- **As-shipped:** `pages/api/auth/login.js` and `pages/api/auth/register.js` drop the four `setHeader` calls + the OPTIONS preflight handler. `pages/api/setup-database.js` was deleted entirely by Brief 3. `pages/api/auth/verify.js` still has the wildcard header — see follow-up convoy `cors-tighten`.
|
||
- **Owns:** `role-implementer`.
|
||
|
||
### 6. No rate limiting anywhere — **PARTIAL 2026-05-23**
|
||
|
||
- **Partially resolved by:** `fix-auth-bypass` Brief 4, commit `297afca` (PR #9). Login + register only; the rest of the listed endpoints are **deferred** to the queued `add-rate-limiting` convoy.
|
||
- **Impact:** Login endpoint accepts unlimited attempts; card-search endpoint can be hammered; image upload endpoints can be exhausted. The `pages/api/cards/import-*.js` endpoints externally hit Scryfall/Pokémon APIs with no caller throttling.
|
||
- **Fix:** Adopt `@upstash/ratelimit` (free tier covers a small launch) or Vercel's built-in middleware-based rate limiting. Apply to: `/api/auth/login`, `/api/auth/register`, `/api/users/search`, `/api/cards/search`, all `/api/cards/import-*`, and `/api/user/avatar*` (upload).
|
||
- **As-shipped:** `lib/rate-limit.js` (new) provides `checkAuthRateLimit(req)` via `@upstash/ratelimit@^2.0.8` + `@upstash/redis@^1.38.0` (5 attempts / 15-min sliding window per IP). Wired into login + register. Env vars are `KV_REST_API_URL` / `KV_REST_API_TOKEN` (auto-provisioned by Vercel's Upstash Marketplace integration — note this is a rename from the brief's original `UPSTASH_REDIS_REST_*` spec; see `.convoys/fix-auth-bypass/brief-4-tighten-auth-surface.md` § Post-merge addendum). Fails closed in prod when env vars are unset; warn-and-no-ops in dev. Search / import / avatar endpoints are unchanged.
|
||
- **Owns:** `role-architect` (pattern) → `role-implementer` (per-route).
|
||
|
||
### 7. Layout default-prop leaks maintainer email
|
||
|
||
- **File:** `components/Layout.js` line 562: `function Layout({ children, user = { email: 'me@randallstillwell.com', role: 'user' }, ... })`.
|
||
- **Impact:** Any page that renders Layout without passing a `user` prop displays your real email and impersonates you as the logged-in user.
|
||
- **Fix:** Default `user = null` and render a logged-out state branch. Verify every page passes `user` explicitly (the graph shows ~13 pages call `Layout`; audit each).
|
||
- **Owns:** `role-implementer`.
|
||
|
||
### 8. Next.js 15.4.3 — Vercel platform blocks deploys (vulnerable version)
|
||
|
||
- **Discovered:** 2026-05-22 during the bootstrap PR CI run. Vercel build completes successfully (~29s) but the deployment exits with status `Error` and `"Vulnerable version of Next.js detected, please update immediately"`.
|
||
- **Files:** `package.json` line 22 (`"next": "^15.4.2"` → locked at `15.4.3`), `package-lock.json`.
|
||
- **Impact:** **Vercel will not deploy any branch — including `main` — until Next.js is bumped.** Preview URLs are unavailable, which means `preview-smoke.yml` and `visual-diff.yml` can't fire. The last successful deploy on `main` was 2025-08-01; production may already be running an outdated build.
|
||
- **CVE context:** Next.js shipped a middleware auth-bypass advisory (CVE-2025-29927) patched in 15.2.3, plus subsequent advisories. The exact CVE Vercel is flagging on 15.4.3 needs confirmation via `npm audit` and the Next.js security advisory page.
|
||
- **Fix:** Bump `next` to the latest secure 15.x (`npm install next@^15.5` and run smoke tests) OR the latest 16.x (`next@^16.2.6` — major bump; review breaking changes in [Next.js 16 release notes](https://nextjs.org/blog/next-16)).
|
||
- **Owns:** `role-architect` (pick target version + assess breaking changes) → `role-implementer` (bump + verify dev/build/start + smoke).
|
||
- **Convoy:** `bump-next-js` — runs before `fix-auth-bypass` lands, OR in parallel as a separate PR. **Without this convoy, every L3 gate that depends on a Vercel preview is non-functional.**
|
||
|
||
## P1 — pre-launch quality bar
|
||
|
||
### 8. Two SQL clients in parallel (`@neondatabase/serverless` + `@vercel/postgres`)
|
||
|
||
- **Impact:** Two different param-handling APIs, two different transaction stories, two different connection-pool stories. Plus `lib/database.js`'s manual interpolation + `sql.unsafe(query)` is a SQL-injection vector if any caller passes user input through.
|
||
- **Fix:** Pick `@vercel/postgres` (tagged-template, no injection vector). Migrate every call site of `lib/database.js::db.query`. Delete `lib/database.js`.
|
||
- **Reviewer/Architect call:** small enough to fit in one convoy; touches ~3 files based on graph.
|
||
|
||
### 9. Three parallel client-side auth implementations
|
||
|
||
- **Files:** `lib/auth-context.js` (`AuthProvider` / `useAuth`), `lib/admin-auth.js` (`AdminProvider` / `useAdmin` / `useIsAdmin`), `lib/use-auth.js` (`useAuth`).
|
||
- **Impact:** Pages randomly import from one of three places. State is duplicated. Logout in one provider doesn't necessarily clear the others. Token-verify roundtrips happen 3× on initial page load if all three providers mount.
|
||
- **Fix:** Collapse to `lib/use-auth.js` as the canonical hook. Migrate every importer. Delete `auth-context.js` and `admin-auth.js`. Roll up `useIsAdmin` semantics into `useAuth().user?.role === 'admin'`.
|
||
- **Owns:** `role-architect` (decision) → `role-implementer` (per-page migration; ~30 importers).
|
||
|
||
### 10. No tests
|
||
|
||
- **Impact:** The first agent-driven refactor of `getUserFromRequest` (P0 #1) is high-blast-radius with no safety net.
|
||
- **Fix sequence:**
|
||
1. Install `vitest`. Add `npm run test:run` script.
|
||
2. Install `@playwright/test`. Wire up `tests/smoke/app.smoke.spec.ts` (already drafted; needs `playwright.config.ts`).
|
||
3. Re-enable the `test:` job in `.github/workflows/ci.yml` (commented out at install time).
|
||
4. Add unit tests for `lib/permission-middleware.js`, `lib/slug-utils.js`, `pages/api/auth-utils.js`.
|
||
5. Wire `preview-smoke.yml` to run against the Vercel preview URL.
|
||
- **Owns:** `role-architect` (test strategy) → `role-implementer` (initial suite).
|
||
|
||
### 11. No migration tool — `scripts/add-*.js` graveyard
|
||
|
||
- **Files:** 27 scripts in `scripts/` of the form `add-foo-column.js`, `fix-bar-constraint.js`, `seed-baz.js`. No idempotency tracking, no `schema_migrations` table, no rollback.
|
||
- **Impact:** Onboarding a new env requires re-running every script in the right order. No way to know what's been run on a given Neon branch. Every new column is at risk of being missed in prod.
|
||
- **Fix:** Adopt `node-pg-migrate` (lightweight, matches the existing pattern best) OR migrate to `drizzle-kit` if the team wants schema-as-code. Backfill a single "initial" migration matching current prod schema. From there, every new column ships as a migration file.
|
||
- **Owns:** `role-architect` (tool selection) → `role-implementer` (backfill + first new migration).
|
||
|
||
### 11.5. Codebase has ~100 pre-existing ESLint errors
|
||
|
||
- **Discovered:** 2026-05-22 during the bootstrap PR. The repo had `"lint": "next lint"` in `package.json` but no `.eslintrc.json` — meaning lint was never run. Bootstrap added the config; lint now surfaces ~100 errors.
|
||
- **Most serious:** `react-hooks/rules-of-hooks` violations (hooks called conditionally) in several components. These are **real bugs** — React's hook ordering is undefined when hooks are called after early returns. They likely manifest as state-loss / stale-closure bugs in edge cases.
|
||
- **Less serious:** `react/no-unescaped-entities` (cosmetic), `react-hooks/exhaustive-deps` (warnings about missing useEffect deps), `@next/next/no-img-element` (cosmetic).
|
||
- **Impact:** The L3 CI lint job is currently `continue-on-error: true` (see `.github/workflows/ci.yml`) so it doesn't block PRs. Lint output is visible in logs but PRs merge regardless of lint state until this is cleaned up.
|
||
- **Fix:** Triage each error. The rules-of-hooks ones need genuine code restructuring (move hooks before any early returns). The unescaped-entities are mechanical (`'` → `'`). After cleanup, remove `continue-on-error: true`.
|
||
- **Convoy:** `fix-lint-baseline` — run after `fix-auth-bypass` and `drop-public-setup`. Multitask-safe: split into briefs by file group.
|
||
- **Owns:** `role-architect` (group strategy) → `role-implementer` (per-group fan-out).
|
||
|
||
### 12. Branding mismatch — "TCG Vault" vs. "Deck Hearth"
|
||
|
||
- **Files:** README, `package.json`, seed data say "TCG Vault" / `admin@tcgvault.com`. `components/Layout.js` lines 596 + 689 render "Deck Hearth" + "DH" logo. The `.env.local` template, `vercel.json`, and Vercel project name should also be audited.
|
||
- **Impact:** Confusing for users. Confusing for marketing. Confusing for analytics. Pick one.
|
||
- **Fix:** Brand workshop → final name → global replace → update README, package.json `"name"`, every UI string, Vercel project name, email sender, support pages. Schedule a redirect from the old domain.
|
||
- **Owns:** `role-ia-architect` (which name? — needs human decision) → `role-implementer`.
|
||
|
||
## P2 — refactor priorities
|
||
|
||
### 13. God components (10 files over 500 lines)
|
||
|
||
| File | Lines | Notes |
|
||
| --- | --- | --- |
|
||
| `pages/cards.js` | 1499 | `AuthenticatedCards` (886) + `Card3D` (502) live in one file. Split into `pages/cards/index.js` + `components/Card3D.js`. |
|
||
| `pages/collection/[identifier].js` | 1044 | `CollectionView` is one mega-component. Extract: header, card-grid, share-modal-wrapper, edit-form. |
|
||
| `pages/collections.js` | 989 | Similar structure to collection/[identifier]. Possibly share extracted pieces. |
|
||
| `pages/card/[id].js` | 913 | `CardDetail` — split into header, owned-badge, add-to-collection-flow. |
|
||
| `pages/deck-builder.js` | 823 | `DeckBuilder` — extract card-search, deck-list, mana-curve panels. |
|
||
| `components/CameraScanner.js` | 817 | Camera + AI-OCR + detection-loop — extract the detection loop into a hook. |
|
||
| `pages/admin/card-editor.js` | 778 | Form heavy. Use a `useFormState` pattern + separate the search-results subview. |
|
||
| `pages/scanner.js` | 776 | Mirror of CameraScanner concerns plus queue management. |
|
||
| `pages/settings.js` | 669 | One screen per settings section is the usual fix. |
|
||
| `pages/profile.js` | 625 | Avatar generation logic alone is ~150 lines — extract `useGeneratedAvatar` hook. |
|
||
|
||
Each is one convoy of its own. Use the architect role's `slice_dependencies:` to fan out implementers safely.
|
||
|
||
### 14. Schema-design smells (documented in `docs/SCHEMA_MAP.md`)
|
||
|
||
- `users` has two avatar columns (`profile_image_url` + `avatar_url`). Reconcile.
|
||
- `collections` has two visibility flags (`is_public BOOLEAN` + `visibility VARCHAR`). Reconcile.
|
||
- `cards.quantity` + `cards.favorited` are unused (they belong on `user_cards` / `user_favorites`). Drop.
|
||
- `user_settings` table duplicates several `users` columns. Reconcile.
|
||
- All enum-shaped VARCHARs (`role`, `condition`, `theme`, `game`, `visibility`) should be CHECK-constrained or proper Postgres ENUMs.
|
||
- `collections.tags` is `TEXT` (comma-separated). Migrate to `JSONB` or a join table.
|
||
|
||
### 15. Component coupling warning from graph
|
||
|
||
`user-code-review-graph` flagged:
|
||
- High coupling (44 edges) between `components-handle` and `pages-handle` (largely `Layout`, `CardItem`, `ManaCost` — expected for a shared UI surface).
|
||
- High coupling (34 edges) between `lib-admin` and `api-handler` — almost all via `getUserFromRequest`. After P0 #1 is fixed, this number stays high because the auth check is genuinely shared — that's fine.
|
||
|
||
### 16. Lots of inline SVG and emoji
|
||
|
||
The `getIcon` registry in `Layout.js` and `MobileNavigation.js` redefines the same SVG paths. Extract to `components/icons/` with named exports. Then audit the codebase for inline SVG that should be a named import. Bonus: lazy-load the larger icon families.
|
||
|
||
## P3 — UX, IA, design-system
|
||
|
||
### Role-ia-architect findings
|
||
|
||
- **URL structure** — solid. `/cards`, `/collections`, `/collection/[slug]`, `/deck-builder`, `/community/collections`. Coherent. One quirk: `/card/[id]` (singular) for detail vs. `/cards` (plural) for index — typical Next.js shape but worth a redirect rule so `/cards/[id]` also resolves.
|
||
- **Logged-out homepage** — current `pages/index.js` is 316 lines; needs an editorial pass. What's the value prop in one sentence? Right now it's mostly "we have cards".
|
||
- **Onboarding** — signup → profile setup → first collection → scan-or-import card. Currently each step is a separate page. Consider a multi-step wizard at `/onboarding` to keep the new user in flow.
|
||
- **Discoverability** — `/community/decks` and `/community/forums` are in the nav but flagged as placeholders. Either ship the MVP for each before launch (forums likely too big) or hide the nav items until they exist.
|
||
|
||
### Role-ux-reviewer findings
|
||
|
||
- **Loading states** — most data fetches set `loading: true` then re-render; very few show skeletons. Card grids should use shimmer placeholders; modals should disable submit while in flight.
|
||
- **Error states** — error messages bubble to `console.error` and toast nothing. Add a global toast system (e.g. `sonner`) and wire every catch block.
|
||
- **Empty states** — `/my-cards` and `/collections` when empty drop to "no cards yet". Replace with first-time CTA: "Scan your first card" or "Browse popular sets".
|
||
- **Mobile drawer** — `MobileNavigation` is solid (recent commit `442e906`). One thing: the bottom-bar's active state contrast looks low in light mode; verify against AA.
|
||
- **Camera scanner UX** — 817 lines of detection loop. Add a one-line "scanning…" status under the viewfinder and a single "captured N cards" badge. The current toolbar is busy.
|
||
|
||
### Role-design-system-auditor findings
|
||
|
||
- **Two visual languages mixing** — Tailwind classes AND CSS variables on the same elements. This is documented in `.cursor/rules/ui-and-theming.mdc`; the cleanup is to define which property goes where and enforce.
|
||
- **Hardcoded hex colors** — grep for `bg-\[#` and `style={{ backgroundColor: '#`. There are still a handful; convert to theme tokens.
|
||
- **Logo + brand** — see P1 #12. Then once the name is settled, the "DH" logo + AnimatedFireLogo need to be unified into one brand mark.
|
||
- **Modal patterns** — `CollectionSelectionModal`, `ShareModal`, `UploadImageModal` each have their own backdrop + focus-trap implementation. Extract `<Modal>` primitive. Use `headlessui` or `radix-ui`'s Dialog to get focus management for free.
|
||
- **Card grid spacing + density** — `pages/cards.js` (the 1499-line monster) does responsive grid math inline. Extract a `<CardGrid>` component that handles density (compact / comfortable / spacious) + sort + filter chrome.
|
||
|
||
### Role-a11y-auditor findings
|
||
|
||
- **Focus traps in modals** — none of the modals trap focus. Tab through `ShareModal` and you leave to the background. Critical for keyboard users + screen readers.
|
||
- **ESC to close modals** — inconsistent. Some have it, some don't.
|
||
- **Skip-to-content** — no `<a href="#main" class="sr-only focus:not-sr-only">`. Add to `_app.js`.
|
||
- **Image alts** — card images use `alt={card.name}` (good); avatar images sometimes have empty alts. Audit.
|
||
- **Color contrast** — verify the muted text colors (`var(--text-secondary)`) hit AA on both themes. The mobile bottom-bar inactive state is a likely fail.
|
||
- **Form errors** — login/signup form errors are visually red but not connected to inputs via `aria-describedby`. Screen readers don't know which field failed.
|
||
- **Keyboard ops on non-button elements** — most clickable `<div>`s already have `onKeyDown` but a few don't (audit with `rg "onClick" components pages | rg -v "<button"`).
|
||
|
||
### Role-doc-writer findings
|
||
|
||
- **README** — needs a public-facing rewrite. Currently mixes user docs + dev setup + admin credentials. Split into `README.md` (project landing) + `docs/DEVELOPMENT.md` (dev setup) + delete the admin credentials section entirely.
|
||
- **`docs/SCHEMA_MAP.md`** — installed at bootstrap (this convoy). Keep it fresh on every schema change.
|
||
- **CHANGELOG** — none yet. Adopt Keep-a-Changelog format. Backfill `[0.1.0] — initial private alpha` covering everything to date.
|
||
- **`TESTING_GUIDE.md`** — currently the only test doc; rename to `docs/MANUAL_QA.md` once `vitest` + `playwright` land.
|
||
- **`docs/API_REFERENCE.md`** — would help. Could be auto-generated by walking `pages/api/**/*.js` and extracting JSDoc; or hand-curated to start.
|
||
- **Privacy policy / Terms of service** — required before public launch. Use a template (Termly / Iubenda) and customize.
|
||
|
||
## Proposed launch sequence
|
||
|
||
Each phase is one Conductor-created convoy. Don't run more than two in parallel until tests exist.
|
||
|
||
0. **`bump-next-js`** (P0 #8). One PR. **MUST land first** — Vercel is currently blocking all deployments, which makes every other PR's preview-smoke / visual-diff gate non-functional. Trivial bump; risk is breaking changes if going to 16.x.
|
||
1. **`fix-auth-bypass`** (P0 #1, #2, #4, #5, #6 partial). One PR. Highest risk; needs human review.
|
||
2. **`drop-public-setup`** (P0 #3, #4). One PR. Trivial; do as a hotfix.
|
||
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.
|
||
5. **`pick-a-name`** (P1 #12). Human decision first, then one or two PRs.
|
||
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.
|
||
9. **`single-auth-provider`** (P1 #9). 3-5 PRs via multitask.
|
||
10. **`adopt-playwright-smoke`** (P1 #10 step 2). One PR.
|
||
11. **`schema-cleanup`** (P2 #14). Multi-PR convoy via multitask.
|
||
12. **`god-component-split`** (P2 #13). One convoy per file; fan out via multitask once architect's `slice_dependencies` are written.
|
||
13. **`launch-polish`** (P3). UX/IA/a11y/docs convoy.
|
||
|
||
Total: ~14 convoys to get from current state to public-launch-ready. Estimate 4-8 weeks at one human-in-the-loop reviewer per convoy. Multitask + Cursor 3.2 worktrees compress steps 8-12 substantially.
|
||
|
||
## Self-analytics
|
||
|
||
After each convoy, `scripts/log-convoy-event.sh` emits a record to `.convoys/.metrics.jsonl` (gitignored). After 3-5 convoys, run the upstream `agent-pipeline/analytics/` aggregator to see where token spend goes — that data feeds whether to add or remove rules.
|
||
|
||
## How to start
|
||
|
||
Per `.cursor/agents/role-conductor.md`, start the next convoy with:
|
||
|
||
> *"Run role-conductor: start a new convoy `fix-auth-bypass` to address P0 #1, #2, #4, #5, #6 partial in `.convoys/ship-readiness.md`. Success = `getUserFromRequest` returns null for missing tokens; no API route accepts unauthenticated requests; CI green."*
|
||
|
||
The Conductor will set classification, skip flags, and hand off to subsequent roles.
|