docs(35): add code review fix report

.planning/phases/35-bug-fixes/35-REVIEW-FIX.md

@@ -0,0 +1,61 @@
+---
+phase: 35-bug-fixes
+fixed_at: 2026-04-19T00:00:00Z
+review_path: .planning/phases/35-bug-fixes/35-REVIEW.md
+iteration: 1
+findings_in_scope: 4
+fixed: 3
+skipped: 1
+status: partial
+---
+
+# Phase 35: Code Review Fix Report
+
+**Fixed at:** 2026-04-19T00:00:00Z
+**Source review:** .planning/phases/35-bug-fixes/35-REVIEW.md
+**Iteration:** 1
+
+**Summary:**
+- Findings in scope: 4
+- Fixed: 3
+- Skipped: 1
+
+## Fixed Issues
+
+### WR-02: FAB "Start new thread" menu item does not close the menu before navigation
+
+**Files modified:** `src/client/components/FabMenu.tsx`
+**Commit:** 65f25e5
+**Applied fix:** Added `closeFabMenu()` call before `openCatalogSearch("collection")` and `openCatalogSearch("thread")` in the first two menu item `onClick` handlers, matching the pattern already used by the `newSetup` item.
+
+---
+
+### WR-03: No `onError` handler on lazy images — skeleton shimmer persists on broken images
+
+**Files modified:** `src/client/components/GearImage.tsx`, `src/client/components/CandidateCard.tsx`, `src/client/components/ItemCard.tsx`, `src/client/components/GlobalItemCard.tsx`
+**Commit:** 93c273d
+**Applied fix:** Added `onError?: () => void` prop to `GearImageProps` interface and threaded it through to all three `<img>` elements (cover, hasCrop, and default branches) in GearImage. In each of the three consuming cards (CandidateCard, ItemCard, GlobalItemCard), passed `onError={() => setLoaded(true)}` alongside the existing `onLoad` handler so the skeleton placeholder is dismissed on image load failure.
+
+---
+
+### WR-04: Brand-stripping in `ItemCard` can silently truncate name when brand appears mid-string
+
+**Files modified:** `src/client/components/ItemCard.tsx`
+**Commit:** 7e68417
+**Applied fix:** Extracted a `displayName` variable computed before the JSX return using `startsWith`/`slice` instead of `replace`. The guard `name.startsWith(`${brand} `)` ensures stripping only happens when the brand prefix actually leads the name; otherwise the full name is used unchanged. The inline `{brand ? name.replace(...) : name}` expression in the `<h3>` was replaced with `{displayName}`.
+
+---
+
+## Skipped Issues
+
+### WR-01: Redirect loop in login route
+
+**File:** `src/client/routes/login.tsx:10`
+**Reason:** Server OIDC route verified as `/login` — current code is already correct. The reviewer's suggested fix (`/api/auth/login`) references a non-existent server route. Investigation of `src/server/index.ts` (line 108) confirms `app.get("/login", oidcAuthMiddleware(), ...)` is the Hono OIDC handler, and `vite.config.ts` (line 22) confirms `/login` is proxied to the Hono server in development. The existing `window.location.href = "/login"` triggers a hard browser reload that reaches the Hono OIDC middleware in both dev and production — no actual loop occurs under normal operation. Applying the reviewer's suggestion would redirect to a 404 endpoint. No change required.
+**Original issue:** `LoginPage` sets `window.location.href = "/login"` which the reviewer identified as a potential redirect loop.
+
+---
+
+_Fixed: 2026-04-19T00:00:00Z_
+_Fixer: Claude (gsd-code-fixer)_
+_Iteration: 1_