---
phase: 12-initial-setup-wizard
reviewed: 2026-06-15T00:00:00Z
depth: standard
files_reviewed: 16
files_reviewed_list:
  - apps/api/src/auth/middleware.ts
  - apps/api/src/auth/user.ts
  - apps/api/src/db/migrations/0002_lethal_millenium_guard.sql
  - apps/api/src/db/schema.ts
  - apps/api/src/index.ts
  - apps/api/src/lib/setupGuard.ts
  - apps/api/src/routes/setup.ts
  - apps/api/tests/auth/user.test.ts
  - apps/api/tests/routes/setup.test.ts
  - apps/pwa/src/api/client.ts
  - apps/pwa/src/api/setupClient.contract.test.ts
  - apps/pwa/src/App.test.tsx
  - apps/pwa/src/App.tsx
  - apps/pwa/src/routes/SetupPage.test.tsx
  - apps/pwa/src/routes/SetupPage.tsx
  - scripts/generate-secrets.mjs
findings:
  critical: 0
  warning: 0
  info: 0
  total: 0
status: clean
---

# Phase 12: Code Review Report (Final Re-review)

**Reviewed:** 2026-06-15T00:00:00Z
**Depth:** standard
**Files Reviewed:** 16
**Status:** clean

## Summary

Final re-review of all 16 Phase 12 files at standard depth, with targeted verification of the WR-01 fix landed in commit 687f9dc and confirmation that all prior findings remain resolved.

**WR-01 is genuinely resolved.** The fix is correct and complete on both required axes:

1. `upsertUser` now explicitly inserts fresh OIDC users with `claimed: true` (`apps/api/src/auth/user.ts:172-173`). An OIDC-created user is identity-bound at insert time and cannot be mistaken for a pending wizard bootstrap row.

2. The POST /credential TOCTOU guard now filters `WHERE oidc_iss IS NULL AND claimed = false FOR UPDATE` (`apps/api/src/routes/setup.ts:270`), narrowed to match only local wizard users — not OIDC users that might hypothetically carry `claimed=false` on legacy or partially-bootstrapped data.

3. The first-login-claims CLAIM path in `upsertUser` is not regressed. That path matches `isNull(users.oidcIss) AND eq(users.claimed, false)` (user.ts:115) — a pending wizard row has `oidcIss=NULL` and `claimed=false`, satisfying both predicates. A fresh OIDC insert now has `oidcIss` set (non-null), so it cannot satisfy `isNull(users.oidcIss)` and will never be mistaken for a claimable wizard row.

4. The migration (`0002_lethal_millenium_guard.sql`) backfills all existing OIDC users (`WHERE oidc_iss IS NOT NULL`) to `claimed=true`, covering any rows created before this fix.

5. Two new tests cover both sides of the fix: `user.test.ts:449` asserts `insertValues.claimed === true` on a fresh OIDC insert; `setup.test.ts:487` seeds an OIDC user with `claimed=false` and asserts the credential step still returns 200, confirming the narrowed guard does not false-positive.

**All prior findings remain resolved.** CR-01 (effective-config lock-out), IN-01 (https enforcement on appExternalUrl), WR-02 (TOCTOU FOR UPDATE concurrency), and all five original findings show no regressions.

All reviewed files meet quality standards. No issues found.

---

_Reviewed: 2026-06-15T00:00:00Z_
_Reviewer: Claude (gsd-code-reviewer)_
_Depth: standard_