visionA/local-tool/.autoflow/05-implementation/review/m9-4-frontend-firmware-review.md

# Reviewer Report — M9-4 Frontend FW UI + Backend WS hot-fix

> 審查日期：2026-05-25
> Reviewer：Reviewer Agent
> 審查對象：M9-4 Frontend FW UI（12 新 + 4 修改 = 16 檔、3052 行）+ Backend WS hot-fix（2 新 + 1 修改）
> 審查層：A 層（per-task）+ B 層（多檔案 PR、18 檔 / >500 行）
> 依據文件：Design Spec v2/firmware-management.md（948 行）/ PRD feature-firmware-management.md / TDD v2/firmware-management.md

---

## TL;DR

- **審查結果：⚠️ 需修改後通過**
- **Critical：0 | Major：3 | Minor：8 | Suggestion：5**
- **是否阻擋 M9-5：否**（3 個 Major 都是 UX/i18n/test schema 層級、不阻擋三平台實機 E2E；M9-5 可平行開跑、Frontend 第 2 輪修這 3 項）
- **是否升 security agent：否**（5 軸 security 粗篩無發現、`navigator.clipboard?.writeText` 已守、無 XSS / hardcoded secret、loopback origin 由 backend 既有 `CheckOrigin` 共用）
- **是否需 frontend 第 2 輪**：是（3 個 Major + 部分 Minor 修完即可）
- **是否需 backend 第 2 輪**：否（hot-fix 對稱 flash_ws.go、smoke test 通過、Minor M5 屬 testing agent 範圍、可在 M9-5 testing 階段補）

---

## 審查範圍

| # | 檔案 | 行數 | 類別 |
|---|------|------|------|
| 1 | `frontend/src/types/device.ts` | +62 | Frontend / Types |
| 2 | `frontend/src/stores/firmware-store.ts` | 262（新） | Frontend / Store |
| 3 | `frontend/src/hooks/use-firmware-progress.ts` | 81（新） | Frontend / Hook |
| 4 | `frontend/src/components/firmware/firmware-badge.tsx` | 95（新） | Frontend / Component |
| 5 | `frontend/src/components/firmware/firmware-upgrade-button.tsx` | 51（新） | Frontend / Component |
| 6 | `frontend/src/components/firmware/firmware-upgrade-dialog.tsx` | 227（新） | Frontend / Component |
| 7 | `frontend/src/components/firmware/firmware-progress-view.tsx` | 109（新） | Frontend / Component |
| 8 | `frontend/src/components/firmware/firmware-error-view.tsx` | 150（新） | Frontend / Component |
| 9 | `frontend/src/components/firmware/index.ts` | 10（新） | Frontend / Barrel |
| 10 | `frontend/src/components/devices/device-card.tsx` | +X | Frontend / Modified |
| 11 | `frontend/src/lib/i18n/types.ts` | +71 | Frontend / i18n |
| 12 | `frontend/src/lib/i18n/zh-TW.ts` | +68 | Frontend / i18n |
| 13 | `frontend/src/lib/i18n/en.ts` | +68 | Frontend / i18n |
| 14 | `frontend/src/tests/stores/firmware-store.test.ts` | 265（新、22 tests）| Frontend / Test |
| 15 | `frontend/src/tests/components/firmware-badge.test.tsx` | 98（新、10 tests）| Frontend / Test |
| 16 | `server/internal/api/ws/firmware_ws.go` | 50（新） | Backend / Hot-fix |
| 17 | `server/internal/api/ws/firmware_ws_test.go` | 165（新、2 tests） | Backend / Test |
| 18 | `server/internal/api/router.go` | +2 | Backend / Modified |

**漏審檢查（R-B2）**：18 檔全部覆蓋、無漏審。

---

## 文件符合性檢查（R-A2）

### PRD 功能符合性

| PRD AC 項目 | 是否實作 | 實作位置 | 符合程度 | 備註 |
|------------|---------|----------|---------|------|
| AC-FW-1.1 FW badge 4 色（綠 / 黃 / 紅 / 灰）| ✅ | `firmware-badge.tsx:20-30` `computeBadgeState` | 完全 | legacy/unknown/current/older 四 state、優先序合理 |
| AC-FW-1.1 衍生欄位（`firmwareIsLegacy / firmwareCanUpgrade / bundledFirmwareVersion`）| ✅ | `types/device.ts:11-14` | 完全 | 與 TDD §3.1 一致 |
| AC-FW-1.2 升級按鈕（黃 / 紅 badge 條件）| ✅ | `firmware-upgrade-button.tsx:26` 用 `firmwareCanUpgrade` gate | 完全 | 條件由 backend 衍生欄位決定、Frontend 不自己 derive |
| AC-FW-1.2 progress modal stage 5-6（含 preparing/loading/flashing/verifying）| ✅ | `firmware-progress-view.tsx:42-57` | 完全 | 4 stage + done 對齊 §4.3 |
| AC-FW-1.2 4 階段 / 3 階段 split（KDP1→KDP2 vs KDP2→KDP2）| ✅ | `firmware-store.ts:213-243` `stageOrdinal` | 完全 | `isLegacyUpgrade` 切 4 vs 3 stage、跟 Design §5.3 對齊 |
| AC-FW-1.2 不可中斷警告 | ✅ | `firmware-upgrade-dialog.tsx:147-160` 拒絕 onOpenChange/ESC/outside-click | 完全 | R-FW-11 緩解、ESC + interactOutside 都防 |
| AC-FW-1.3 升級成功 toast + rescan | ✅ | `firmware-upgrade-dialog.tsx:85-107` | 完全 | success effect → showSuccess + fetchDevices + 1.5s 後關閉 |
| AC-FW-1.4 8 種 reason 對應 UI | ✅ | `firmware-store.ts:150-200` 三個 helper | 完全 | `errorMessageKeyFor` / `canRetryReason` / `primaryActionKeyFor` 三個 pure helper |
| AC-FW-1.4 「複製錯誤訊息」按鈕 | ✅ | `firmware-error-view.tsx:120-131` | 完全 | clipboard fallback 靜默 OK |
| AC-FW-1.4 技術資訊 collapsible | ✅ | `firmware-error-view.tsx:101-118` `<details>` 元素 | 完全 | rawError 在 collapsible 內、不在主畫面 |
| AC-FW-1.5 升級期間鎖住其他操作 | ✅ | `device-card.tsx:63-65` `disabled={isBusy}` | 完全 | 由 device-store 既有 `isBusy` 機制涵蓋 |
| AC-FW-1.6 升級後 rescan | ✅ | `firmware-upgrade-dialog.tsx:100` `fetchDevices()` | 完全 | 但 5-8s wait 在 backend 處理、frontend 不需 |
| AC-FW-1.7 KL520 ~30s / KL720 ~180s 預估 | ✅ | `firmware-store.ts:206-211` `estimatedDurationSeconds` | 完全 | chip 分流邏輯正確 |
| AC-FW-2 降版 UI（B 階段 M9-12）| N/A | — | — | 本批不在範圍 |
| AC-FW-3.5 KL630/KL730 升降版 | N/A | — | — | B 階段 M9-10、本批不在範圍 |

**PRD 功能完整度**：M9-4 範圍內 14/14 條 AC 全部實作、無遺漏。

### TDD 技術規格符合性

| TDD 規格項目 | 是否符合 | 實作位置 | 備註 |
|------------|---------|----------|------|
| §3.1 衍生欄位 `firmwareIsLegacy / firmwareCanUpgrade / bundledFirmwareVersion` | ✅ | `types/device.ts:11-14` | 與 backend 一致 |
| §3.4 stage → reason → i18n key 對應表（8 種）| ✅ | `firmware-store.ts:150-172` + i18n keys | 完全 mapping、附 stage-only fallback |
| §4.2 `FirmwareProgress` schema | ✅ | `types/device.ts:57-75` | json field 對應 backend struct（已驗 `server/internal/firmware/types.go:34-53` 完全對齊：`elapsedMs/etaMs/rawError/beforeVersion/afterVersion/errorCode/reason`）|
| §4.3 stage 命名（`preparing/loading/flashing/verifying/done/error`）| ✅ | `types/device.ts:35-41` | 完全對齊 |
| §4.2 失敗欄位（`reason/rawError/beforeVersion/errorCode`）| ✅ | `types/device.ts:67-72` | 完全 |
| §5.1 流程：先 connectAndWait → 再 POST API | ✅ | `firmware-upgrade-dialog.tsx:109-130` `handleStart` | 順序對、避免漏 event |
| WS endpoint `/ws/devices/:id/firmware-progress` | ✅ | `use-firmware-progress.ts:52` + `router.go:125` + `firmware_ws.go:30` | 三端命名一致 |
| WS room key `firmware:<deviceID>` | ✅ | `firmware_ws.go:30` + `firmware_handler.go:174` | 完全 |
| §8.6 Wails graceful shutdown — `GET /api/firmware/active-tasks` | ✅ | `firmware-store.ts:252-262` `fetchActiveFirmwareTasks` | A 階段預埋、實際 UI 由 Wails / control-panel M9-12 接 |

**TDD 規格符合度**：9/9 項全部符合。

### 設計規格符合性

| Design Spec 項目 | 是否符合 | 實作位置 | 備註 |
|----------------|---------|----------|------|
| §3.4 stage 命名（preparing/loading/flashing/verifying/done/error）| ✅ | `firmware-store.ts:36-41` | 完全 |
| §4.2 Badge 規格（pill / radius.full / 11px medium）| ✅ | `firmware-badge.tsx:86` `rounded-full px-2 py-0.5 text-xs font-medium` | 視覺對齊 |
| §4.2 Badge 顏色（綠 success / 黃 warning / 紅 destructive）| ⚠️ 部分 | `firmware-badge.tsx:38-43` 用 emerald/amber + semantic destructive/muted | 見 M2 — 暫代 Tailwind palette、Design §11.2 明示 6 個 component token 由 M9-12 落地、本批合理 |
| §5.1 升級確認 modal 寬度 480px | ⚠️ 部分 | 沿用 shadcn `Dialog` 預設 max-w-lg | 見 Suggestion S1 — 視覺檢查時可微調 |
| §5.2 升級進度 modal 不顯示 ✕ / 取消按鈕 | ✅ | `firmware-upgrade-dialog.tsx:154` `showCloseButton={!isInProgress}` | R-FW-11 緩解 |
| §5.3 stage 對應表（preparing=5% / loading=20% / flashing=50% / verifying=90% / done=100%）| ⚠️ | UI 信任 backend `progress.percent`、不 hardcode 數值 | 見 Suggestion S2 — 信任 backend 是對的、但 backend 是否真按表發 percent 是另一個 audit 項 |
| §5.4 成功 toast variant + 6s 停留 | ⚠️ | 用既有 `showSuccess`（既有 toast、停留時間未確認）| 見 Minor M4 |
| §6 降版 UI（B 階段）| N/A | — | 不在範圍 |
| §7.1 8 種 reason → friendly message | ✅ | `firmware-store.ts:errorMessageKeyFor` + i18n 8 個 key | 完全 mapping |
| §7.1 destructive reason 不提供 Retry | ✅ | `firmware-store.ts:179-186` + `firmware-error-view.tsx:91-98, 142-146` | `disconnect_during_op / verify_mismatch / verify_not_found` 三種不可 retry |
| §7.2 失敗 modal 含技術資訊 collapsible | ✅ | `firmware-error-view.tsx:101-118` | 對齊 |
| §7.2 技術資訊預設收合 | ✅ | `firmware-error-view.tsx:36` `useState(false)` | 對齊 |
| §9 i18n keys 51 個 + zh-TW / en 雙語 | ⚠️ 部分 | i18n 結構不照 §9 namespace（用 `firmware.error.messageXxx` flat 結構 vs Design 的 `settings.firmware.error.message.xxx` 巢狀）| 見 Major MJ1 — namespace 與 Design Spec §9 不同 |
| §11.2 對比比率（current 4.7:1 / older 5.2:1 / legacy 4.8:1）| ⚠️ 部分 | 暫代 Tailwind palette、註解明示 M9-12 落地正式 token | 由 M9-12 Frontend 用 axe-core 實測（Design §11.2 備註明示）|
| §12.1 R-FW-11 緩解（不可關 modal）| ✅ | `firmware-upgrade-dialog.tsx:146-160` 三層擋（onOpenChange / interactOutside / EscapeKeyDown）| 完全 |
| §12.2 R-FW-11.5（嚴格 `===` 比對「DOWNGRADE」）| N/A | — | 降版 B 階段、本批不適用 |
| §10 A11y（role / aria-label / aria-live）| ✅ | `firmware-progress-view.tsx:69-71, 84-88` + `firmware-error-view.tsx:74, 78` | `role="status" aria-live="polite"` 在進度區、`role="alertdialog"` 在 error modal、合理 |

**設計規格符合度**：13/16 項完全符合、3 項部分符合（其中 MJ1 是 Major、其餘是合理的暫代 / 範圍外）。

---

## 5 軸 + 測試軸審查（R-A1）

### 3.1 Correctness（正確性）

實質判斷（≥20 字）：實作正確性高、phase state machine 嚴謹、多裝置隔離（`activeDeviceId` mismatch return）對齊 flash-store M4 fix、WS connectAndWait 與 POST API 順序正確避免漏早期 event、destructive reason 不提供 Retry 安全防護到位、clipboard fallback 靜默 OK。發現的問題集中在 i18n namespace 對齊與測試 schema 偏差（見 Major / Minor）。

具體 finding：
- ✅ `firmware-store.ts:110-115` 多裝置隔離邏輯正確（`if (active !== null && active !== ev.deviceId) return`）
- ✅ `firmware-upgrade-dialog.tsx:109-130` `connectAndWait` 在 POST 之前、避免 WS 還沒 open 漏掉 backend 早期 event（對齊 flash-store pattern）
- ⚠️ `firmware-store.ts:248-249` test case `expect(estimatedDurationSeconds('unknown_device')).toBe(30)` 與函數實作不一致 — 函數 line 207-211 `if (!deviceType) return 60`，但 `'unknown_device'` 是 truthy 字串會走到 `if (low.includes('kl720')) return 180` → false → fallback return 30（KL520 路徑）；測試正確、註解標「預設走 KL520 路徑」、但 line 247-249 同時測 `undefined → 60` 和 `'unknown_device' → 30` 邏輯不一致（一個沒 type 走 60、有 type 但不認得走 30）。語意不直覺、但兩個 case 都通過、屬 minor 表達問題（見 Minor M3）

### 3.2 Readability（可讀性）

實質判斷：命名清晰、註解充足（每個元件頂部有 phase / 對齊文件條目註解）、TypeScript 型別嚴格（沒 `any`）、phase 機（confirming → upgrading → success / error）邏輯線性、helpers 從 store 分離為 pure functions（便於測試與 M9-12 重用）。控制流可讀、最深巢狀 `firmware-error-view.tsx:110-117` 的 template literal 較長但語意清楚。

具體 finding：
- ✅ `firmware-store.ts:147-243` pure helpers 區塊分離（無 React 依賴）
- ✅ 每個元件首段 JSDoc 都引用 Design / TDD 條目（追溯性強）
- ⚠️ `firmware-error-view.tsx:109-117` 技術資訊 template literal 較複雜（多層 `${cond ? a : ''}`）、可讀但不易調整（見 Suggestion S5）

### 3.3 Architecture（架構）

實質判斷：架構正確遵循既有 pattern — Zustand store + React hook + 純元件分離、與 flash-store 結構對稱、useFirmwareProgress 100% mimic useFlashProgress；backend hot-fix 對稱 flash_ws.go（同 upgrader、同 CheckOrigin、同 RegisterSync、同 read pump pattern）。Activity store reuse `flash_*` event types 屬合理跨領域共用（兩者語意都是 device 進行中操作）、雖然 type literal 字面是 `flash_*` 但語意通用。

具體 finding：
- ✅ `firmware-store.ts:102-106, 121, 127-129` reuse `flash_start / flash_complete / flash_error` activity types — 合理權衡、避免 activity 型別爆炸
- ✅ `firmware_ws.go` 與 `flash_ws.go` 結構幾乎 1:1 對稱、易維護
- ✅ 純 helpers 從 store 分離（`errorMessageKeyFor / canRetryReason / primaryActionKeyFor / estimatedDurationSeconds / stageOrdinal`）方便 M9-12 重用、且測試友善

### 3.4 Security（粗篩）

實質判斷：**5 軸 security 粗篩無發現、不升級給 Security Auditor**。

- ✅ **未 sanitize 的 HTML**：`firmware-error-view.tsx:108-118` 用 `<pre>{...template literal}` 渲染、React 自動 escape、無 dangerouslySetInnerHTML（已 grep `dangerouslySetInnerHTML / v-html`：零 hit）
- ✅ **raw error 不在主畫面**：rawError 只出現在 collapsible `<details>` 內（line 109-117）、預設收合（line 36）、Design §7.2 規格符合
- ✅ **clipboard 操作**：`firmware-error-view.tsx:61-70` 用 `navigator.clipboard?.writeText` optional chain 守、reject 路徑靜默不洩漏
- ✅ **WS Origin check**：backend `firmware_ws.go:23` 共用 package-level upgrader、`device_events_ws.go:14` 設定 `CheckOrigin: CheckOrigin`（loopback 白名單、`origin.go:18-40`）、Frontend 連 `/ws/devices/:id/firmware-progress` 與 backend 一致
- ✅ **無 hardcoded secret / token**：grep `password / secret / api_key / token`：零 hit（不算 `taskId / activeTaskId` 內部命名）
- ✅ **無 SQL 拼接**：本批無 SQL（純 WS + frontend state）
- ✅ **無 OAuth / 第三方 SDK 新增**：純內部 WS + REST

### 3.5 Performance（效能）

實質判斷：性能合理、無明顯瓶頸。

- ✅ Badge 在每張 card render 是 O(N) cost、`computeBadgeState` 是 pure function、無重 React.memo 需求（N 通常 < 10 dongles）
- ✅ Progress modal re-render 頻率受 WS event 控制、backend 每階段 push（typically < 5 events / 30-180s）、不會頻繁 re-render
- ✅ Zustand subscription 範圍合理、`useFirmwareStore()` 整 hook 拿、無 selector micro-optimization 必要（state 本身小、且 phase 變化即整個 modal 切換）
- ⚠️ `firmware-upgrade-dialog.tsx:45-55` 用 `useFirmwareStore()` 整個拿、不 selector — 一般 OK、但 progress event 每秒推時整個 dialog 重 render（含 confirming phase 的 UI 樹）；非問題、屬可接受 trade-off（見 Suggestion S3）

### 3.6 測試（測試軸）

實質判斷：測試覆蓋度高 — 22 + 10 = 32 個 frontend tests、覆蓋 phase 轉移 / 多裝置隔離 / cancelConfirm R-FW-11 緩解 / 8 種 reason mapping / 4-stage vs 3-stage / a11y attribute。Backend 2 個 smoke test 驗 broadcast + room isolation、合理。

具體 finding：
- ✅ 多裝置隔離測試（`firmware-store.test.ts:123-138`）— 直接模擬 dev-A active + dev-B event、驗 dev-B 不污染、品質高
- ✅ R-FW-11 緩解測試（`firmware-store.test.ts:58-67`）— 驗 `cancelConfirm` 在 `upgrading` phase 是 no-op
- ✅ destructive reason 不可 retry 測試（`firmware-store.test.ts:201-219`）— 3 種 destructive + 5 種 recoverable + undefined 都涵蓋
- ⚠️ 缺 `useFirmwareProgress` hook 的測試（WS connectAndWait timeout / cleanup）— 既有 `useFlashProgress` 也沒測（屬既有不嚴格 pattern）、可加但不阻擋
- ⚠️ `firmware_ws_test.go:62-67` 的測試 message schema 用 `phase: "flashing"` 是 ad-hoc map（不是真的 firmwareProgressMessage struct）、smoke test 只驗 plumbing 不驗 schema → 見 Major MJ3
- ⚠️ 缺 `FirmwareErrorView` 元件渲染測試（destructive reason 顯示 brick warning / Retry 按鈕隱藏）— 見 Minor M5
- ⚠️ 缺 `FirmwareUpgradeDialog` integration 測試（phase 切換時 modal 視覺與 R-FW-11 擋 ESC 行為）— 見 Suggestion S4

---

## §12.2 通用退出條件（6 條）

| # | 條件 | 觸發狀態 |
|---|------|---------|
| 1 | No silent failures | ✅ 未觸發：`firmware-store.ts:86-104` startUpgrade 失敗有 catch + addActivity 紀錄、`firmware-upgrade-dialog.tsx:112-128` connectAndWait 失敗有 catch + 進 error phase、`firmware-error-view.tsx:65-69` clipboard reject 是有意靜默（design 規格允許） |
| 2 | No dead code | ✅ 未觸發：grep TODO / FIXME / commented-out code、本批 0 hit；所有 import 都被使用 |
| 3 | No hardcoded secrets | ✅ 未觸發 |
| 4 | No unsafe HTML / SQL | ✅ 未觸發（見 3.4 security 軸） |
| 5 | Doc 同步 | ⚠️ **觸發**：i18n key namespace 與 Design Spec §9 不一致（見 MJ1） |
| 6 | Working tree clean | N/A：reviewer 自己只產 report、被審 commit 由 backend / frontend agent 負責、`git status` 不在 reviewer 範圍 |

---

## 🔴 Critical

**無 Critical 問題。**

---

## 🟠 Major（必須修復）

### MJ1：i18n key namespace 與 Design Spec §9 不一致

| 欄位 | 內容 |
|------|------|
| 檔案 | `frontend/src/lib/i18n/types.ts:140-198`、`zh-TW.ts:139-197`、`en.ts:139-197` |
| 規則 | Doc 同步 / Design Spec §9 對齊 |
| 違規程度 | Major（影響 M9-12 Settings 韌體面板 i18n 對齊 + Design Spec source-of-truth 失效） |
| 對應文件 | Design Spec §9.6 / §9.8 |
| 問題描述 | Design Spec §9.6 定義 `settings.firmware.progress.stage.preparing` 等 51 個 keys，全部住在 `settings.firmware.*` namespace 下；Frontend 實作改用 `firmware.error.messageScanNotFound` / `firmware.progress.stagePreparing` 等 flat 結構、住在頂層 `firmware.*` namespace。 |
| 影響 | (1) M9-12 Settings 韌體面板（B 階段）落地時、若照 Design §9 規格用 `settings.firmware.*` 會與本批 key 衝突或產生重複；(2) Design Spec §9 是 source of truth、未來文件 ↔ code 對照查 key 時找不到；(3) 51 keys vs 實際 ~28 keys 差距大（Design §9 含 settings 分頁 label / accordion / downgrade modal 等 B 階段 keys、A 階段確實不需全 51 個、但本批產的 keys 命名規則應對齊 Design namespace） |
| 建議修改方式 | 兩個方案二選一（讓 Orchestrator 與 Design 共同裁決）：<br>**A.** 把 i18n keys 搬到 `settings.firmware.*` namespace 下（雖然 A 階段不在 settings 頁、但對齊 Design 文件）：例如 `firmware.error.messageScanNotFound` → `settings.firmware.error.message.scanNotFound`<br>**B.** 維持 `firmware.*` flat 結構，但回頭請 Design Agent 修 Design Spec §9 把 namespace 改為與本批一致，明確說明「i18n key 與分頁位置解耦」<br>**Reviewer 偏好**：B（commit cost 低、命名更簡潔）、但需 Design 明示同意 |

### MJ2：FirmwareErrorView 的 `actionContactSupport` 按鈕 `title` 屬性硬編碼中文（i18n leak）

| 欄位 | 內容 |
|------|------|
| 檔案 | `frontend/src/components/firmware/firmware-error-view.tsx:143` |
| 規則 | i18n 完整性 / no hardcoded user-facing string |
| 違規程度 | Major（影響英文版 UX、a11y tooltip） |
| 對應文件 | Design Spec §7.2 + i18n 完整性原則 |
| 問題描述 | `<Button ... disabled title="請聯絡技術支援">` — `title` 屬性是 user-visible tooltip、但直接寫死中文字串，沒走 `t()`；英文用戶 hover 會看到中文。 |
| 影響 | en 用戶看到中文 tooltip；a11y screen reader 在某些設定下也會讀 title。 |
| 建議修改方式 | `title={t('firmware.error.actionContactSupportTooltip')}`，並在 i18n types.ts / zh-TW.ts / en.ts 補一條 `actionContactSupportTooltip` key（zh-TW: 「請聯絡技術支援」/ en: `Please contact technical support`）。或乾脆移除 `title`（按鈕本身已有 text label `actionContactSupport`、重複） |

### MJ3：Backend smoke test 用 `phase` field 而非 production schema 的 `stage`

| 欄位 | 內容 |
|------|------|
| 檔案 | `server/internal/api/ws/firmware_ws_test.go:62-67` |
| 規則 | Test 應反映 production schema（即使是 smoke test）|
| 違規程度 | Major（test 通過但不驗 schema、未來 schema 改 test 不會 fail） |
| 對應文件 | TDD §4.2 `FirmwareProgress.Stage` |
| 問題描述 | 測試 broadcast 用 `map[string]interface{}{"type": "firmware:progress", "deviceId": ..., "phase": "flashing", "percent": 42}` — 但 production handler `firmwareProgressMessage` embed `firmware.FirmwareProgress`、json field 是 `stage`（見 `server/internal/firmware/types.go:36`）、不是 `phase`。同時 production `type` 值是 `"firmware_progress"`（見 `firmware_handler.go:180`、底線而非冒號）、測試用 `"firmware:progress"`（冒號）也不對。 |
| 影響 | (1) Test 把錯誤 schema 通過，未來真有 backend handler bug 推 `phase` 而非 `stage` 不會被測抓到；(2) Frontend `useFirmwareProgress` 解析 `ev.stage` —— 如果哪天 backend 因某種重構真的改 schema，frontend 會 silently break、smoke test 仍 pass。 |
| 建議修改方式 | 改用真的 `firmware.FirmwareProgress` struct 或對齊 production 的 `firmwareProgressMessage` schema：<br>```go<br>hub.BroadcastToRoom(room, map[string]interface{}{<br>    "type":     "firmware_progress",  // 底線、對齊 firmware_handler.go:180<br>    "deviceId": deviceID,<br>    "stage":    "flashing",          // 不是 phase<br>    "percent":  42,<br>    "elapsedMs": int64(5000),<br>})<br>```<br>並把 assertion `got["phase"] != "flashing"` 改為 `got["stage"] != "flashing"`、`got["type"] != "firmware:progress"` 改為 `got["type"] != "firmware_progress"`。 |

---

## 🟡 Minor（建議修復）

| # | 檔案 | 行數 | 問題描述 | 建議修改方式 |
|---|------|------|----------|-------------|
| M1 | `firmware-upgrade-dialog.tsx` | 102-104 | `setTimeout(() => onOpenChange(false), 1500)` 與 Design §5.4 提到 toast 停留 6 秒不一致；AC-FW-1.3 提到 5 秒、註解寫 1.5s「給 toast 過渡」。實作偏向更積極關閉、可能讓使用者來不及讀 toast。 | 與 Design 確認 1.5s 是否合理（toast 還會繼續顯示 6 秒、modal 早關不影響）、若 OK 在註解明確寫「modal 1.5s 關閉、toast 由 `showSuccess` 自身控制停留」 |
| M2 | `firmware-badge.tsx` | 38-43 | Tailwind palette 直貼（`bg-emerald-600 / bg-amber-500 / dark:bg-emerald-500 / dark:bg-amber-400`）、Dark mode 對比實測責任在 M9-12（Design §11.2 備註明示）。本批 OK、但缺一個 TODO 條目供 M9-12 追蹤。 | 在 `firmware-badge.tsx:32-37` 註解區補一條 `// TODO(M9-12): 用 axe-core 實測對比、若 < 4.5:1 → 改 component token`、或寫進 progress.md「未解決問題」 |
| M3 | `firmware-store.test.ts` | 248-249 | 同一 `describe` 內測 `undefined → 60` 和 `'unknown_device' → 30`、語意不直覺（一個沒 type 走 60、有 type 但不認得走 30）。雖然測試正確、但 fallback 邏輯本身應該統一（要嘛全 fallback 60、要嘛全 fallback 30） | 確認 `firmware-store.ts:206-211` `estimatedDurationSeconds` 意圖：若 `!deviceType` 是「不知 chip 給保守值 60」、`unknown_device` 是「認不出 chip 給 KL520 預設」、邏輯可以接受但建議加註解；或乾脆統一 fallback 為 60 |
| M4 | `firmware-upgrade-dialog.tsx` | 90-98 | `showSuccess()` 停留時間未確認是否符合 Design §5.4「6 秒」規格 | 查 `lib/toast` 既有 `showSuccess` 預設 duration、若 < 6 秒考慮加 `showSuccess(..., { duration: 6000 })` 或在 toast util 層補 |
| M5 | testing | — | 缺 `FirmwareErrorView` 元件渲染測試（destructive reason 顯示 brick warning / Retry 按鈕隱藏 / disabled ContactSupport 按鈕） | 加 3-5 個 RTL 測試覆蓋 destructive vs recoverable reason 的 UI 差異（屬 Testing Agent 範圍、可在 M9-5 補） |
| M6 | `firmware-error-view.tsx` | 142-146 | `disabled` ContactSupport 按鈕無實際動作、只是視覺占位、與 destructive UX 衝突（按鈕本意是「請聯絡技術支援」、disabled 反而傳達「不能聯絡」） | 兩個方案：(a) 改 enabled、`onClick={() => window.open('mailto:support@...')}` 或開既有 support 文件；(b) 改 plain `<p>` 不用 Button；建議 (a)、由 Design Agent 提供具體 support 入口 |
| M7 | `firmware-upgrade-button.tsx` | 26 | `if (!device.firmwareCanUpgrade) return null` 在元件內 early return，但 `device-card.tsx:63` 也已用 `device.firmwareCanUpgrade &&` gate；雙重檢查、無害但冗餘 | 移除 `device-card.tsx:63` 的 `device.firmwareCanUpgrade &&`、讓 button 元件單一決定渲染（更符合 single responsibility）；或反之 |
| M8 | `firmware-store.ts` | 252-262 | `fetchActiveFirmwareTasks` 是 module-level export（不在 store action 內）、命名 `useFirmwareStore` 一族但這個是 helper； | 移到 `lib/api/firmware.ts` 或 `lib/firmware-active-tasks.ts` 等專用檔；或在註解明確寫「此函數刻意不放 store 因為它是 Wails 端 callback 用、無 reactive state」 |

---

## 💡 Suggestion（非必要）

| # | 檔案 | 行數 | 建議內容 |
|---|------|------|----------|
| S1 | `firmware-upgrade-dialog.tsx` | DialogContent | Design §5.1 規格寫 480px，shadcn Dialog 預設 `max-w-lg`（512px）、未顯式設寬度；視覺檢查時可加 `className="max-w-[480px]"` 對齊 |
| S2 | TDD §5.3 stage % 對照 | — | 本批信任 backend `progress.percent` 不 hardcode 是對的、但 reviewer 沒驗 backend 是否真按 §5.3 表發 percent（5/20/50/90/100）。建議 M9-5 testing 階段加 stage-percent 對照測試 |
| S3 | `firmware-upgrade-dialog.tsx` | 45-55 | 用 `useFirmwareStore()` 整 hook 拿、不 selector — progress event 每秒推時整個 dialog 重 render（含 confirming UI tree）；非問題、屬可接受 trade-off；若優化可拆 selector |
| S4 | testing | — | 缺 `FirmwareUpgradeDialog` integration 測試（phase 切換時 modal 視覺與 R-FW-11 擋 ESC 行為）。屬 Testing Agent 範圍、可在 M9-5 / M9-12 補 |
| S5 | `firmware-error-view.tsx` | 109-117 | 技術資訊 template literal 較複雜（多層 `${cond ? a : ''}`）、可讀但不易調整。考慮抽 helper `formatTechnicalInfo(progress: FirmwareProgressEvent): string`、減少 JSX 內邏輯 |

---

## Design Spec 對齊評估（重點章節）

| Design Spec 章節 | 對齊狀態 | 評估 |
|----------------|---------|------|
| §3.4 stage / reason / i18n 對應 | ✅ 完全對齊 | stage 4 種 + reason 8 種 + i18n fallback 完整 |
| §6 升級 modal 4 phase（confirming / upgrading / success / error） | ✅ 完全對齊 | 雖然降版（§6 主題）B 階段才做、但 phase machine 結構已預埋 |
| §7 8 種 reason → 4 種 UX | ✅ 完全對齊 | `errorMessageKeyFor` + `canRetryReason` + `primaryActionKeyFor` 三個 helper 涵蓋全部 |
| §12 R-FW-11 緩解（upgrading 不可關 modal） | ✅ 完全對齊 | 三層擋（onOpenChange / interactOutside / EscapeKeyDown）+ `showCloseButton={false}` |
| §9 i18n 完整性 | ⚠️ 部分（MJ1） | namespace 偏離、key 集合本身 A 階段所需都齊 |
| §11 Design Tokens（暫代評估） | ✅ 合理 | Tailwind palette 暫代、M9-12 由 Frontend 用 axe-core 落地 6 個 component token、Design §11.2 備註已明示此責任分工 |

---

## TDD §4.2 WS schema 對齊

實質判斷：完全對齊。

| TDD §4.2 欄位 | Frontend 型別 | Backend struct json | 對齊狀態 |
|--------------|--------------|-------------------|---------|
| `percent` | `number` | `int` | ✅ |
| `stage` | `FirmwareStage` enum | `string` (`preparing/loading/flashing/verifying/done/error`) | ✅ |
| `direction` | `'upgrade'\|'downgrade'?` | `string,omitempty` | ✅ |
| `message` | `string?` | `string,omitempty` | ✅ |
| `elapsedMs` | `number` | `int64` | ✅ |
| `etaMs` | `number?` | `int64,omitempty` | ✅ |
| `error` | `string?` | `string,omitempty` | ✅ |
| `reason` | `FirmwareReason` enum | `string,omitempty` | ✅ |
| `rawError` | `string?` | `string,omitempty` | ✅ |
| `beforeVersion` | `string?` | `string,omitempty` | ✅ |
| `errorCode` | `string?` | `string,omitempty` | ✅ |
| `afterVersion` | `string?` | `string,omitempty` | ✅ |
| `method` | `string?` | `string,omitempty` | ✅ |
| `deviceId` | `string`（在 type 上）| `string` | ✅（backend 在 struct 必填、frontend `FirmwareProgressEvent.deviceId` 也必填）|
| `type` (wrapper) | `'firmware_progress'` literal | wrapper struct `Type: "firmware_progress"` | ✅ 對齊（除非看 firmware_ws_test.go：見 MJ3）|

---

## 32 新測試 + 2 backend smoke test 評估

### Frontend tests（32 個）

| Test 群組 | 數量 | 品質評估 |
|---------|------|---------|
| `useFirmwareStore — phase transitions` | 8 | 高 — phase machine 完整覆蓋、含 R-FW-11 緩解 + 多裝置隔離 |
| `errorMessageKeyFor — reason → i18n key` | 2 | 高 — 8 種 reason + stage fallback 全覆蓋 |
| `canRetryReason — destructive failures` | 3 | 高 — 3 destructive + 5 recoverable + undefined 全覆蓋 |
| `primaryActionKeyFor — recommended action` | 3 | 高 |
| `estimatedDurationSeconds — chip estimates` | 3 | 中 — 涵蓋 KL520/KL720/unknown，但 fallback 語意不一致（見 M3）|
| `stageOrdinal — 4-stage vs 3-stage` | 2 | 高 |
| `computeBadgeState — AC-FW-1.1 4-color` | 6 | 高 — 含 legacy precedence 邊界 case |
| `FirmwareBadge — render + a11y` | 4 | 中 — render & data-state 涵蓋，缺 hover tooltip 互動測試 |

**漏測項目**：
- `useFirmwareProgress` hook（WS connectAndWait timeout / cleanup）— 既有 `useFlashProgress` 也沒測、屬既有 pattern、可接受
- `FirmwareErrorView` 元件渲染測試（destructive UI 切換）— 見 M5
- `FirmwareUpgradeDialog` integration（phase 切換 + R-FW-11 擋 ESC）— 見 S4

### Backend smoke tests（2 個）

| Test | 品質評估 |
|------|---------|
| `TestFirmwareProgressHandler_ReceivesBroadcast` | 中 — plumbing 正確、但 schema 用錯欄位（`phase` vs `stage`、見 MJ3）|
| `TestFirmwareProgressHandler_RoomIsolation` | 高 — room key 隔離驗證合理、connB 不收 + timeout assertion 正確 |

---

## 安全軸特別評估（粗篩、不升級）

| 項目 | 狀態 |
|------|------|
| WS origin check（loopback only）| ✅ backend 共用 `CheckOrigin`（origin.go:18-40），白名單 127.0.0.1 / localhost / ::1、http only |
| raw error 不渲染主畫面 | ✅ 只在 collapsible `<details>` 內（firmware-error-view.tsx:101-118）、預設收合 |
| XSS / dangerouslySetInnerHTML / v-html | ✅ grep 零 hit、React 自動 escape 所有 template literal interpolation |
| Hardcoded secrets / tokens | ✅ 零 hit |
| Auth / session 變更 | N/A — 本批無 auth 改動 |
| 新增第三方 SDK | N/A — 無新依賴 |
| PII / 金融 / 健康資料處理 | N/A |
| 對外 webhook / 新 OAuth | N/A |

**結論**：5 軸 security 粗篩無發現、**不升級給 Security Auditor**。

---

## Concurrency 評估

### Frontend
- ✅ 多裝置隔離：`firmware-store.ts:110-115` 用 `activeDeviceId` mismatch return（對齊 flash-store M4 fix）
- ✅ WS race：`connectAndWait` 必須先 resolve 才送 POST（避免 server 在 client 連 WS 前就送 first event）
- ✅ Retry 流程：`firmware-upgrade-dialog.tsx:132-136` 重置 progress → 重連 WS → 重送 POST、無 race
- ⚠️ `firmware-store.ts:82-108` `startUpgrade` 是 async、其他 phase transition 可能在它執行期間發生（如 cancelConfirm）；但 cancelConfirm 已守 `if (phase !== 'confirming') return`、整體安全

### Backend hot-fix
- ✅ 純對稱 flash_ws.go、低風險
- ✅ `hub.RegisterSync` 確保 join room 完成才進讀寫 loop（避免 race）
- ✅ Read pump goroutine drain incoming（純為偵測斷線）、與主 goroutine 不衝突

---

## Concurrency / 跨檔案一致性檢查（R-B1）

| 比對項目 | 一致性 | 備註 |
|---------|-------|------|
| Frontend `FirmwareProgressEvent` ↔ Backend `firmwareProgressMessage` + `firmware.FirmwareProgress` | ✅ 13 欄位全對齊 | 含 `type: "firmware_progress"` wrapper |
| Frontend `/ws/devices/:id/firmware-progress` ↔ Backend router.go:125 ↔ firmware_ws.go:30 room `firmware:<deviceID>` | ✅ 三端一致 | |
| Frontend `firmwareCanUpgrade / firmwareIsLegacy / bundledFirmwareVersion` ↔ Backend DeviceInfo 衍生欄位 | ✅ 完全 | 與 TDD §3.1 一致 |
| Frontend `errorMessageKeyFor(stage, reason)` ↔ Design §7.1 8 種 reason | ✅ 8/8 對齊 | 含 stage-only fallback |
| Frontend phase machine ↔ Design §8 狀態機 | ✅ 完全 | idle/confirming/upgrading/success/error |
| i18n key namespace ↔ Design Spec §9 | ⚠️ 偏離 | 見 MJ1 |
| Backend smoke test schema ↔ production `firmwareProgressMessage` | ⚠️ 偏離 | 見 MJ3 |

---

## B 層 verification（R-B2 / R-B3 / R-B4）

| # | 條件 | 狀態 |
|---|------|------|
| R-B1 | 跨檔案一致性檢查 | ✅ 已做（見上表 7 項比對）|
| R-B2 | 漏審檢查 | ✅ 18/18 檔全部覆蓋、無漏審 |
| R-B3 | 大改動 commit 拆分 | ✅ 無單檔 > 500 行（最大是 firmware-store.ts 262 行 + 測試 265 行、合理）|
| R-B4 | 三方文件互審狀態 | N/A — 本次為程式碼審查、非文件審 |

**B 層跑了、無暫緩。**

---

## 優點（R-A4）

1. **Pure helper 分離**：`firmware-store.ts:147-243` 5 個 pure functions（無 React 依賴）從 store 拆出、便於測試 + M9-12 B 階段重用。設計遠見好。
2. **多裝置隔離正確**：`firmware-store.ts:110-115` 對齊既有 flash-store M4 fix、不重蹈覆轍、跨 dongle 同時升級不互污。
3. **R-FW-11 緩解三層擋**：`firmware-upgrade-dialog.tsx:146-160` 用 `onOpenChange` + `onInteractOutside` + `onEscapeKeyDown` + `showCloseButton={false}` 四層、覆蓋所有 modal 關閉路徑。
4. **destructive reason 不提供 Retry**：`firmware-error-view.tsx:91-98, 142-146` 對 3 種 destructive reason（disconnect / verify_mismatch / verify_not_found）顯示 brick warning + 不可重試、安全防護到位。
5. **Backend hot-fix 對稱**：`firmware_ws.go` 與 `flash_ws.go` 結構 1:1 對稱、減少維護心智成本。
6. **A11y 用心**：`role="status" aria-live="polite"`（進度區、不打斷 SR）+ `role="alertdialog"`（error modal、主動朗讀）+ `role="note"` 區別語意級別、合理。
7. **註解追溯性強**：每個元件 / 函數 JSDoc 都引用 Design / TDD 章節條目（如「對齊 Design §7.1」「TDD §3.4」），方便未來追溯設計意圖。
8. **type 嚴格**：TypeScript 型別嚴謹（`FirmwareStage` / `FirmwareReason` 都是 literal union enum）、無 `any`。
9. **預埋 B 階段擴展點**：`firmware-store.ts:1-263` 註解明示「Settings 韌體面板（M9-12 B 階段）日後將與本目錄共用元件」、namespace 設計 forward-compatible。

---

## Needs investigation（R-A5）

| 項目 | 為什麼不確定 | 建議後續行動 |
|------|------------|------------|
| 1 | Design Spec §9 namespace `settings.firmware.*` vs Frontend 實作 `firmware.*` 哪個是 source of truth | Orchestrator 派 Design Agent 與 Frontend 共同確認、見 MJ1 |
| 2 | `showSuccess()` toast 預設停留時間是否符合 Design §5.4 「6 秒」 | Frontend agent 確認 `lib/toast.showSuccess` 預設行為、見 M4 |
| 3 | backend `progress.percent` 是否真按 TDD §5.3 表發（5 / 20 / 50 / 90 / 100） | 由 Testing Agent 在 M9-5 三平台實機驗證階段對照、見 S2 |
| 4 | `device-card.tsx` modified lines 確切 diff 數（任務說明寫 +X）| 不影響審查結論、log 已涵蓋 |

---

## 是否阻擋 M9-5（三平台實機驗證）

**不阻擋**。3 個 Major 都是：
- MJ1：i18n namespace 對齊（純命名、邏輯正確）
- MJ2：硬編碼中文 tooltip（一行修）
- MJ3：backend smoke test schema 用錯欄位（測試正確性、不影響 production）

→ Frontend / Backend 第 2 輪可在 M9-5 並行執行期間修完、不阻擋 M9-5 三平台 KL520+KL720 完整 E2E。

## 是否升 Security Auditor

**不升**。5 軸 security 粗篩無發現、無 auth/secrets/對外 API/第三方整合變更、WS origin 由 backend 既有 CheckOrigin 共用、raw error 不在主畫面渲染、無 XSS / hardcoded secret。

## 是否需 Frontend / Backend 第 2 輪

| Agent | 是否需第 2 輪 | 修哪幾項 |
|-------|------------|---------|
| Frontend | 是 | MJ1（i18n namespace、需與 Design 對齊）+ MJ2（hardcoded title）+ Minor M1 / M2 / M3 / M4 / M6 / M7 / M8（視 Frontend agent 評估）|
| Backend | 否（建議在 M9-5 順手修）| MJ3（smoke test 用 `stage` 而非 `phase`、用 `firmware_progress` 而非 `firmware:progress`）— 屬 testing 範疇、可由 Testing agent 在 M9-5 階段補 |

---

## 結論

**審查結果：⚠️ 需修改後通過（第 1 輪）**

實作品質高、文件對齊度高、5 軸 + 測試軸都實質審過。3 個 Major 都是 i18n / test schema 層級的對齊問題、不影響核心邏輯與 production 行為。建議 Frontend 第 2 輪聚焦 MJ1 + MJ2 + 部分 Minor、Backend 在 M9-5 順手修 MJ3。M9-5 三平台 E2E 可平行開跑、不被本輪 review 阻擋。

**主要產出位置**：`.autoflow/05-implementation/review/m9-4-frontend-firmware-review.md`

---

## Verification 自評（reviewer §12）

### A 層
- [x] R-A1：5 軸 + 測試軸全跑過（每軸 ≥ 20 字實質判斷、無「OK」一字結案）
- [x] R-A2：文件符合性 checklist 完整（PRD 14/14 / TDD 9/9 / Design 13/16 三表全填）
- [x] R-A3：3 個 Major 都附 `file:line` + 規則名稱 + 具體建議修改
- [x] R-A4：優點段落 9 條（≥ 1 條）
- [x] R-A5：Needs investigation 4 項明寫
- [x] R-A6：通用 6 條退出條件表全標狀態（5 條未觸發 / 1 條觸發 MJ1 / 1 條 N/A）

### B 層
- [x] R-B1：跨檔案一致性表 7 項
- [x] R-B2：18/18 檔覆蓋、無漏審
- [x] R-B3：無 > 500 行單檔
- [x] R-B4：N/A（純程式碼審查、非文件）+ 原因明寫

### 總結
- B 層**已跑**、無暫緩、無需 Orchestrator 計數機制介入
- C 層：本批屬 milestone 內 review、非 PR 合 master 前最終 review、C 層不適用