docs: post-convoy cleanup for fix-reset-db-script

Fill in fix-reset-db-script.md § As-shipped with the merged-state record
(PR #25, squash 3ab9bf8, +161/-18) — three bugs atomically resolved by
mirroring setup-neon-db.js post-drop-public-setup shape; CI green
including 5th-consecutive smoke-spec cross-validation; lint-against-
cjs-in-esm-scripts surfaced as new P3 queue entry.

Update ship-readiness.md Queued convoys:
- convert-reset-db-to-esm → RESOLVED (audit-trail kept, not removed)
- purge-weak-creds-from-helpers → scope reduced (reset-db.js half done;
  create-test-users.js + TESTING_GUIDE.md remain)
- lint-against-cjs-in-esm-scripts → NEW (P3 polish; would have caught
  both DPS-B2 and PR #25 at lint time)

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Randall Stillwell 2026-05-26 22:10:13 -05:00
parent 3ab9bf840c
commit 5f1fa60cba
2 changed files with 90 additions and 4 deletions

View file

@ -1,6 +1,6 @@
# fix-reset-db-script (P2 hygiene — fold of two queued follow-ups) # fix-reset-db-script (P2 hygiene — fold of two queued follow-ups)
**Status:** in-progress **Status:** RESOLVED 2026-05-26 (PR #25, squash commit `3ab9bf8`)
**Priority:** P2 hygiene (not a security blocker; `npm run reset-db` is dev-only **Priority:** P2 hygiene (not a security blocker; `npm run reset-db` is dev-only
and currently broken on Node 22, so blast radius is low — but the bug and currently broken on Node 22, so blast radius is low — but the bug
pattern is the same as the P0-grade weak-creds shape that pattern is the same as the P0-grade weak-creds shape that
@ -8,6 +8,7 @@ pattern is the same as the P0-grade weak-creds shape that
**Convoy owner:** parent (no architect — fold of two well-scoped **Convoy owner:** parent (no architect — fold of two well-scoped
follow-ups; single-file fix following an established proven pattern) follow-ups; single-file fix following an established proven pattern)
**Opened:** 2026-05-25 **Opened:** 2026-05-25
**Merged:** 2026-05-26
## Problem (3 bugs in 1 file) ## Problem (3 bugs in 1 file)
@ -121,4 +122,88 @@ subagent required).
## As-shipped ## As-shipped
(To be appended post-merge.) Single squash commit `3ab9bf8` (PR #25, merged 2026-05-26). Parent-owned
end-to-end per the convoy spec — no architect, no implementer subagent
dispatched. Mirror-the-pattern fix exactly as planned; no mid-execution
surprises that would have forced an architect bounce.
**Diff: 2 files, +161 / -18.** `scripts/reset-db.js` +55 / -18 (the
actual fix); `.convoys/fix-reset-db-script.md` +124 (the planning
document, committed atomically with the fix).
**The three bugs, atomically resolved:**
1. **CJS-in-ESM** (was lines 10, 12, 142): three `require()` calls
replaced by ESM top-level imports (`import dotenv from 'dotenv'`,
`import { neon } from '@neondatabase/serverless'`, `import bcrypt from
'bcryptjs'`). `npm run reset-db` now executes on Node 22.x instead
of throwing `ReferenceError: require is not defined`.
2. **Hardcoded weak admin password** (was line 143): replaced
`bcrypt.hash('admin123', 12)` with `bcrypt.hash(adminPassword, 12)`,
where `adminPassword` is read from `process.env.ADMIN_INITIAL_PASSWORD`
with a fail-loud check at the top of `resetDatabase()`. The check
exits with code 1 BEFORE opening any DB connection, matching
`setup-neon-db.js`'s post-`drop-public-setup` shape verbatim.
3. **Password echoed to stdout** (was line 156): replaced the
`console.log(' Admin Password: admin123')` line with
`console.log(' Admin user ready (email: admin@deckhearth.com)')`
— exactly the line `setup-neon-db.js` line 157 uses post-DPS.
**Bonus defensive shape:** the seed `INSERT` now uses `ON CONFLICT
(email) DO NOTHING` (matching `setup-neon-db.js` line 149) so a
double-run doesn't error on the existing admin row. This is defensive
only; the convoy is destructive (drops all tables first), so the only
realistic "existing admin row" scenario is operator confusion between
setup-db and reset-db.
**Verification (all gates green at merge):**
- `node --check scripts/reset-db.js` → exit 0
- `npm run lint` → 128 problems (baseline preserved, no regression)
- `npm run test:run` → 21/21 pass
- Grep `scripts/reset-db.js`: 0 `require(` | 0 `admin123` | 0 `'Admin
Password'` | 3 `ADMIN_INITIAL_PASSWORD` references (docstring + const
+ error msg)
- CI on PR #25: Lint ✓ (39s) | Vitest 21/21 ✓ (28s) | Playwright smoke
3/3 ✓ (1m5s) | `forbidden-endpoints` ✓ (5s) | `forbidden-cors-headers`
✓ (4s) | Vercel preview deploy ✓ | Aggregate gate ✓
- `Screenshot diff`: not triggered (script-only PR — `paths:` filter
excludes `scripts/**`, so the queued `tighten-visual-diff-path-filter`
follow-up correctly did NOT fire here)
**Live verification deferred per convoy spec.** The script is
destructive (drops all tables); we did not exercise it against a Neon
branch in this convoy. If the operator wants end-to-end proof, the
optional post-merge action is `npm run reset-db` against a throwaway
Neon branch with `ADMIN_INITIAL_PASSWORD` set (and verify that
unsetting it triggers the fail-loud exit with the helpful error
message).
**Cross-validation finding (organic, not a planned AC).** The
`Playwright smoke` 3/3 PASS on a script-only PR confirms that
`adopt-playwright-smoke`'s smoke spec is correctly insensitive to
`scripts/**` edits — the deployed preview is unaffected by changes to
dev-only utility scripts, and the smoke spec correctly green-lights
the deployment. This is the fifth consecutive convoy where the same
3-test smoke spec has defended the auth surface (PR #15 Layout
default-user → PR #19 CORS-tighten → PR #20 rate-limiting → PR #21
pick-a-name → PR #25 reset-db-fix) without anyone writing a dedicated
test.
**Operator action required going forward:** **none.** No new env vars
(`ADMIN_INITIAL_PASSWORD` was already required by `setup-neon-db.js`
post-`drop-public-setup`; this convoy adds nothing new to the env
contract). No schema change. No infra change.
**Surfaced follow-up (newly queued):** `lint-against-cjs-in-esm-scripts`
(P3 polish) — add an ESLint `no-restricted-syntax` rule against
`require(` calls in `scripts/**` once `package.json` has `"type":
"module"`. Would have caught both this convoy AND the
`drop-public-setup` Brief 2 bug at lint time. Filed in
`.convoys/ship-readiness.md` § Queued convoys.
**`purge-weak-creds-from-helpers` scope reduction.** This convoy
satisfies the `scripts/reset-db.js` portion of the queued
`purge-weak-creds-from-helpers` follow-up. Remaining scope of that
queued convoy: `scripts/create-test-users.js` (alice/bob test fixtures)
and `TESTING_GUIDE.md` (documents the weak creds). Both are out of
scope here per the convoy spec's single-file boundary.

View file

@ -362,11 +362,12 @@ 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. - **`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 `<select>` permanently. Surfaced 2026-05-24 in `add-rate-limiting` Decision 1: the architect ran `rg 'import-lorcana' pages/ components/` and found zero frontend callers — `pages/admin/card-import.js`'s `<select>` 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. - **`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 `<select>` permanently. Surfaced 2026-05-24 in `add-rate-limiting` Decision 1: the architect ran `rg 'import-lorcana' pages/ components/` and found zero frontend callers — `pages/admin/card-import.js`'s `<select>` 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). - **`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 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). - **`purge-weak-creds-from-helpers`** (priority: P2 hygiene). **Scope reduced 2026-05-26** by `fix-reset-db-script` (squash `3ab9bf8`, PR #25): `scripts/reset-db.js` is fully clean — the `admin123` literal at the old line 143 and the `Admin Password: admin123` echo at the old line 156 are both gone (script now reads `ADMIN_INITIAL_PASSWORD` from env and never echoes the password). **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 the three helper paths were swept to `@deckhearth.com` together with the migration script. **Remaining scope:** `scripts/create-test-users.js` (alice/bob test fixtures still have weak passwords) + `TESTING_GUIDE.md` (still documents the weak creds). Small enough now that it may fold into the next pass of test-fixture cleanup or a `rename-testing-guide` doc convoy.
- **`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:<owner>/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`. - **`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:<owner>/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. - **`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. - **`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. - **`convert-reset-db-to-esm`** — **RESOLVED 2026-05-26** by `fix-reset-db-script` (squash `3ab9bf8`, PR #25). `scripts/reset-db.js` now uses ESM top-level imports (`import dotenv`, `import { neon }`, `import bcrypt`) and executes cleanly on Node 22.x. Same fix shape as `setup-neon-db.js` post-`drop-public-setup` B2. As predicted in this entry's prior note, the fold-with-`purge-weak-creds-from-helpers` shape was the right call — both ailments in `scripts/reset-db.js` were fixed atomically with a single 55-line diff. Entry kept (not removed) to preserve the audit trail. See `.convoys/fix-reset-db-script.md` § As-shipped.
- **`lint-against-cjs-in-esm-scripts`** (priority: P3 polish). Add an ESLint `no-restricted-syntax` rule against `CallExpression[callee.name="require"]` (or equivalent) in `scripts/**` so any future helper script that re-introduces CJS `require()` calls in this ESM-since-`bump-next-js` repo fails at lint time instead of at first execution. Would have caught both `drop-public-setup` Brief 2's pre-fix `setup-neon-db.js` AND `fix-reset-db-script`'s pre-fix `scripts/reset-db.js` before the bug shipped. Surfaced 2026-05-26 as the natural follow-up to PR #25. Small surface (one ESLint rule entry in `eslint.config.mjs`); the only design call is whether to scope to `scripts/**` only or to all `.js` files at the repo root (the former matches the actual blast radius; the latter is broader-than-necessary since `pages/api/**` is already correctly using ESM imports throughout).
- **`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`). - **`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. - **`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. - **`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.