# single-auth-provider (P1 quality — collapse three client auth surfaces onto one) **Status:** OPEN 2026-05-26 (this convoy) **Priority:** P1 quality (launch sequence step 9 — `.convoys/ship-readiness.md` § P1 entry 9) **Convoy owner:** parent (architect + implementer rolled together — diff is mechanical once the shape-parity decision is made) **Branch:** `convoy/single-auth-provider` **Opened:** 2026-05-26 ## Background The repo carried **three parallel client-side auth implementations** since the early days of the project. `.convoys/ship-readiness.md` § P1 entry 9 ("Three parallel client-side auth implementations") is the canonical spec; AGENTS.md § 3 already documented `lib/use-auth.js` as the canonical surface and instructed new code to avoid the other two. This convoy executes the collapse. The three surfaces: 1. **`lib/use-auth.js::useAuth`** (the keeper). Hook-only — reads `auth_token` from `localStorage` on mount, hits `/api/auth/verify`, exposes `{ user, loading, logout, refreshAuth }`. No React context, no `` wrapper required. 2. **`lib/auth-context.js::{ AuthProvider, useAuth }`** (legacy). Context provider + consumer hook with the same verify-on-mount semantics, plus `login()` and `register()` helpers that `pages/login.js` / `pages/signup.js` no longer use (those pages call `/api/auth/{login,register}` directly and write the token to `localStorage` themselves). Wired in `pages/_app.js` as ``. 3. **`lib/admin-auth.js::{ AdminProvider, useAdmin, useIsAdmin }`** (legacy). A redundant context that does the *same* verify-on-mount roundtrip, plus a hook-only `useIsAdmin()` that does its own verify roundtrip on top of that. `AdminProvider` is **not** wired in `_app.js` (verified by reading `_app.js` pre-convoy: only `` + ``), so `useAdmin()` would have thrown at runtime if anyone called it — nobody does. Only `useIsAdmin()` has a live consumer (`pages/card/[id].js`). **Symptom that drove ranking this P1.** When `pages/card/[id].js` mounts, it calls `useAuth()` from `lib/use-auth.js` AND `useIsAdmin()` from `lib/admin-auth.js`, each issuing its own `GET /api/auth/verify`. With `AuthProvider` mounted on every page via `_app.js`, that's a **third** verify roundtrip on the very first page load. Three roundtrips, identical request, serial cost on a cold connection. Post-convoy: 1 roundtrip per page-load. ## Decisions ### D1 — Shape parity check on `lib/use-auth.js`. Verdict: no parity gap; do **not** extend. `lib/auth-context.js::useAuth()` exposed `{ user, loading, login, register, logout }`. `lib/use-auth.js::useAuth()` exposes `{ user, loading, logout, refreshAuth }`. The apparent gap is `login` / `register`. Verified-by-grep: **zero call sites** invoke `useAuth().login(…)` or `useAuth().register(…)` anywhere in `pages/**` or `components/**`. The only callers of those flows are `pages/login.js` and `pages/signup.js`, both of which `fetch` `/api/auth/{login,register}` directly and write the returned token to `localStorage`. `useAuth()`'s `useEffect` then picks up the new token on the next mount (or the page can call `refreshAuth()` to re-verify in place). Conclusion: do **not** add `login` / `register` to `use-auth.js`. The legacy methods were dead code on the consumer surface; preserving them would be cargo-culting and would re-create a non-DRY login flow (one in `pages/login.js`, one in the hook). `auth-and-permissions.mdc` § "Authentication state on the client" was updated to document the post-convoy `useAuth()` shape and to spell out the `login.js` / `signup.js` direct-fetch pattern. ### D2 — `useIsAdmin()` migration shape. Verdict: collapse onto the existing `useAuth()` call. `pages/card/[id].js` is the **only** consumer of `useIsAdmin()`. The page already called `useAuth()` from `lib/use-auth.js` at line 13 (added by `fix-layout-default-user` Brief 2). The migration is: ```js // Before const { user } = useAuth(); // ... const { isAdmin, loading: adminLoading } = useIsAdmin(); // ... usage at line 524: {isAdmin && !adminLoading && (...)} // After const { user, loading: authLoading } = useAuth(); // ... const isAdmin = user?.role === 'admin'; const adminLoading = authLoading; // ... usage at line 524 unchanged: {isAdmin && !adminLoading && (...)} ``` `adminLoading` is kept as a local alias rather than substituting `authLoading` directly at the call site, to keep the diff minimal and the rendering condition byte-identical. The `loading` window from `useAuth()` covers exactly the same period (`/api/auth/verify` resolution) that `useIsAdmin`'s own loading covered, so there is no UX regression. ### D3 — `pages/_app.js` provider tree. Before / after. ```jsx // Before // After ``` `useAuth()` from `lib/use-auth.js` is hook-only — no Provider needed. The `` wrapper is removed entirely; no replacement Provider is added. `` stays (out of scope). `` was never in the tree to begin with. ### D4 — Token-verify roundtrip count. Per the spec: pre-convoy a worst-case page mount issued **3** identical `GET /api/auth/verify` requests: 1. `` in `_app.js` calls `verifyToken()` on mount. 2. `pages/card/[id].js` calls `useAuth()` from `lib/use-auth.js`, which calls `checkAuth()` on mount → another verify. 3. The same page calls `useIsAdmin()` from `lib/admin-auth.js`, which calls its inline `checkAdmin()` on mount → another verify. Post-convoy: 1. `` is gone. 2. `pages/card/[id].js` calls `useAuth()` once → 1 verify. 3. `useIsAdmin()` call site is gone; admin status is computed synchronously from the same `user` returned by step 2. Net: **3 → 1** verify roundtrip on `card/[id].js` mount. Other pages drop from **2 → 1** (no `useIsAdmin` involved, but `` was). The 1× pattern is the floor; further reduction would require server-side hydration of the user object, which is a separate architectural conversation (out of scope; see Follow-ups). ### D5 — Test impact. Verdict: zero test files modified. The 21-test vitest suite covers: - `test/lib/auth-secret.test.js` (3) — server-side, untouched by this convoy. - `test/lib/permission-middleware.test.js` (8) — server-side, untouched. - `test/api/auth-utils.test.js` (5) — server-side, untouched. - `test/components/Layout.test.js` (5) — passes `user` as a *prop*, not via any hook. The legacy `auth-context` and `admin-auth` modules are not imported. Unaffected. All four files were `grep`-checked for `auth-context|admin-auth|use-auth` references — zero hits. No test was written against the legacy hooks themselves; the deletion is risk-free from a test-suite perspective. Vitest stays green at 21/21 post-convoy. ## Importer inventory Generated via `rg "from ['\"].*lib/auth-context['\"]" --type js` and `rg "from ['\"].*lib/admin-auth['\"]" --type js` against the worktree (excluding docs / convoys). ### Importers of `lib/auth-context.js` (6 source files) | File | Symbol | Migration | | --- | --- | --- | | `pages/_app.js` | `AuthProvider` | Wrapper removed; no replacement (D3) | | `pages/index.js` | `useAuth` | Path swap → `lib/use-auth.js` | | `pages/scanner.js` | `useAuth` | Path swap → `lib/use-auth` | | `pages/decks.js` | `useAuth` | Path swap → `lib/use-auth` | | `pages/deck/[id].js` | `useAuth` | Path swap → `lib/use-auth` (depth `../../`) | | `pages/deck-builder.js` | `useAuth` | Path swap → `lib/use-auth` | All 5 page-level `useAuth` consumers destructured only `{ user }` or `{ user, loading }` (verified by grep). No `login` / `register` / other-method consumer found, confirming D1. ### Importers of `lib/admin-auth.js` (1 source file) | File | Symbol | Migration | | --- | --- | --- | | `pages/card/[id].js` | `useIsAdmin` | Replaced with `user?.role === 'admin'` from existing `useAuth()` (D2) | `AdminProvider` and `useAdmin()` had **zero** importers in the source tree — confirming they were dead exports. ### Adjacent doc / config edits | File | Change | | --- | --- | | `pages/_app.js` | Removed `import { AuthProvider } from '../lib/auth-context.js'` and the wrapper | | `.github/CODEOWNERS` | Removed the two CODEOWNERS lines for the deleted files | | `AGENTS.md` § 2 + § 3 | Updated the Auth row of the architecture table and the "Auth (client)" convention bullet to describe the post-convoy single-surface state | | `.cursor/rules/auth-and-permissions.mdc` | Reframed § "Legacy" to "deleted by this convoy"; updated § "Authentication state on the client" to the post-convoy `useAuth()` shape and the direct-fetch login flow | | `.cursor/rules/no-go-zones.mdc` | Auth-refactors bullet updated to drop the deleted files | | `.cursor/skills/add-page/SKILL.md` | Updated checklist bullet + anti-pattern row to refer to the deletion | `.convoys/**` and `.convoys/fix-layout-default-user/**` were **not** edited — those are historical convoy records and are append-only by repo convention. The doc-writer post-convoy sweep will add the as-shipped section at the bottom of this file plus update `.convoys/ship-readiness.md` § P1 → entry 9 with the squash commit reference. ## The fix (per-category translation rules) ### Category A — `useAuth` from `auth-context` → `useAuth` from `use-auth` ```js // before import { useAuth } from '../lib/auth-context'; // or auth-context.js // after import { useAuth } from '../lib/use-auth'; // or use-auth.js ``` The destructure pattern (`const { user } = useAuth()` / `const { user, loading } = useAuth()`) stays byte-identical. No call-site changes. ### Category B — `AuthProvider` wrapper in `_app.js` ```jsx // before import { AuthProvider } from '../lib/auth-context.js'; return ( ); // after return ( ); ``` Plus delete the import line. ### Category C — `useIsAdmin` in `pages/card/[id].js` See D2 for the full diff. Three line-ranges touched: the import block, the `useAuth` destructure, and the `useIsAdmin` line block. Usage at line 524 is unchanged. ### Category D — `useAdmin`, `AdminProvider` No call sites. No work to do; these symbols disappear when the file is deleted. ## Verification plan 1. **`rg "lib/auth-context|lib/admin-auth" --type js`** → expect zero hits in `pages/`, `lib/`, `components/`. Achieved. 2. **`npm run lint`** → baseline 128 problems pre-convoy → 125 problems post-convoy (3 fewer errors, since the deleted files contained 3 unused-import / unused-var lints; no new lint surface introduced). No regression. 3. **`npm run test:run`** → 21/21 pass pre- and post-convoy. Layout test confirmed unaffected. 4. **`npm run build`** → succeeds end-to-end. All 26 pages compile (10 dynamic API routes + 16 `pages/**` views including `card/[id]`, `_app`, `decks`, `deck/[id]`, `deck-builder`, `scanner`, `index` — every file modified by the sweep). No SSR-level breakage; importantly no "useAuth must be used within an AuthProvider" runtime error during static generation, which would have indicated the page tried to use the legacy context hook unwrapped. 5. **Manual smoke:** _deferred_ — the build pass + vitest pass + zero-hit grep is the gate for merging; the parent does not have a logged-in admin browser session ready in this conversation. Documenting in As-shipped post-merge once the operator runs `npm run dev` and exercises dashboard / profile / settings / collections / cards / admin/card-editor. ## Risks - **R1 — Shape parity gap breaks runtime auth state.** *Mitigated by D1.* The grep audit confirmed no consumer reads `login` / `register` / any other surface that exists on the legacy hook but not on `use-auth`. `loading` and `user` were preserved with identical semantics. - **R2 — SSR mismatch from removing ``.** *Mitigated.* `lib/use-auth.js` reads `localStorage` inside a `useEffect`, so SSR sees `user === null, loading === true` and never touches the browser-only API on the server — same guarded shape as the legacy provider. `npm run build` confirms no SSR error during static generation. (`auth-context.js`'s `useEffect` had the same guard, so removing the provider didn't change the SSR surface.) - **R3 — Missed importer.** *Mitigated.* Post-delete grep over `--type js` returned zero hits. The deletion would itself surface any missed importer at module-load time during `npm run build` (Node would throw "Cannot find module"); build succeeded. - **R4 — Verify-roundtrip dedup creates a regression where a page never re-verifies.** *Mitigated.* Pre-convoy, three providers each ran their own verify on mount but they did not coordinate state — one provider's success had no effect on another's loading flag. Post-convoy we have a single source of truth. Pages that need to re-verify (e.g. after an action that might have invalidated the token) can call `refreshAuth()` from the same hook; no consumer currently does this, but the surface is preserved for future use. - **R5 — Stale `useAuth` cache across components.** *Out of scope; see Follow-ups.* Each `useAuth()` call site instantiates its own state via `useState`. Two components on the same page that both call `useAuth` will issue two verify roundtrips and hold two independent `user` references. This was true pre-convoy too (the legacy `useIsAdmin` was already a separate verify). Hoisting state into a shared module-level cache or wrapping `useAuth` in a context (the very thing we just removed!) is a separate decision — see "Follow-ups". ## As-shipped _Stub for doc-writer post-merge:_ - Squash commit: `` - PR: #`` - Files changed: 13 (2 deletions: `lib/auth-context.js`, `lib/admin-auth.js`; 11 modifications: `pages/_app.js`, `pages/index.js`, `pages/scanner.js`, `pages/decks.js`, `pages/deck/[id].js`, `pages/deck-builder.js`, `pages/card/[id].js`, `.github/CODEOWNERS`, `AGENTS.md`, `.cursor/rules/auth-and-permissions.mdc`, `.cursor/rules/no-go-zones.mdc`, `.cursor/skills/add-page/SKILL.md`). - Verify roundtrip count: documented 3 → 1 on `card/[id].js`, 2 → 1 on every other page-load. - `.convoys/ship-readiness.md` § P1 entry 9 to be marked RESOLVED with this convoy's squash SHA. - Lint baseline updated 128 → 125 (no regression; 3 fewer errors from deleted unused-import lines). ## Follow-ups (out of scope here) - **Component-level `useAuth` cache audit.** Two components on the same page that both call `useAuth()` will issue two verify roundtrips. This was the original motivation for the legacy context, and was the *one* legitimate thing those providers did right. A future convoy should consider either (a) returning a shared module-level state via a small Zustand-style store, (b) reintroducing a thin `` that *only* hoists state without re-implementing fetch logic, or (c) accepting the duplicate roundtrip as the price of hook-only simplicity. Today's call sites already deduplicate at the page level (one `useAuth` per page is the prevailing pattern), so this is a soft optimisation, not a correctness fix. - **Rate-limit-aware re-auth on 429.** `lib/use-auth.js`'s `checkAuth` does not currently back off if `/api/auth/verify` returns 429 (the rate-limiter from `add-rate-limiting` would only kick in if a single client exceeded 60 verify calls / minute, which is unrealistic in practice but worth a defensive guard). - **Server-side hydration of user.** The page-mount verify roundtrip is unavoidable in this hook-only shape because the token is only readable on the client. Moving to an HTTP-only cookie + Next.js `getServerSideProps` hydration would eliminate the round-trip entirely and is a larger architectural conversation that should not piggyback on a quality convoy. - **Doc-writer cleanup.** Update `.convoys/ship-readiness.md` § P1 → entry 9 with the RESOLVED stamp + squash SHA; trim the "three parallel surfaces" framing from any other doc that still mentions it; refresh the "Auth refactors" no-go-zones bullet if any other files become canonical (none today).