A 階段第四個 milestone、完整 Frontend FW UI(badge / modal / 8 種 reason 復原)+ backend WS hot-fix(補對稱於 flash 的 firmware WS endpoint)。 Frontend(13 修改 / 7 新檔): - 新 firmware/ component group (badge / upgrade-button / upgrade-dialog 4-phase / progress-view / error-view 8-reason / index) - Zustand store (firmware-store.ts) + WS hook (use-firmware-progress.ts) 對齊既有 useFlashProgress pattern - DeviceCard 整合 FirmwareBadge + FirmwareUpgradeButton - i18n: settings.firmware.* namespace (對齊 Design Spec §9 SoT) + devices.card.fwBadge.* (zh-TW + en, 57 leaf keys × 2 lang = 114 strings) - toast.ts ToastOptions interface (duration param) - types/device.ts: FW 衍生欄位 + FirmwareStage/Reason/ProgressEvent/ActiveTask types Backend WS hot-fix (3 檔): - ws/firmware_ws.go (50 行、純對稱 flash_ws.go) - ws/firmware_ws_test.go (165 行、2 smoke tests: broadcast + room isolation) - router.go: GET /ws/devices/:id/firmware-progress 關鍵設計: - R-FW-11 緩解: upgrading phase modal 不可關 (onInteractOutside/onEscapeKeyDown preventDefault + 隱藏 X) - 多裝置隔離 defense in depth: store handleEvent activeDeviceId mismatch 直接 return - 8 種 reason → 4 種 UX (recoverable/destructive/brick 警告/contactSupport) - ContactSupport mailto handler (RFC 6068 + encodeURIComponent) Reviewer 兩輪審查: - Round 1: 0 Critical / 3 Major / 8 Minor / 5 Suggestion - Round 2: 0 Critical / 0 Major / 0 Minor / 2 Suggestion(接受方案 A、不需 frontend 第 3 輪) - MJ1 i18n namespace 採方案 A (settings.firmware.*)、Design SoT 優先、Reviewer 同意 測試: - pnpm test --run: 60 tests pass (32 firmware: 22 store + 10 badge + 新 9 error-view + 19 既有) - npx tsc --noEmit: 0 error - pnpm build: production build 成功 - go test ./internal/api/ws/... -race: 1.964s 全綠 - pnpm lint firmware/: 0 hit (17 既有 lint 問題不屬 M9-4、follow-up) 未做(範圍外): - Settings 韌體面板 (M9-12 B 階段) - 手動降版 UI (M9-12) - 版本切換 dropdown (B 階段) - Wails 控制台 force-quit modal (M9-4.5) A 階段 MVP 後端 + 前端開發全部完成、剩 M9-4.5 (SIGTERM + Wails OnBeforeClose) + M9-5 (三平台實機驗證) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
38 KiB
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-130connectAndWait在 POST 之前、避免 WS 還沒 open 漏掉 backend 早期 event(對齊 flash-store pattern) - ⚠️
firmware-store.ts:248-249test caseexpect(estimatedDurationSeconds('unknown_device')).toBe(30)與函數實作不一致 — 函數 line 207-211if (!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-243pure 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-129reuseflash_start / flash_complete / flash_erroractivity 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(已 grepdangerouslySetInnerHTML / 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?.writeTextoptional 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在upgradingphase 是 no-op - ✅ destructive reason 不可 retry 測試(
firmware-store.test.ts:201-219)— 3 種 destructive + 5 種 recoverable + undefined 都涵蓋 - ⚠️ 缺
useFirmwareProgresshook 的測試(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 - ⚠️ 缺
FirmwareUpgradeDialogintegration 測試(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 共同裁決): A. 把 i18n keys 搬到 settings.firmware.* namespace 下(雖然 A 階段不在 settings 頁、但對齊 Design 文件):例如 firmware.error.messageScanNotFound → settings.firmware.error.message.scanNotFoundB. 維持 firmware.* flat 結構,但回頭請 Design Agent 修 Design Spec §9 把 namespace 改為與本批一致,明確說明「i18n key 與分頁位置解耦」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: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>並把 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 互動測試 |
漏測項目:
useFirmwareProgresshook(WS connectAndWait timeout / cleanup)— 既有useFlashProgress也沒測、屬既有 pattern、可接受FirmwareErrorView元件渲染測試(destructive UI 切換)— 見 M5FirmwareUpgradeDialogintegration(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用activeDeviceIdmismatch 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-108startUpgrade是 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)
- Pure helper 分離:
firmware-store.ts:147-2435 個 pure functions(無 React 依賴)從 store 拆出、便於測試 + M9-12 B 階段重用。設計遠見好。 - 多裝置隔離正確:
firmware-store.ts:110-115對齊既有 flash-store M4 fix、不重蹈覆轍、跨 dongle 同時升級不互污。 - R-FW-11 緩解三層擋:
firmware-upgrade-dialog.tsx:146-160用onOpenChange+onInteractOutside+onEscapeKeyDown+showCloseButton={false}四層、覆蓋所有 modal 關閉路徑。 - destructive reason 不提供 Retry:
firmware-error-view.tsx:91-98, 142-146對 3 種 destructive reason(disconnect / verify_mismatch / verify_not_found)顯示 brick warning + 不可重試、安全防護到位。 - Backend hot-fix 對稱:
firmware_ws.go與flash_ws.go結構 1:1 對稱、減少維護心智成本。 - A11y 用心:
role="status" aria-live="polite"(進度區、不打斷 SR)+role="alertdialog"(error modal、主動朗讀)+role="note"區別語意級別、合理。 - 註解追溯性強:每個元件 / 函數 JSDoc 都引用 Design / TDD 章節條目(如「對齊 Design §7.1」「TDD §3.4」),方便未來追溯設計意圖。
- type 嚴格:TypeScript 型別嚴謹(
FirmwareStage/FirmwareReason都是 literal union enum)、無any。 - 預埋 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 層
- R-A1:5 軸 + 測試軸全跑過(每軸 ≥ 20 字實質判斷、無「OK」一字結案)
- R-A2:文件符合性 checklist 完整(PRD 14/14 / TDD 9/9 / Design 13/16 三表全填)
- R-A3:3 個 Major 都附
file:line+ 規則名稱 + 具體建議修改 - R-A4:優點段落 9 條(≥ 1 條)
- R-A5:Needs investigation 4 項明寫
- R-A6:通用 6 條退出條件表全標狀態(5 條未觸發 / 1 條觸發 MJ1 / 1 條 N/A)
B 層
- R-B1:跨檔案一致性表 7 項
- R-B2:18/18 檔覆蓋、無漏審
- R-B3:無 > 500 行單檔
- R-B4:N/A(純程式碼審查、非文件)+ 原因明寫
總結
- B 層已跑、無暫緩、無需 Orchestrator 計數機制介入
- C 層:本批屬 milestone 內 review、非 PR 合 master 前最終 review、C 層不適用