docs: post-convoy cleanup for fix-layout-default-user (+ queue CI-protection follow-up)
Reflects the merged fix-layout-default-user convoy (PR #15) and the companion CI permissions fix (PR #16) in repo documentation. Also queues the new fix-vercel-deployment-protection-in-ci convoy that PR #16 exposed. .convoys/ship-readiness.md: - P0 #7: mark RESOLVED 2026-05-24 with squash commitca302a8. Document the as-shipped Layout default-null change, the 7-page sweep, the 5 new regression-lock vitest assertions, and the queued follow-ups (single-auth-provider, MobileNavigation cleanup) that stayed explicitly out of scope. - Queued convoys: add fix-vercel-deployment-protection-in-ci (P2, CI infra) — PR #16's permissions fix exposed that Vercel Deployment Protection 401s anonymous CI requests; needs a bypass-secret plumb to land cleanly. New section also captures other in-flight follow-ups (rotate-default-admin, cors-tighten, add-rate-limiting, purge-weak-creds-from-helpers, single-auth-provider, cleanup-mobile-nav-dead-props, bump-eslint-10) so the audit trail is centralized. AGENTS.md: - § 4 Gotcha #8: mark RESOLVED with commit refca302a8. Mirror the convention used by the prior cleanup commits for #2, #3, #4, #5 (entry kept, not renumbered). - § 3 Key conventions: add a new "Layout user prop" bullet documenting the new default-null + logged-out-CTA contract so the convention is discoverable from the conventions list, not just the resolved-gotcha entry. .convoys/fix-vercel-deployment-protection-in-ci.md (new): - Queued scaffold. Operator must seed VERCEL_AUTOMATION_BYPASS_SECRET as a repo secret before the implementer can run. Decisions to ratify (query param vs. header), known constraints, acceptance criteria, and out-of-scope all enumerated. No changes to: package.json, lib/**, pages/**, components/**, scripts/**, .github/**, README.md, .cursor/rules/**. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
ca302a89c1
commit
b7ddd0895e
3 changed files with 268 additions and 2 deletions
239
.convoys/fix-vercel-deployment-protection-in-ci.md
Normal file
239
.convoys/fix-vercel-deployment-protection-in-ci.md
Normal file
|
|
@ -0,0 +1,239 @@
|
||||||
|
---
|
||||||
|
name: fix-vercel-deployment-protection-in-ci
|
||||||
|
classification: convoy
|
||||||
|
success_metric: |
|
||||||
|
`Playwright smoke` and `Screenshot diff` workflows reach their actual
|
||||||
|
smoke / capture step on every PR (no more 401-from-Vercel-SSO 10-min
|
||||||
|
timeouts). Both workflows complete in < 5 minutes. Failures, when they
|
||||||
|
occur, are real assertion failures — not auth failures against the
|
||||||
|
preview URL.
|
||||||
|
skip:
|
||||||
|
- role-design-system-auditor # CI infra only
|
||||||
|
- role-a11y-auditor # no a11y surface
|
||||||
|
- role-ux-reviewer # no UX surface
|
||||||
|
- role-ia-architect # no IA surface
|
||||||
|
- browser-smoke # the convoy IS the smoke pipeline
|
||||||
|
status: queued
|
||||||
|
created: 2026-05-24
|
||||||
|
parent: ship-readiness
|
||||||
|
addresses: P0 #7 (CI infrastructure side-effect)
|
||||||
|
depends_on:
|
||||||
|
- fix-layout-default-user (shipped — PR #15, ca302a8)
|
||||||
|
- PR #16 fix(ci) scoped permissions (shipped — 7e97254)
|
||||||
|
---
|
||||||
|
|
||||||
|
# Fix Vercel Deployment Protection in CI
|
||||||
|
|
||||||
|
Plumb `VERCEL_AUTOMATION_BYPASS_SECRET` into the `Playwright smoke` and
|
||||||
|
`Screenshot diff` workflows so anonymous GitHub Actions runners can
|
||||||
|
actually GET the preview URL without hitting Vercel's SSO 401 challenge.
|
||||||
|
Without this, both workflows permanently red on every PR — just slower
|
||||||
|
red than before PR #16.
|
||||||
|
|
||||||
|
## Why now
|
||||||
|
|
||||||
|
PR #16 (`fix(ci): scoped permissions`, squash commit `7e97254`) added
|
||||||
|
minimal scoped `permissions:` blocks to `.github/workflows/preview-smoke.yml`
|
||||||
|
and `.github/workflows/visual-diff.yml`. That fixed the 5-second 403
|
||||||
|
"Resource not accessible by integration" failure both workflows hit when
|
||||||
|
trying to call the GitHub deployments API. **However**, with permissions
|
||||||
|
correct, both workflows now reach the actual deployment check and fail
|
||||||
|
with a different error: a 10-minute timeout from
|
||||||
|
`patrickedqvist/wait-for-vercel-preview@v1.3.2`'s subsequent HTTP GET
|
||||||
|
against the preview URL, which Vercel returns 401 for because Deployment
|
||||||
|
Protection is on (anonymous GitHub-runner request → Vercel SSO challenge).
|
||||||
|
Cost: ~10 minutes of runner time per workflow per PR — and zero signal,
|
||||||
|
since neither workflow ever reaches its smoke step. This blocks PR #15's
|
||||||
|
recurring follow-up convoys (visual-regression baselines, Playwright smoke
|
||||||
|
for `adopt-playwright-smoke`) from getting any CI feedback.
|
||||||
|
|
||||||
|
The bypass token already exists locally as `VERCEL_AUTOMATION_BYPASS_SECRET`
|
||||||
|
in `.env.local` (Protection Bypass for Automation, configured in the
|
||||||
|
Vercel project). Documented in `AGENTS.md` § 7 — Deployment. The work here
|
||||||
|
is plumbing it from operator-supplied repo secret → workflow env →
|
||||||
|
`wait-for-vercel-preview`'s `path:` input + the eventual Playwright
|
||||||
|
`BASE_URL` so anonymous runner requests bypass the SSO challenge.
|
||||||
|
|
||||||
|
## Operator action required (BEFORE this convoy can run)
|
||||||
|
|
||||||
|
This convoy CANNOT proceed without the operator first seeding the secret
|
||||||
|
into GitHub Actions. The implementer has nothing to wire up if the
|
||||||
|
secret isn't visible to the workflows.
|
||||||
|
|
||||||
|
1. **Seed the secret:**
|
||||||
|
```bash
|
||||||
|
gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body "<value from local .env.local>"
|
||||||
|
```
|
||||||
|
(The value is whatever `VERCEL_AUTOMATION_BYPASS_SECRET=…` says in
|
||||||
|
`.env.local`. Do not paste it anywhere logged. Do not echo it from a
|
||||||
|
workflow step.)
|
||||||
|
2. **Confirm visibility:**
|
||||||
|
```bash
|
||||||
|
gh secret list
|
||||||
|
```
|
||||||
|
Expect to see `VERCEL_AUTOMATION_BYPASS_SECRET` listed alongside the
|
||||||
|
existing repo secrets. Note: `gh secret list` shows names only — never
|
||||||
|
values — by design.
|
||||||
|
3. **Notify the next agent** that steps 1 + 2 are done. The convoy file's
|
||||||
|
frontmatter `status:` should flip from `queued` to `in-progress` only
|
||||||
|
after this notification.
|
||||||
|
|
||||||
|
This is the same pattern `npm run setup-db`'s `ADMIN_INITIAL_PASSWORD`
|
||||||
|
established (`drop-public-setup` Brief 1, commit `ff80753`): CI / scripts
|
||||||
|
that need a secret get an actionable fail-loud error when the secret is
|
||||||
|
missing, and the operator seeds it once per environment.
|
||||||
|
|
||||||
|
## Decisions to ratify with operator
|
||||||
|
|
||||||
|
Queued; do not pre-decide.
|
||||||
|
|
||||||
|
1. **Bypass via query param vs. request header.**
|
||||||
|
- **Option A — query param.** Append `?x-vercel-protection-bypass=...&x-vercel-set-bypass-cookie=true`
|
||||||
|
to the wait-action's `path:` input AND to the Playwright `BASE_URL`.
|
||||||
|
The first request sets a `_vercel_jwt` cookie on the runner's
|
||||||
|
ephemeral browser context; subsequent same-origin requests reuse it.
|
||||||
|
Pro: works with any HTTP client, no custom config in Playwright.
|
||||||
|
Con: the bypass token shows up in workflow run logs if any step
|
||||||
|
echoes the URL (mitigation: never `echo` or `cat` a URL containing
|
||||||
|
the token; log `${{ steps.wait.outputs.url }}` only after stripping
|
||||||
|
the query string).
|
||||||
|
- **Option B — request header (`x-vercel-protection-bypass: <secret>`).**
|
||||||
|
Cleaner — the token never appears in any URL. But requires custom
|
||||||
|
HTTP-client config in `playwright.config.js` (`extraHTTPHeaders`)
|
||||||
|
AND in `wait-for-vercel-preview` (the action's docs need confirming —
|
||||||
|
header support may not be exposed via inputs).
|
||||||
|
2. **CI assertion that bypass actually works.** Should we add a step
|
||||||
|
that explicitly asserts `200` on the preview URL during the wait-
|
||||||
|
action's healthcheck phase, before handing off to Playwright /
|
||||||
|
screenshot capture? This would surface bypass-misconfiguration as a
|
||||||
|
fast-fail step instead of letting Playwright time out 8 minutes
|
||||||
|
later on a different error. Cost: ~5 lines of YAML; benefit: clearer
|
||||||
|
failure signal for the next operator-touch event.
|
||||||
|
3. **Workflow concurrency cancellation.** The workflows already use
|
||||||
|
`concurrency:` keyed on `github.ref`. Confirm that the bypass-token
|
||||||
|
wiring doesn't inadvertently break the cancel-stale behavior (e.g.
|
||||||
|
if the `secrets.VERCEL_AUTOMATION_BYPASS_SECRET` reference is in a
|
||||||
|
`concurrency:` expression, that's a syntax error and the implementer
|
||||||
|
should pull it into a job-level `env:` instead).
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
|
||||||
|
**In scope:**
|
||||||
|
|
||||||
|
- `.github/workflows/preview-smoke.yml` — wire the bypass into the
|
||||||
|
`wait-for-vercel-preview` step's `path:` input (Option A) OR add the
|
||||||
|
bypass header via the action's input shape (Option B, pending
|
||||||
|
confirmation that the action exposes header inputs).
|
||||||
|
- `.github/workflows/visual-diff.yml` — same treatment as preview-smoke
|
||||||
|
(the two workflows have similar shapes; whatever pattern works for one
|
||||||
|
should land in both).
|
||||||
|
- `playwright.config.js` (when it exists — the `adopt-playwright-smoke`
|
||||||
|
convoy ships it) — add `use: { extraHTTPHeaders: { 'x-vercel-protection-bypass': process.env.VERCEL_AUTOMATION_BYPASS_SECRET } }`
|
||||||
|
if Decision #1 picks Option B; OR build the BASE_URL with the query
|
||||||
|
param (Option A).
|
||||||
|
- Any test-setup file or helper that constructs the preview URL for
|
||||||
|
`screenshot-diff`-style workflows.
|
||||||
|
|
||||||
|
**Out of scope:**
|
||||||
|
|
||||||
|
- Writing new Playwright tests. Test authoring lives in
|
||||||
|
`adopt-playwright-smoke`. This convoy only makes the existing smoke
|
||||||
|
pipeline reachable.
|
||||||
|
- Broadening workflow `permissions:` blocks. PR #16 already landed the
|
||||||
|
minimal scope; this convoy should not need to touch them again.
|
||||||
|
- Replacing `patrickedqvist/wait-for-vercel-preview` with a different
|
||||||
|
action. The action retrieves the URL successfully (confirmed in PR #16's
|
||||||
|
run logs); the failure is the subsequent HTTP GET, which is a
|
||||||
|
configuration issue, not an action choice. A wholesale action swap is
|
||||||
|
a deeper rewrite — separate convoy if/when it's needed.
|
||||||
|
- Authoring new visual-regression baselines. The screenshot diff workflow
|
||||||
|
has nothing meaningful to compare against today; baseline authoring is
|
||||||
|
its own convoy.
|
||||||
|
- Disabling Vercel Deployment Protection on the project. Operator may
|
||||||
|
prefer to keep protected previews (cheap defense-in-depth against
|
||||||
|
preview-URL leakage); this fix lets CI work _around_ the protection
|
||||||
|
without weakening it.
|
||||||
|
|
||||||
|
## Known constraints
|
||||||
|
|
||||||
|
- **`wait-for-vercel-preview@v1.3.2` `path:` input is supported.** PR
|
||||||
|
#16's run logs confirm the action retrieves the URL successfully — the
|
||||||
|
subsequent HTTP GET is what fails. The action's `path:` input accepts a
|
||||||
|
full path including query string, so Option A (`?x-vercel-protection-bypass=...`)
|
||||||
|
is mechanically straightforward. Whether the action exposes a way to
|
||||||
|
inject custom request headers (Option B) needs to be confirmed by
|
||||||
|
reading the action's source / README before the implementer commits to
|
||||||
|
it.
|
||||||
|
- **The same secret will need to be plumbed into Playwright's `BASE_URL`
|
||||||
|
or into a request header in `playwright.config.js`** when the
|
||||||
|
`adopt-playwright-smoke` convoy ships. Coordinating shape now (this
|
||||||
|
convoy) vs. shape later (when Playwright lands) saves churn — the
|
||||||
|
implementer should pick whichever option keeps both call sites
|
||||||
|
consistent.
|
||||||
|
- **`npm run setup-db`'s `ADMIN_INITIAL_PASSWORD` is a parallel
|
||||||
|
precedent** for "CI needs a secret the operator must seed." Same
|
||||||
|
pattern applies: secret is repo-scoped, fail-loud (or fail-noisy) when
|
||||||
|
unset, never echoed to logs. See `drop-public-setup` Brief 1
|
||||||
|
(commit `ff80753`).
|
||||||
|
- **Token rotation.** The Vercel bypass token can be rotated from the
|
||||||
|
Vercel dashboard. If/when that happens, the operator must re-seed the
|
||||||
|
GitHub secret (`gh secret set ...`). No automation here — this is a
|
||||||
|
human responsibility per the same pattern as `JWT_SECRET` rotation.
|
||||||
|
|
||||||
|
## Acceptance criteria
|
||||||
|
|
||||||
|
The convoy is shippable when ALL of the following hold:
|
||||||
|
|
||||||
|
1. `Playwright smoke` workflow reaches its actual smoke step on a fresh
|
||||||
|
PR. It either passes (smoke green) OR fails on a real assertion
|
||||||
|
(Playwright reports a test failure or a runtime error from the
|
||||||
|
smoke spec). It does NOT fail with a 10-min timeout from the
|
||||||
|
`wait-for-vercel-preview` step or with a 401 from the preview URL.
|
||||||
|
2. `Screenshot diff` workflow reaches its screenshot capture step and
|
||||||
|
posts the "Visual Diff" comment to the PR (even if the diff itself
|
||||||
|
is empty / first-run / null-baseline). Same constraint: no 10-min
|
||||||
|
timeout, no 401.
|
||||||
|
3. Both workflows complete in < 5 minutes on a typical PR (the
|
||||||
|
pre-PR-16 baseline was ~30 seconds for the workflow body; adding a
|
||||||
|
bypass query string or header shouldn't materially affect runtime).
|
||||||
|
4. The bypass token does not appear in any workflow run log. Verify by
|
||||||
|
downloading the raw log of a passing run and grepping for the token's
|
||||||
|
first 8 chars.
|
||||||
|
5. Workflow YAML still passes basic actionlint review (`actionlint .github/workflows/*.yml`
|
||||||
|
exits 0). PR #16's permissions blocks remain unchanged.
|
||||||
|
6. `AGENTS.md` § 7 deployment paragraph (the "Preview protection bypass
|
||||||
|
for automation" line) still reflects reality after the change. May
|
||||||
|
need a one-sentence update if the implementer picks Option B
|
||||||
|
(`x-vercel-protection-bypass` header) vs. Option A (query string).
|
||||||
|
|
||||||
|
## Anything flagged but not acted on (in advance)
|
||||||
|
|
||||||
|
These are real findings that the architect / implementer should NOT try
|
||||||
|
to solve in this convoy. Each is queued separately if it warrants a fix.
|
||||||
|
|
||||||
|
- **The `wait-for-vercel-preview` action is no longer maintained** (last
|
||||||
|
release Mar 2024; no v2). Could be replaced with a few lines of
|
||||||
|
`gh api` + `curl`-loop in the workflow itself. Not in scope here —
|
||||||
|
this convoy needs to fix the immediate auth failure, not rewrite the
|
||||||
|
wait logic. Queue as `replace-wait-for-vercel-preview` if the action
|
||||||
|
ages out further or has a security advisory.
|
||||||
|
- **Playwright config doesn't exist yet.** `playwright.config.js`,
|
||||||
|
`tests/smoke/`, and `@playwright/test` all land in
|
||||||
|
`adopt-playwright-smoke` (P1 #10 step 2 / launch sequence step 10).
|
||||||
|
Until that convoy ships, the only `Playwright smoke` workflow body is
|
||||||
|
a no-op. This convoy can pre-wire the bypass infrastructure (env var,
|
||||||
|
workflow secrets) so `adopt-playwright-smoke` only needs to add the
|
||||||
|
test files and the Playwright config — but it can't ship a real
|
||||||
|
smoke-pass without that follow-up.
|
||||||
|
- **`Screenshot diff` baseline authoring.** Even after this convoy lands,
|
||||||
|
the visual-diff workflow has nothing to compare against on its first
|
||||||
|
run. That's expected and orthogonal — baseline authoring is a separate
|
||||||
|
scope.
|
||||||
|
- **Operator-rotation hygiene for `VERCEL_AUTOMATION_BYPASS_SECRET`.**
|
||||||
|
Vercel's bypass tokens don't auto-expire. If the team wants a periodic
|
||||||
|
rotation policy, that's an ops-runbook concern outside this convoy.
|
||||||
|
- **`AGENTS.md` § 7 wording.** The current "Smoke/visual-diff workflows
|
||||||
|
pass this header (`x-vercel-protection-bypass`)" line in § 7 is
|
||||||
|
aspirational — it describes intent, not what was actually wired. After
|
||||||
|
this convoy ships, that line becomes accurate. The doc-writer pass at
|
||||||
|
convoy close should reword to past-tense reality.
|
||||||
|
|
@ -88,11 +88,24 @@ These MUST land before any anonymous traffic touches the production URL.
|
||||||
- **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.
|
- **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).
|
- **Owns:** `role-architect` (pattern) → `role-implementer` (per-route).
|
||||||
|
|
||||||
### 7. Layout default-prop leaks maintainer email
|
### 7. Layout default-prop leaks maintainer email — **RESOLVED 2026-05-24**
|
||||||
|
|
||||||
|
- **Resolved by:** `fix-layout-default-user` convoy (PR #15, squash commit `ca302a8`). Brief 1 (pre-squash `ddf8fd2`) shipped the Layout default-null + logged-out branch + vitest lock-in; Brief 2 (pre-squash `8c7d127`, rebased to `0f6bfbb` pre-merge) swept the 7 pages that needed page-level fixes.
|
||||||
- **File:** `components/Layout.js` line 562: `function Layout({ children, user = { email: 'me@randallstillwell.com', role: 'user' }, ... })`.
|
- **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.
|
- **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).
|
- **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).
|
||||||
|
- **As-shipped:**
|
||||||
|
1. `components/Layout.js` default prop changed from hardcoded `{ email: 'me@randallstillwell.com', role: 'user' }` to `null`. `UserProfileDropdown` now branches on `user === null` and renders a `<Link href="/login">Sign in</Link>` CTA in place of the avatar + email + dropdown menu (`NavigationContent`'s `authenticatedNavigation` / `myCollectionNavigation` / `adminNavigation` were already null-safe via existing optional chains; no change there).
|
||||||
|
2. **7 pages swept** (Brief 2, 11 `<Layout>` call sites total). `pages/scanner.js` (×1), `pages/decks.js` (×3), `pages/deck-builder.js` (×4), `pages/deck/[id].js` (×3) now pass `user={user}` explicitly. `pages/profile.js` and `pages/settings.js` replaced their leaky `useState({ email: 'me@randallstillwell.com', role: 'admin' })` initializer with `useState(null)` (15 sync `user.*` reads in profile + 1 in settings got null-guards). `pages/card/[id].js` replaced its hardcoded `const user = { email: 'me@…', role: 'user' }` with `const { user } = useAuth()` from `lib/use-auth.js`.
|
||||||
|
3. **10 pages already correct** (architect's per-page audit, Decision B in `.convoys/fix-layout-default-user.md`): `dashboard`, `my-cards`, `cards`, `collections`, `collection/[identifier]`, `community/collections`, `admin/card-import`, `admin/card-editor`, `invite/accept`, `invite/decline`. No changes there.
|
||||||
|
4. **Test coverage:** `test/components/Layout.test.js` (new) adds 5 regression-lock assertions — no maintainer email when `user` is `null`/omitted; "Sign in" link present when logged out; supplied email renders when supplied; no accidental `Guest` placeholder. Vitest 21/21 green at merge (16 pre-existing auth tests still green).
|
||||||
|
5. **New devDeps:** `jsdom@^29` + `@testing-library/react@^16` (test-only). `vitest.config.js` got a 3-line `esbuild` block to parse JSX in `.js` files (per-file `// @vitest-environment jsdom` directive — no global env change).
|
||||||
|
6. **Verification at merge:** `rg 'me@randallstillwell.com' pages/` → 0 hits; anonymous `curl /cards` returned HTTP 200 with no maintainer email; lint baseline match (128 problems, unchanged); CI Aggregate gate / Lint / Vitest / Vercel preview / forbidden-endpoints all green. `Playwright smoke` + `Screenshot diff` red but for an unrelated CI-infra reason — see CI infrastructure side-effect note below.
|
||||||
|
- **Flagged-but-deferred** (deliberately out of scope per the convoy spec):
|
||||||
|
1. 4 pages still import `useAuth` from `lib/auth-context.js` (`pages/scanner.js`, `pages/decks.js`, `pages/deck-builder.js`, `pages/deck/[id].js`) — collapsing the three parallel client-side auth surfaces is the queued `single-auth-provider` convoy (P1 #9 in this file), not this one.
|
||||||
|
2. `components/MobileNavigation.js` still receives a dead `user` prop (it accepts `{ user, onMenuOpen }` but never reads `user.*` — the bottom-bar items are static). Queued as `cleanup-mobile-nav-dead-props` (or fold into `god-component-split` if that lands first).
|
||||||
|
3. `pages/card/[id].js` still imports `useIsAdmin` from `lib/admin-auth.js` — third parallel auth surface; same `single-auth-provider` convoy will collapse it.
|
||||||
|
- **CI infrastructure side-effect (not part of this convoy).** PR #16 (squash commit `7e97254`) landed alongside as a CI permissions fix, adding scoped `permissions:` blocks to `.github/workflows/preview-smoke.yml` + `.github/workflows/visual-diff.yml`. That fixed the 5-second 403 "Resource not accessible by integration" failure on both workflows but exposed a second issue: with permissions correct, both now reach the actual deployment check and 10-min-timeout against Vercel Deployment Protection's 401 SSO challenge (anonymous GitHub runner GETs the preview URL). New queued convoy `fix-vercel-deployment-protection-in-ci` (`.convoys/fix-vercel-deployment-protection-in-ci.md`) tracks that follow-up.
|
||||||
- **Owns:** `role-implementer`.
|
- **Owns:** `role-implementer`.
|
||||||
|
|
||||||
### 8. Next.js 15.4.3 — Vercel platform blocks deploys (vulnerable version)
|
### 8. Next.js 15.4.3 — Vercel platform blocks deploys (vulnerable version)
|
||||||
|
|
@ -259,6 +272,19 @@ Each phase is one Conductor-created convoy. Don't run more than two in parallel
|
||||||
|
|
||||||
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.
|
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.
|
||||||
|
|
||||||
|
## Queued convoys
|
||||||
|
|
||||||
|
Follow-ups surfaced mid-convoy or mid-PR that didn't fit the original launch sequence but need to land before public traffic. Listed in priority order; not all will be P0/P1 — most are CI / DX / hygiene polish.
|
||||||
|
|
||||||
|
- **`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.
|
||||||
|
- **`cors-tighten`** (priority: P1 quality). Drop the wildcard `Access-Control-Allow-Origin` header from `pages/api/auth/verify.js`. Surfaced in P0 #5 (deferred from `fix-auth-bypass` Brief 4).
|
||||||
|
- **`add-rate-limiting`** (priority: P1 quality, also listed in launch sequence step 4). Extend `lib/rate-limit.js` to `/api/users/search`, `/api/cards/search`, all `/api/cards/import-*`, and `/api/user/avatar*`. Login + register already wired in `fix-auth-bypass` Brief 4.
|
||||||
|
- **`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.
|
||||||
|
- **`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.
|
||||||
|
- **`fix-vercel-deployment-protection-in-ci`** (priority: P2 CI infra; **operator action required**). PR #16's permissions fix exposed that Vercel Deployment Protection 401s anonymous CI requests, so `Playwright smoke` and `Screenshot diff` now 10-min-timeout instead of 5-second-403. Plumb `VERCEL_AUTOMATION_BYPASS_SECRET` into both workflows + (eventually) Playwright config. Operator must seed the secret into GitHub Actions before the implementer can run. See `.convoys/fix-vercel-deployment-protection-in-ci.md`. Created 2026-05-24.
|
||||||
|
|
||||||
## Self-analytics
|
## 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.
|
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.
|
||||||
|
|
|
||||||
|
|
@ -33,6 +33,7 @@ Code graph is indexed by `user-code-review-graph` MCP (122 files, 628 nodes, 560
|
||||||
|
|
||||||
- **Auth (server):** `import { getUserFromRequest } from '../../lib/permission-middleware'` → returns `{ userId, email, role }` or `null`. `null` means "send 401" — always early-return when the user is null before doing any work that depends on their identity.
|
- **Auth (server):** `import { getUserFromRequest } from '../../lib/permission-middleware'` → returns `{ userId, email, role }` or `null`. `null` means "send 401" — always early-return when the user is null before doing any work that depends on their identity.
|
||||||
- **Auth (client):** `import { useAuth } from '../lib/use-auth'`. Avoid `lib/auth-context.js` and `lib/admin-auth.js` for new code — they are legacy parallel implementations.
|
- **Auth (client):** `import { useAuth } from '../lib/use-auth'`. Avoid `lib/auth-context.js` and `lib/admin-auth.js` for new code — they are legacy parallel implementations.
|
||||||
|
- **Layout `user` prop:** pages should pass `user` from `useAuth()` to `<Layout>`. Layout's default is `null` and renders a logged-out "Sign in" CTA when no user is supplied — both paths are valid (some surfaces like `pages/invite/{accept,decline}.js` legitimately render Layout for anonymous visitors). Do not reintroduce a hardcoded user object as a default prop.
|
||||||
- **JWT secret + TTL:** `import { JWT_SECRET, JWT_TOKEN_TTL } from '../../lib/auth-secret.js'`. This is the only place either value is defined; do not reintroduce literal fallbacks. `JWT_TOKEN_TTL = '24h'` is canonical.
|
- **JWT secret + TTL:** `import { JWT_SECRET, JWT_TOKEN_TTL } from '../../lib/auth-secret.js'`. This is the only place either value is defined; do not reintroduce literal fallbacks. `JWT_TOKEN_TTL = '24h'` is canonical.
|
||||||
- **Auth helper (token mint / verify / password hash):** `import { ... } from '../../pages/api/auth-utils'` (`generateToken`, `verifyToken`, `hashPassword`, `verifyPassword`). Reads the secret + TTL from `lib/auth-secret.js` under the hood.
|
- **Auth helper (token mint / verify / password hash):** `import { ... } from '../../pages/api/auth-utils'` (`generateToken`, `verifyToken`, `hashPassword`, `verifyPassword`). Reads the secret + TTL from `lib/auth-secret.js` under the hood.
|
||||||
- **Rate limiting:** `import { checkAuthRateLimit } from '../../lib/rate-limit.js'` for any new auth-surface endpoint (`/api/auth/login` + `/api/auth/register` already wired). Returns `{ allowed, remaining, reset }`; on `!allowed` return 429 with a `Retry-After` header. See `.cursor/rules/api-routes.mdc` § "Rate limiting" for the verbatim shape.
|
- **Rate limiting:** `import { checkAuthRateLimit } from '../../lib/rate-limit.js'` for any new auth-surface endpoint (`/api/auth/login` + `/api/auth/register` already wired). Returns `{ allowed, remaining, reset }`; on `!allowed` return 429 with a `Retry-After` header. See `.cursor/rules/api-routes.mdc` § "Rate limiting" for the verbatim shape.
|
||||||
|
|
@ -53,7 +54,7 @@ Code graph is indexed by `user-code-review-graph` MCP (122 files, 628 nodes, 560
|
||||||
- **#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.
|
- **#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.
|
- **#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.
|
- **#7 — Dual `is_public` semantics.** Collections and decks both have `is_public` columns; check which controls discovery vs. anonymous read in the relevant route.
|
||||||
- **#8 — Layout has hardcoded default user.** `Layout({ user = { email: 'me@randallstillwell.com', role: 'user' } })`. Anything rendering Layout without passing `user` will impersonate the maintainer. Pass `user` explicitly from every page.
|
- **#8 — Layout has hardcoded default user. RESOLVED** by `fix-layout-default-user` convoy (PR #15, squash commit `ca302a8`). `components/Layout.js`'s default prop is now `null`; `UserProfileDropdown` renders a `<Link href="/login">Sign in</Link>` CTA when `user === null`. Brief 2 also swept the 7 pages that needed page-level fixes (`scanner` / `decks` / `deck-builder` / `deck/[id]` now pass `user={user}` to Layout; `profile` / `settings` replaced leaky `useState({email:'me@…'})` with `useState(null)` + null-guards on every sync `user.*` read; `card/[id]` swapped a hardcoded `const user = {...}` for `useAuth()` from `lib/use-auth.js`). `test/components/Layout.test.js` adds 5 regression-lock assertions (no maintainer email when user is null/omitted; "Sign in" link present; supplied email renders; no "Guest" placeholder); vitest 21/21 green at merge. New devDeps: `jsdom@^29` + `@testing-library/react@^16`. See `.convoys/fix-layout-default-user.md` and `.convoys/ship-readiness.md` P0 #7. Entry kept (not renumbered) to preserve cross-references.
|
||||||
- **#9 — `typescript` is a devDep, but the source is still JavaScript-only.** `package.json` lists `typescript@^5.9.3` purely so `eslint-config-next@16`'s bundled `typescript-eslint` chain can satisfy its hard `require('typescript')` at module load (the `peerDependenciesMeta.typescript.optional: true` flag in `eslint-config-next` only suppresses npm's install-time warning, not the runtime require). There is no `tsconfig.json`, no `.ts`/`.tsx` files, and no `// @ts-check` directives. Do not rename `.js` files to `.ts` or add a `tsconfig.json` without an explicit convoy decision — TypeScript adoption is its own scope. See `.convoys/bump-next-js.md` § Decisions C.
|
- **#9 — `typescript` is a devDep, but the source is still JavaScript-only.** `package.json` lists `typescript@^5.9.3` purely so `eslint-config-next@16`'s bundled `typescript-eslint` chain can satisfy its hard `require('typescript')` at module load (the `peerDependenciesMeta.typescript.optional: true` flag in `eslint-config-next` only suppresses npm's install-time warning, not the runtime require). There is no `tsconfig.json`, no `.ts`/`.tsx` files, and no `// @ts-check` directives. Do not rename `.js` files to `.ts` or add a `tsconfig.json` without an explicit convoy decision — TypeScript adoption is its own scope. See `.convoys/bump-next-js.md` § Decisions C.
|
||||||
- **#10 — ESLint pinned to v9 (maintenance), not v10 (latest).** `devDependencies.eslint` is `^9.39.4` even though `latest` is `10.4.0`. We tried v10 and `npm run lint` crashed with `TypeError: scopeManager.addGlobals is not a function` because `eslint-config-next@16`'s bundled `typescript-eslint@8.x` predates ESLint v10's redesigned global-ingestion path. Reverted to v9 under Decision D. **Do NOT bump ESLint independently** — wait for the queued `bump-eslint-10` follow-up convoy, which is upstream-blocked until `typescript-eslint` ships a v10-tested release that `eslint-config-next` bundles. See `.convoys/bump-next-js.md` § Decisions D + "Follow-up convoys queued".
|
- **#10 — ESLint pinned to v9 (maintenance), not v10 (latest).** `devDependencies.eslint` is `^9.39.4` even though `latest` is `10.4.0`. We tried v10 and `npm run lint` crashed with `TypeError: scopeManager.addGlobals is not a function` because `eslint-config-next@16`'s bundled `typescript-eslint@8.x` predates ESLint v10's redesigned global-ingestion path. Reverted to v9 under Decision D. **Do NOT bump ESLint independently** — wait for the queued `bump-eslint-10` follow-up convoy, which is upstream-blocked until `typescript-eslint` ships a v10-tested release that `eslint-config-next` bundles. See `.convoys/bump-next-js.md` § Decisions D + "Follow-up convoys queued".
|
||||||
- **#11 — Turbopack is now the default bundler.** `next dev` and `next build` use Turbopack by default in Next.js 16. The fallback per command is `--webpack` (e.g. `next build --webpack`). We have no custom `webpack:` block in `next.config.js`, no custom loaders/aliases, and no Sass tilde imports, so Turbopack should "just work" — but if a build/runtime regression appears, reproduce on both bundlers before deciding whether to revert or pin a script to webpack. Do not pre-emptively switch to `--webpack`.
|
- **#11 — Turbopack is now the default bundler.** `next dev` and `next build` use Turbopack by default in Next.js 16. The fallback per command is `--webpack` (e.g. `next build --webpack`). We have no custom `webpack:` block in `next.config.js`, no custom loaders/aliases, and no Sass tilde imports, so Turbopack should "just work" — but if a build/runtime regression appears, reproduce on both bundlers before deciding whether to revert or pin a script to webpack. Do not pre-emptively switch to `--webpack`.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue