ui/_docs/03_implementation/reviews/batch_13_review.md

Code Review Report — Batch 13

Batch: AZ-510 (Auth bootstrap refresh consolidation) Cycle: 3 Date: 2026-05-13 Verdict: PASS

---

Phase 1: Context Loading

• Task spec: _docs/02_tasks/todo/AZ-510_auth_bootstrap_consolidation.md — replace broken GET /api/admin/auth/refresh (no credentials:'include') with POST /api/admin/auth/refresh (with credentials) chained to GET /api/admin/users/me. Closes Finding B3 / Vision P3.
• Architecture vision principle P3 (bearer in memory, refresh in HttpOnly cookie) requires the bootstrap path to send the HttpOnly refresh cookie; the prior code violated this.
• Architecture compliance baseline (_docs/02_document/architecture_compliance_baseline.md) carries B3 as the open downstream item AZ-510 was created to close.

Phase 2: Spec Compliance

AC	Mechanism	Test Evidence
AC-1 — POST refresh + credentials:'include', no GET refresh	runBootstrap() direct fetch(..., {method:'POST', credentials:'include'}) (AuthContext.tsx:45-48)	AuthContext.test.tsx FT-P-01 asserts method, credentials, chain
AC-2 — Successful refresh chains to /users/me and resolves loading:false	setToken(refreshData.token) then api.get<AuthUser>(endpoints.admin.usersMe()) (AuthContext.tsx:51-53); setUser(result) + setLoading(false) (:78-79)	FT-P-01 asserts usersMeHits === 1; getToken() becomes BEARER in NFT-SEC-01
AC-3 — Failed refresh → /login exactly once, no flash	if (!refreshRes.ok) return null (:49) → setUser(null) + setLoading(false) (:78-79)	ProtectedRoute.test.tsx covers spinner→/login paths under POST-refresh handlers
AC-4 — /users/me failure clears bearer + logs	try/catch around api.get calls setToken(null) + console.error + returns null (:54-61); top-level .then then sets user:null + loading:false	New AC-4 (AZ-510) test in AuthContext.test.tsx:108-138 asserts getToken() becomes null, console.error carries "/users/me failed"
AC-5 — Returning user not bounced to /login	Successful bootstrap path sets user before loading:false; ProtectedRoute only redirects when !loading && !user	Implicit in ProtectedRoute.test.tsx admin-route success cases (no /login rendered)
AC-6 — 401-retry path unchanged	runBootstrap uses direct fetch, not api; api/client.ts:73-98 unchanged	NFT-SEC-01 exercises bootstrap → 401 on /users/me → POST refresh rotation → replay; FT-P-03 covers refresh transparency

Constraints:

• C1 getApiBase() is the only base-URL source — honored (:45).
• C2 No api.post() for refresh — honored; uses direct fetch per the same comment in api/client.ts:88.
• C3 MSW handlers exercise production paths — honored; no vi.mock('api/client').
• C4 setToken(null) precedes setUser(null) on every failure path — honored:
  • /users/me failure: setToken(null) (:59) → return null → top-level setUser(null) (:78).
  • Outer fetch reject: setToken(null) (:87) → setUser(null) (:88).

Risk 4 (StrictMode double-mount): addressed via module-scoped bootstrapInflight promise (AuthContext.tsx:25, 70-74). Test-only escape hatch __resetBootstrapInflightForTests exported via the src/auth barrel and called in tests/setup.ts afterEach to prevent inter-test promise leakage (was the proximate cause of ProtectedRoute.test.tsx hangs during implementation).

No spec-gap findings.

Phase 3: Code Quality

• SOLID / SRP: runBootstrap has one responsibility (refresh + chain + clear-on-failure); AuthProvider's effect orchestrates the inflight guard and react state — clean separation.
• Error handling: explicit try/catch around /users/me; outer .catch handles network errors on the POST refresh itself. Both log via console.error with diagnostic prefix. No bare catches introduced. (Pre-existing try { await api.post(authLogout()) } catch {} in logout is out of scope.)
• Naming: bootstrapInflight, runBootstrap, __resetBootstrapInflightForTests are precise and self-documenting. Test export name carries the __…ForTests convention.
• Defensive hasPermission: user?.permissions?.includes(perm) ?? false — correctly guards against legacy /users/me payloads that omit permissions. Required because several existing test fixtures returned the bare User shape without permissions.
• Comments: comments explain why (StrictMode race, CORS posture for api.post, Constraint #4 ordering) — not what. Conforms to coderule.mdc.
• Test quality: AC-4 test asserts getToken() === null AND that console.error was called with the diagnostic prefix — meaningful state + log assertion, not just "no throw".

No findings.

Phase 4: Security Quick-Scan

• No hardcoded secrets, no SQL/string-interp queries, no eval/exec.
• console.error('[AuthContext] Refresh succeeded but /users/me failed:', err) logs the error object. The error object originates from api.get which throws a structured error without bearer material; the bearer was set via setToken before the try block but is not in the thrown error. No bearer leak.
• HttpOnly refresh cookie continues to flow via credentials:'include' — never touched in JS. NFT-SEC-02 explicitly verifies document.cookie carries no refresh-prefixed cookie.

No findings.

Phase 5: Performance

• Two sequential network calls (POST refresh → GET /users/me) on every cold mount. Spec NFR budgets 200 ms p95 for the chain on dev compose; same nginx/auth/host. Within budget.
• Module-scoped inflight promise prevents double-bootstrap under StrictMode dev double-mount, removing the wasted second round-trip.

No findings.

Phase 6: Cross-Task Consistency

Single-task batch — N/A.

Phase 7: Architecture Compliance

Check	Result
Layer direction	src/auth/AuthContext.tsx imports from ../api (barrel) and ../types only — auth → api allowed per architecture
Public API respect	All cross-component imports go through src/api/index.ts and src/types/index.ts barrels; no deep imports
New cyclic deps	None introduced
Duplicate symbols	None
Cross-cutting in component dir	bootstrapInflight is auth-specific state; correctly lives in the auth component

STC-ARCH-01 (cross-component deep imports) static gate: passed after fixing the tests/setup.ts → src/auth/AuthContext deep import by re-exporting __resetBootstrapInflightForTests from src/auth/index.ts (barrel) and switching the import to ../src/auth.

STC-ARCH-02 (no hardcoded API literals) static gate: passed; new endpoints.admin.usersMe builder added (src/api/endpoints.ts) and used at the only callsite.

Baseline Delta

Status	Finding	Notes
Resolved	B3 — Auth bootstrap missing credentials:'include'	Was open in _docs/02_document/04_verification_log.md; bootstrap now POST + credentials:'include' + chained /users/me.
Carried over	(none in this file's scope)	—
Newly introduced	(none)	—

Verdict

PASS — no Critical / High / Medium / Low findings. All ACs covered with tests; constraints honored; static and fast profiles green (231 passed, 13 quarantined skips unchanged); Finding B3 resolved.