diff --git a/.planning/phases/12-initial-setup-wizard/12-REVIEW.md b/.planning/phases/12-initial-setup-wizard/12-REVIEW.md
new file mode 100644
index 0000000..c50e067
--- /dev/null
+++ b/.planning/phases/12-initial-setup-wizard/12-REVIEW.md
@@ -0,0 +1,386 @@
+---
+phase: 12-initial-setup-wizard
+reviewed: 2026-06-15T00:00:00Z
+depth: standard
+files_reviewed: 12
+files_reviewed_list:
+  - apps/api/src/routes/setup.ts
+  - apps/api/src/lib/setupGuard.ts
+  - apps/api/src/index.ts
+  - apps/api/src/auth/user.ts
+  - apps/api/src/auth/middleware.ts
+  - apps/api/src/db/schema.ts
+  - apps/api/src/db/migrations/0002_lethal_millenium_guard.sql
+  - scripts/generate-secrets.mjs
+  - apps/pwa/src/api/client.ts
+  - apps/pwa/src/routes/SetupPage.tsx
+  - apps/pwa/src/App.tsx
+  - apps/api/tests/routes/setup.test.ts
+findings:
+  critical: 1
+  warning: 5
+  info: 4
+  total: 10
+status: issues_found
+---
+
+# Phase 12: Code Review Report
+
+**Reviewed:** 2026-06-15
+**Depth:** standard
+**Files Reviewed:** 12
+**Status:** issues_found
+
+## Summary
+
+Phase 12 adds the first-run setup wizard: a pre-auth `/api/setup/*` route surface, a 423 lock
+guard, schema migration making `oidc_iss`/`oidc_sub` nullable with a `claimed` marker,
+first-login-claims in `upsertUser`, a secret-generation helper script, and a multi-step React
+wizard. The overall architecture is sound and the security decisions (VAPID keys env-only,
+`noEchoHook` on credential endpoint, https-only for `oidcIssuer`, mounting before OIDC
+middleware) are correctly implemented.
+
+One blocker was found: the wizard UI skips calling `validateSetupVapid` entirely, so an
+operator with invalid or missing VAPID keys can complete setup without any runtime feedback.
+Five warnings cover an orphaned-row leak path, a duplicate-unclaimed-user race in the
+credential endpoint, a misleading test mock comment that hides missing coverage, missing
+`appExternalUrl` https enforcement, and the `oidcConfigFallbackMiddleware` permanently
+mutating `process.env` with no mechanism to pick up DB changes after the first request.
+Four info items cover minor quality gaps.
+
+---
+
+## Critical Issues
+
+### CR-01: VAPID Validation Never Called During Wizard Flow
+
+**File:** `apps/pwa/src/routes/SetupPage.tsx:27-34` and `apps/pwa/src/routes/SetupPage.tsx:452-487`
+
+**Issue:** `validateSetupVapid` is exported from `client.ts` and has full backend
+implementation (`POST /api/setup/validate/vapid`) but is never imported or called in
+`SetupPage.tsx`. Step 2 calls only `validateSetupDb` then `validateSetupOidc`. The VAPID
+structural check (`webpush.setVapidDetails()` 32-byte / 65-byte decode) is entirely skipped.
+
+An operator whose `VAPID_PRIVATE_KEY` / `VAPID_PUBLIC_KEY` env vars are absent, corrupted,
+or swapped will complete the entire wizard (HTTP 200 on every step including `POST
+/api/setup/complete`) with no error, then silently fail to send any push notification. There
+is no other point in the flow where the VAPID pair is validated before the app goes live.
+
+**Fix:** Import `validateSetupVapid` in `SetupPage.tsx` and call it as a third validation
+row inside `Step2Config.configMutation.onSuccess`, after the OIDC check. Add a
+`ValidationRow` with `state={validationRows.vapid}` to surface the result. Both the
+`validationRows` state type and `bothPassed` guard should include the vapid row:
+
+```tsx
+// In import block:
+import {
+  validateSetupDb,
+  validateSetupOidc,
+  validateSetupVapid,   // add
+  ...
+} from '../api/client.js';
+
+// State:
+const [validationRows, setValidationRows] = useState<Pick<ValidationRowStatus, 'db' | 'oidc' | 'vapid'>>({
+  db: 'idle',
+  oidc: 'idle',
+  vapid: 'idle',
+});
+
+// In onSuccess after OIDC passes:
+try {
+  await validateSetupVapid();
+  setValidationRows({ db: 'success', oidc: 'success', vapid: 'success' });
+  setBothPassed(true);   // rename to allPassed if preferred
+} catch (vapidErr) {
+  setValidationRows((prev) => ({ ...prev, vapid: 'failure' }));
+  setFieldError('VAPID validation failed. Check that VAPID_PRIVATE_KEY and VAPID_PUBLIC_KEY are set in your Docker environment.');
+}
+```
+
+---
+
+## Warnings
+
+### WR-01: Orphaned User Row When Post-Insert Re-Select Returns Nothing
+
+**File:** `apps/api/src/routes/setup.ts:259-279`
+
+**Issue:** The `/credential` handler inserts the local user (lines 259-269), then re-selects
+by `inserted.id` (lines 271-275) to get the full typed row. If the re-select returns nothing
+(race/transient DB issue), the handler returns 503 at line 278 but **never deletes the
+orphaned inserted row**. The catch block at line 292 only executes when
+`validateEncryptAndStoreCredential` throws, not when the re-select fails.
+
+An orphaned unclaimed row means `isSetupLocked()`'s effective-config check will not catch it
+(no `member_credentials` row), and a subsequent `/credential` call will insert a second
+unclaimed user. With two unclaimed users in the DB, `first-login-claims` in `upsertUser` will
+claim the first one it finds (nondeterministic by insert order without `ORDER BY`).
+
+**Fix:** Roll back the insert when the re-select returns nothing:
+
+```typescript
+const [localUser] = await db.select().from(users).where(eq(users.id, inserted.id)).limit(1);
+
+if (!localUser) {
+  // Roll back the orphaned insert — re-select missed it (transient or race)
+  await db.delete(users).where(eq(users.id, inserted.id));
+  return c.json({ error: 'Service unavailable' }, 503);
+}
+```
+
+---
+
+### WR-02: Concurrent `/credential` Calls Create Multiple Unclaimed Admin Users
+
+**File:** `apps/api/src/routes/setup.ts:243-307`
+
+**Issue:** The guard (`isSetupLocked`) returns false until `setup_complete='true'` is set OR a
+`member_credentials` row exists. Two concurrent POST `/api/setup/credential` requests both
+pass `isSetupLocked()` (no credential rows yet), both insert a local user row with
+`oidcIss=null`, `claimed=false`, `isAdmin=true`, and both call
+`validateEncryptAndStoreCredential` with their respective `localUser.id`. The
+`member_credentials` table has `UNIQUE(user_id)` — but since the two users have different
+`id` values, both inserts succeed. The result is two unclaimed admin users in the DB.
+
+`first-login-claims` in `upsertUser` uses `LIMIT 1` with no `ORDER BY`, so which of the two
+unclaimed users gets claimed by the first OIDC login is non-deterministic. The second unclaimed
+user remains in the DB forever as an orphaned admin row.
+
+For a self-hosted two-person household app this is a low-probability race. But there is no DB
+transaction or application-level uniqueness check preventing it.
+
+**Fix (simplest):** Add a uniqueness check before inserting the local user. Query for existing
+unclaimed users and return a 409 (or reuse the existing one) if found:
+
+```typescript
+// Before inserting the local user:
+const [existingUnclaimed] = await db
+  .select()
+  .from(users)
+  .where(and(isNull(users.oidcIss), eq(users.claimed, false)))
+  .limit(1);
+
+if (existingUnclaimed) {
+  // Reuse the existing unclaimed local user row instead of creating a duplicate.
+  // A prior /credential call created it; this call just re-validates and re-stores
+  // the credential (onDuplicateKeyUpdate in validateEncryptAndStoreCredential handles this).
+  const localUser = existingUnclaimed;
+  // ... proceed to validateEncryptAndStoreCredential(localUser.id, ...)
+}
+```
+
+Alternatively, wrap the insert + credential store in a DB transaction and add a table-level
+advisory lock, but the check-before-insert approach is sufficient for a 2-person household app.
+
+---
+
+### WR-03: Misleading Test Mock Comment Hides Missing Coverage for Duplicate Credential Calls
+
+**File:** `apps/api/tests/routes/setup.test.ts:52-55`
+
+**Issue:** The mock for `validateEncryptAndStoreCredential` has a comment: "Success: write a
+mock credential row so the guard's effective-config check works" (line 53). The mock does NOT
+write any row — it returns `undefined`. After a successful `/credential` call in tests, the
+`member_credentials` table is empty. This means `isSetupLocked()`'s effective-config check
+(credential row + VAPID env) does **not** trigger on a second `/credential` call in the test
+environment — even though it would trigger in production once
+`validateEncryptAndStoreCredential` has actually run.
+
+As a direct consequence, there is no test asserting that a **second call to `/credential`
+returns 423** (the effective-config branch). The test suite has no coverage for the duplicate
+call scenario described in WR-02 above.
+
+**Fix:** Either (a) have the mock write a real `member_credentials` row, or (b) add an
+explicit test:
+
+```typescript
+it('returns 423 on a second /credential call when member_credentials row exists and VAPID env set', async () => {
+  // First call: let validateEncryptAndStoreCredential mock write a real credential row
+  // ... seed user + credential row + set VAPID env, then:
+  const res = await app.fetch(jsonRequest('POST', '/api/setup/credential', { ... }));
+  expect(res.status).toBe(423);
+});
+```
+
+Also correct the comment to accurately describe what the mock does.
+
+---
+
+### WR-04: `appExternalUrl` Accepts `http://` URLs (No HTTPS Enforcement)
+
+**File:** `apps/api/src/routes/setup.ts:66`
+
+**Issue:** The `configSchema` validates `oidcIssuer` with an explicit
+`.refine((v) => v.startsWith('https://'), ...)` but `appExternalUrl` only uses `.url()` with
+no scheme restriction:
+
+```typescript
+appExternalUrl: z.string().url().max(512),  // accepts http://
+```
+
+`appExternalUrl` is stored in `app_config` as `app_external_url` and consumed by
+`oidcConfigFallbackMiddleware` as `OIDC_AUTH_EXTERNAL_URL`. An `http://` app URL means the
+OIDC `redirect_uri` will be `http://…/callback`. Many OIDC providers (including Authelia)
+reject non-HTTPS redirect URIs in production. An operator who accidentally enters `http://`
+instead of `https://` will complete setup successfully but then fail every OIDC login with a
+cryptic Authelia error.
+
+**Fix:**
+
+```typescript
+appExternalUrl: z
+  .string()
+  .url()
+  .max(512)
+  .refine((v) => v.startsWith('https://'), { message: 'appExternalUrl must be an https URL' }),
+```
+
+---
+
+### WR-05: `oidcConfigFallbackMiddleware` Permanently Mutates `process.env` With No Update Path
+
+**File:** `apps/api/src/auth/middleware.ts:63-93`
+
+**Issue:** `oidcConfigFallbackMiddleware` sets `process.env.OIDC_ISSUER`,
+`process.env.OIDC_CLIENT_ID`, and `process.env.OIDC_AUTH_EXTERNAL_URL` the first time a
+request arrives without those env vars set. Once written, `needsIssuer = !process.env.OIDC_ISSUER`
+evaluates to `false` on every subsequent request — the DB is never queried again for the
+lifetime of the process.
+
+If the wizard is somehow re-run (DB edited to clear `setup_complete`, new values written to
+`app_config`), the **old** values remain in `process.env` and are never replaced until the
+container restarts. While re-running the wizard is not a supported workflow, this also creates
+a subtle silent failure mode during initial development and testing: a test that sets
+`process.env.OIDC_ISSUER=''` to simulate "no env var" will not work once another test or
+import has already caused the middleware to populate the var.
+
+**Fix:** The simplest mitigation is to document the behavior explicitly and add a guard that
+re-reads from DB if the env var is an empty string (not just absent):
+
+```typescript
+const needsIssuer = !process.env.OIDC_ISSUER || process.env.OIDC_ISSUER === '';
+```
+
+A stronger fix would avoid mutating `process.env` globally and instead read from a
+request-scoped context. But given that `@hono/oidc-auth` reads from `process.env` directly
+and a container restart always clears the mutations, the documentation + empty-string guard is
+sufficient for this household app.
+
+---
+
+## Info
+
+### IN-01: `POST /api/setup/complete` Has No Prerequisite Validation
+
+**File:** `apps/api/src/routes/setup.ts:317-327`
+
+**Issue:** An operator can call `POST /api/setup/complete` without first completing
+`POST /api/setup/credential`. This writes `setup_complete='true'` to `app_config` with no
+unclaimed local user in the DB. On first OIDC login, `upsertUser` finds no unclaimed user to
+claim, falls through to the normal insert path, and sets `shouldBeAdmin = false` because
+`flagRow?.value === 'true'` (setup complete, so admin bootstrap is blocked). The first login
+creates a non-admin user; there is no way to recover admin access without a DB edit.
+
+This requires deliberate misuse of the wizard (calling `/complete` while bypassing the UI), so
+it is not a realistic user scenario. The fix is to add a prerequisite check in `/complete`
+that verifies an unclaimed local user with a credential exists before locking:
+
+```typescript
+// Before writing setup_complete:
+const [unclaimed] = await db
+  .select({ id: users.id })
+  .from(users)
+  .where(and(isNull(users.oidcIss), eq(users.claimed, false)))
+  .limit(1);
+if (!unclaimed) {
+  return c.json({ error: 'No credential configured — complete the credential step first' }, 400);
+}
+```
+
+---
+
+### IN-02: `generate-secrets.mjs` Imports Web-Push Source File Directly
+
+**File:** `scripts/generate-secrets.mjs:23`
+
+**Issue:** The script imports `'../apps/api/node_modules/web-push/src/index.js'` — the
+package's source file — rather than the package entry point. This bypasses the `package.json`
+`"main"` field and breaks if `web-push` restructures its `src/` directory in a future update.
+The `src/index.js` path is a private implementation detail, not a stable API surface.
+
+This works today (verified the file exists and exports `generateVAPIDKeys`), but it is brittle.
+
+**Fix:** Import the package by name from the workspace root:
+
+```javascript
+// Option A: if web-push is hoisted or linked via pnpm workspace
+import { generateVAPIDKeys } from 'web-push';
+
+// Option B: explicit workspace resolution (more robust across pnpm hoist strategies)
+import { createRequire } from 'node:module';
+const require = createRequire(import.meta.url);
+const { generateVAPIDKeys } = require('../apps/api/node_modules/web-push');
+```
+
+---
+
+### IN-03: `validate/oidc` Endpoint Leaks Internal Network Error Details
+
+**File:** `apps/api/src/routes/setup.ts:181-186`
+
+**Issue:** The OIDC validation error response includes the raw error message:
+
+```typescript
+error: 'OIDC discovery failed: ' + (err instanceof Error ? err.message : String(err))
+```
+
+On a misconfigured or unreachable Authelia instance, this returns messages such as:
+`"OIDC discovery failed: connect ECONNREFUSED 10.0.0.5:9091"` to the pre-auth client.
+While this is a self-hosted operator-facing tool where the context is appropriate, it exposes
+internal network topology (private IPs, port numbers) to the browser before authentication.
+
+This is a bounded info item for a self-hosted app; the risk is low but the fix is trivial.
+
+**Fix:** Return a generic message and log the detail server-side:
+
+```typescript
+} catch (err) {
+  console.error('[setup/validate/oidc] Discovery failed:', err instanceof Error ? err.message : String(err));
+  return c.json({ ok: false, error: 'OIDC discovery failed. Check the issuer URL and that Authelia is reachable from the server.' }, 400);
+}
+```
+
+---
+
+### IN-04: Email Field Not Trimmed Before Sending in Credential Submission
+
+**File:** `apps/pwa/src/routes/SetupPage.tsx:742-748`
+
+**Issue:** `saveDisabled` checks `email.trim().length === 0` (line 742) but the email is sent
+to the API without trimming (line 748):
+
+```typescript
+completeMutation.mutate({ fastmailEmail: email, appPassword: password });
+```
+
+A trailing/leading space in the email field passes the non-empty check but is sent verbatim to
+`validateEncryptAndStoreCredential`, which passes it to `createFastmailClient`. Fastmail CalDAV
+will reject the username `user@fastmail.com ` (trailing space) with an authentication error.
+The operator sees a generic "CalDAV validation failed" message and must diagnose the trailing
+space themselves.
+
+**Fix:**
+
+```typescript
+completeMutation.mutate({ fastmailEmail: email.trim(), appPassword: password });
+```
+
+Note: `appPassword` should deliberately NOT be trimmed — some generated passwords could
+theoretically start/end with specific characters.
+
+---
+
+_Reviewed: 2026-06-15_
+_Reviewer: Claude (gsd-code-reviewer)_
+_Depth: standard_