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>
18 KiB
M9-4 Reviewer Round 2 — Frontend FW UI(第 2 輪修改驗證)
審查日期:2026-05-25 Reviewer:Reviewer Agent 審查對象:Frontend 第 2 輪修改(i18n namespace 重組 + 3 Major + 8 Minor 修法驗證) 審查層:A 層(per-task delta) 第 1 輪報告:
.autoflow/05-implementation/review/m9-4-frontend-firmware-review.md
TL;DR
- 第 2 輪審查結果:✅ 通過
- MJ1 方案選擇:接受方案 A(namespace =
settings.firmware.*) — 採納 Frontend 的 pushback、Design Spec §9 v2.2 為 SoT、未做正式 Design Spec 改寫前 Reviewer 不該翻案 - 第 1 輪 issue 修了 / 沒到位:12/12 應修項全部到位、3 項刻意留 follow-up(Frontend 給的理由合理)、1 項 backend 不適用本輪(MJ3 屬 Backend / Testing 範圍)
- 第 2 輪新發現:0 Critical / 0 Major / 0 Minor / 2 Suggestion(都屬 nice-to-have、不阻擋)
- 是否阻擋 M9-5:否
- 是否需 Frontend 第 3 輪:否
- 是否升 Security Auditor:否(ContactSupport mailto handler 已驗證安全、無 XSS / open redirect)
方法說明
本輪採「delta-only」審查:不重審第 1 輪已通過細節(避免疲勞與 false positive),聚焦:
- 第 1 輪每條 issue 是否真的修了 / 修對方向
- 修改本身是否引入新問題(regression)
- 9 個新 FirmwareErrorView tests 品質
完整 5 軸 / 文件符合性 / B 層跨檔案比對等已在第 1 輪報告做過、本輪不重做。
MJ1 方案選擇(Frontend pushback 評估)
Frontend 主張方案 A 的理由
- Design Spec v2.2 §9 是三方對齊後的正式 SoT、未做正式 Design Spec 改寫前 Frontend 不該自行翻案
- B 階段 M9-12 會加入
settings.firmware下 §9.1 page 結構 / §9.2 device card label / §9.3 switch accordion / §9.5 downgrade modal 等更多 keys;現在 flat 結構 (firmware.*) 之後 M9-12 切換成settings.firmware.*會造成兩批 keys 分散維護
Reviewer 評估
| 評估面向 | 結論 |
|---|---|
| Design Spec §9 v2.2 是否正式 SoT | 是。三方 v2.2 已對齊、本輪 review 不應翻案 |
| 方案 A 是否會與既有 i18n 結構衝突 | 不會。settings 是既有 namespace、settings.firmware 是合理子結構(types.ts:305 加註解明示「§9.1 page / §9.2 card label / §9.3 accordion / §9.5 downgrade modal 留待 M9-12 補」、forward-compatible) |
| 方案 A vs B 的長期維護成本 | A 更低。B 階段一定要落 settings.firmware.*、現在做 A 等於一次到位、避免 M9-12 再 rename 一次(耗 PR + 影響 i18n test snapshot) |
| Reviewer 第 1 輪偏好 B 的理由(commit cost 低、命名更簡潔) | 不成立。Frontend 已用 trivial rename + 更新 22 既有 test、cost 實測也很低;「命名簡潔」對單一檔案有意義、但跨 PRD/Design/i18n/code 對照查找時 namespace 對齊更值錢 |
Reviewer 最終決定:接受方案 A。Frontend 的 pushback 邏輯成立、Design SoT 應優先於 Reviewer 個人偏好。本輪 MJ1 視為通過。
第 1 輪 issue 修改驗證(逐項)
Major(3 項)
| Issue | 第 1 輪要求 | 第 2 輪修法 | 驗證結果 |
|---|---|---|---|
| MJ1 i18n namespace 與 §9 不一致 | 二擇一:A 搬到 settings.firmware.* / B 改 Design Spec |
採方案 A — types.ts:310-375、zh-TW.ts:305-370、en.ts:305-370 三檔同步搬入 settings.firmware.*;firmware-store.ts:151-199 8 條 errorMessageKeyFor 路徑 + 4 條 primaryActionKeyFor 路徑全改;5 個元件 (firmware-badge / firmware-upgrade-button / firmware-upgrade-dialog / firmware-progress-view / firmware-error-view) 所有 t() 呼叫全用新路徑;22 既有 + 9 新 firmware-store.test.ts 同步 |
✅ 完全到位。grep t\('firmware\. 與 'firmware\.(不含 settings. 前綴)皆 0 hit、無死引用 |
| MJ2 FirmwareErrorView title 硬編碼中文 | 改 t(...) + 補 i18n key(或乾脆移除 title) |
firmware-error-view.tsx:158 改 title={t('settings.firmware.error.contactSupportTooltip')};types.ts:372 / zh-TW.ts:367 / en.ts:367 三檔都補 contactSupportTooltip key(中:「請聯絡技術支援」/ 英:Please contact technical support);同時把 disabled 改 enabled、補 handleContactSupport 用 mailto: 開啟(line 81-88)— 算修 MJ2 + M6 一併 |
✅ 完全到位。額外修了 M6(disabled ContactSupport 與 UX 衝突)、合理副產品 |
MJ3 Backend smoke test schema 用 phase 而非 stage |
Backend / Testing 在 M9-5 階段補 | 本輪 Frontend 範圍外、未動 backend test | N/A — 不屬本輪 frontend 第 2 輪範圍、按計畫由 testing agent 於 M9-5 補 |
Minor(8 項)
| # | 第 1 輪要求 | 第 2 輪修法 | 驗證結果 |
|---|---|---|---|
| M1 | 確認 1.5s modal 關閉是否與 §5.4 toast 6s 相容、加註解 | firmware-upgrade-dialog.tsx:103-104 註解明寫「modal 1.5s 後關閉、toast 由 showSuccess 自身 6s duration 控制」+ AC-FW-1.3 5 秒提示 |
✅ 到位 |
| M2 | 補 TODO 條目給 M9-12 追蹤對比 | firmware-badge.tsx:39-41 新增 TODO(M9-12) 註解明示用 axe-core 實測對比、< 4.5:1 改 component token、特別注意 amber-400 + emerald-500 暗模 |
✅ 到位 |
| M3 | 確認 estimatedDurationSeconds fallback 語意統一 / 加註解 | firmware-store.ts:202-217 整段 JSDoc 改寫「兩種 fallback 數值不同是刻意的:『不知道有沒有 type』和『有 type 但認不出』資訊量不同」;test (firmware-store.test.ts:247-253) 註解同步說明 |
✅ 到位(決定保留差異 + 文件化、合理) |
| M4 | showSuccess 預設停留是否符合 §5.4 6 秒、不足要加 duration | lib/toast.ts 加 ToastOptions { duration?: number }、4 個 toast helper 都支援;firmware-upgrade-dialog.tsx:99 用 { duration: 6000 } 顯式傳;註解引用 Reviewer M1+M4 |
✅ 到位 |
| M5 | 補 FirmwareErrorView 元件渲染測試 | 新檔 tests/components/firmware-error-view.test.tsx 共 9 tests:destructive 3 種 brick warning + ContactSupport(it.each)/ contact mailto 點擊 / recoverable 2 種 / errorCode 顯示 / collapsible 預設收合 / Close callback |
✅ 到位(品質評估見下節) |
| M6 | disabled ContactSupport 改 enabled + 實際動作 | firmware-error-view.tsx:81-88 加 handleContactSupport:mailto: + subject 帶 errorCode + body 帶 technicalInfo + target="_blank";line 153-162 改用 destructive variant + onClick |
✅ 到位 |
| M7 | 移除 device-card.tsx 與 button 元件的雙重 firmwareCanUpgrade gate | device-card.tsx:61-64 註解明示「由 FirmwareUpgradeButton 內部決定」、實際 JSX 直接 <FirmwareUpgradeButton device={device} disabled={isBusy} /> 無 device.firmwareCanUpgrade && 外層 gate;單一責任落地 |
✅ 到位 |
| M8 | 把 fetchActiveFirmwareTasks 移到專用檔 / 加註解說明刻意設計 | firmware-store.ts:253-262 補完整 JSDoc「刻意設計為 module-level helper、不放 store action」+ 三個理由(無 reactive state / Wails callback / 不需 subscribe);位置保留在 store 檔(Frontend 選擇文件化而非移檔) |
✅ 到位(文件化方案合理、避免 churn) |
Suggestion(5 項)
| # | 第 1 輪建議 | 第 2 輪處置 | 驗證結果 |
|---|---|---|---|
| S1 | 對齊 §5.1 寬度 480px | firmware-upgrade-dialog.tsx:157-158 加 className="sm:max-w-[480px]" + 註解引用 Reviewer S1 |
✅ 修了 |
| S2 | M9-5 testing 階段加 stage-percent 對照測試 | 留 follow-up 給 Testing Agent M9-5 階段 | ⏸ 合理保留 — 屬 testing agent / E2E 範圍 |
| S3 | Dialog selector 優化 re-render | 留 follow-up — 屬 perf nice-to-have、非問題 | ⏸ 合理保留 |
| S4 | FirmwareUpgradeDialog integration test | 留 follow-up — 屬 Testing Agent M9-5 / M9-12 範圍 | ⏸ 合理保留 |
| S5 | 抽 formatTechnicalInfo helper | firmware-error-view.tsx:23-40 抽出 formatTechnicalInfo(progress: FirmwareProgressEvent): string、JSDoc 引用 Reviewer S5;handleCopy + <pre> + handleContactSupport 三處共用同一輸出 |
✅ 修了(且額外用在 mailto body、複用更廣) |
統計:應修 / 已修 = 12 / 12(含 MJ1 / MJ2 / 8 Minor + S1 / S5),刻意保留 3 項(S2 / S3 / S4),不適用本輪 1 項(MJ3 屬 Backend)。
9 個新 FirmwareErrorView tests 品質評估
檔案:frontend/src/tests/components/firmware-error-view.test.tsx(165 行)
| Test 群組 | 覆蓋 | 品質 |
|---|---|---|
destructive reasons(3 種)it.each brick warning + 不顯示 Retry + 顯示 ContactSupport |
disconnect_during_op / verify_mismatch / verify_not_found |
高 — 用 it.each 涵蓋三種、getByRole('note') 驗 brick warning、getAllByRole('button') + label 對比驗 Retry 不存在 + ContactSupport 存在 |
| ContactSupport mailto 點擊 | vi.spyOn(window, 'open') mock + fireEvent.click + 檢查 href schema ^mailto: + decodeURIComponent body 含 stage / reason |
高 — 安全驗證到位(href 是 mailto: scheme 不是 http:)、技術資訊真的進 body |
| recoverable reasons(2 種)顯示 Retry | connect_failed Retry 按鈕 + onRetry callback;scan_not_found 顯示 ReplugRetry |
高 — queryByRole('note') 確認沒 brick warning(negative case)、fireEvent.click 驗 callback |
| errorCode 顯示 | getAllByText(/FW_E102/).length >= 1(同時在 <p> 與 <pre> 出現、刻意接受兩處) |
中-高 — 接受重複出現是對的、若用 getByText 會誤判 |
| collapsible 預設收合 | container.querySelector('details').open === false |
高 — 直接驗 DOM state |
| Close 觸發 onClose | fireEvent.click(closeBtn) → onClose 1 次 |
高 |
整體評估:
- ✅ 用
it.each涵蓋三 destructive reason、簡潔且維護友善 - ✅ 用 i18n label regex
/Contact|聯絡技術支援|Support/同時兼容中英環境、CI 切 locale 不會 fail - ✅ jsdom
window.openmock 寫法正確(spyOn + mockImplementation(() => null)) - ✅ 驗 mailto href schema 是安全把關(防止意外被改成 http: 開外部頁面)
- ⚠️ 缺 handleCopy 測試(clipboard write 行為)— 第 1 輪 review 也沒列、不算缺陷、屬 nice-to-have
- ⚠️ 缺 techOpen toggle 測試(onToggle 後 open=true)— 同上、屬 nice-to-have
結論:9 個 tests 品質高、達到第 1 輪 M5 要求;漏的兩項屬 nice-to-have、不阻擋本輪通過。
第 2 輪新發現
Critical
無。
Major
無。
Minor
無。
Suggestion
| # | 檔案 / 位置 | 建議 |
|---|---|---|
| R2-S1 | firmware-error-view.tsx:81-88 handleContactSupport |
mailto: 地址 support@kneron.com 硬編碼在元件內。雖然 mailto 不會被 attacker 替換(純 link)、但若日後 support email 改 / 多語系不同 region 不同信箱、會要改元件本身。可考慮把 MAILTO_SUPPORT 抽到 lib/config.ts 或讀 i18n(不過 mailto 地址通常不 i18n)。屬 nice-to-have、不阻擋。 |
| R2-S2 | tests/components/firmware-error-view.test.tsx |
缺 handleCopy 測試(clipboard 行為 + copied state 切換 2s 後 reset)+ techOpen toggle 測試。可在後續任務(M9-12 或 testing 補充階段)補。屬 nice-to-have、不阻擋本輪。 |
Regression 檢查(修改本身是否引入新問題)
| 風險點 | 檢查結果 |
|---|---|
| i18n namespace 重組漏 key | grep t\('firmware\.(不含 settings 前綴)= 0 hit;grep 'firmware\. 同 0 hit。所有舊路徑 100% 切完。中英對齊:types.ts:310-375 vs zh-TW.ts:305-370 vs en.ts:305-370 同 namespace 結構、所有 key 三邊都有對應(無 missed key) |
| ContactSupport mailto 安全性 | mailto: scheme 是 RFC 6068 standard、不會觸發 XSS(不執行 JS);target="_blank" 不引入 open redirect 風險(mailto: 由 OS 端 mail handler 處理、不會是 http:);subject + body 用 encodeURIComponent 包裝、即使 progress.errorCode 內含特殊字元也安全 |
| formatTechnicalInfo helper rawError leak | helper 抽出後同時用在 (a) handleCopy clipboard / (b) <pre> 區 / (c) mailto body — 三處都是「使用者主動觸發 + 預設收合的 details / 主動點 copy / 主動點 contact」、不是被動曝露在主畫面;rawError 行只在有值時才加(line 37 if (progress.rawError));無 leak 到非預期位置 |
| toast duration 6s 修改 | lib/toast.ts 改成支援 ToastOptions、向下相容(既有 caller 不傳 options 用 sonner 預設 4000ms);只有 firmware-upgrade-dialog.tsx:99 顯式傳 6000;不影響既有 toast 行為 |
| 22 既有 firmware-store test 是否同步更新 | 看 firmware-store.test.ts:168-234 — 8 條 errorMessageKeyFor 直接 reason / 4 條 stage fallback / 7 條 primaryActionKeyFor 全部 expect 字串都改為 settings.firmware.error.message.* / settings.firmware.error.action.*,namespace 對齊完整 |
| device-card.tsx 移除外層 gate | line 64 <FirmwareUpgradeButton device={device} disabled={isBusy} /> 無 device.firmwareCanUpgrade &&;FirmwareUpgradeButton.tsx:26 內部 if (!device.firmwareCanUpgrade) return null 守住、不會 render;isBusy disabled 條件保留、避免 race;single responsibility 落地 |
| estimatedDurationSeconds 行為未變 | 函數 line 212-217 邏輯仍是 if (!deviceType) return 60; if includes('kl720') return 180; return 30、回傳值未變、test 三個 expect 也未變、只是 JSDoc + test 註解補充說明刻意設計 |
無 regression。
§12.2 通用退出條件(6 條、Round 2 重檢)
| # | 條件 | 狀態 |
|---|---|---|
| 1 | No silent failures | ✅ 未觸發。firmware-error-view.tsx:74-77 clipboard reject 靜默是 design 規格允許;新 handleContactSupport 失敗(window.open 回 null)也是 OS handler 沒 mailto 程式、不是 silent failure(mailto 已被 OS 開啟) |
| 2 | No dead code | ✅ 未觸發。grep TODO / FIXME 只在 firmware-badge.tsx:39 TODO(M9-12) — 是預留 M9-12 任務追蹤、不是 dead code |
| 3 | No hardcoded secrets | ✅ 未觸發。mailto: support@kneron.com 不算 secret(公開聯絡信箱);R2-S1 提到 nice-to-have 抽常數 |
| 4 | No unsafe HTML / SQL | ✅ 未觸發。grep dangerouslySetInnerHTML / v-html = 0 hit;mailto subject + body 用 encodeURIComponent 包;React template literal 全自動 escape |
| 5 | Doc 同步 | ✅ 觸發已解。第 1 輪 MJ1 已修、namespace 100% 對齊 Design Spec §9 |
| 6 | Working tree clean | N/A — reviewer 自己只產 report |
A 層 verification(Round 2 simplified)
本輪屬 delta-only review、不重做第 1 輪完整 5 軸;以下為 Round 2 必過項:
- R-A1(delta):逐項驗每條第 1 輪 issue 的修法(見「第 1 輪 issue 修改驗證」表)
- R-A2:i18n namespace 重組後 PRD/TDD/Design Spec §9 對齊一致(grep 0 hit 死引用 + 中英 + types 三邊對齊)
- R-A3:第 2 輪新發現 0 Major、2 Suggestion 都附建議
- R-A4:優點段落見下節
- R-A5:Needs investigation:0(本輪所有疑點都已決議)
- R-A6:§12.2 通用 6 條全跑過
優點(R-A4)
- Frontend 的 pushback 邏輯紮實:採方案 A 並用 SoT 論證、加 forward-compatible 註解(types.ts:303-309)讓 M9-12 接手更平順、不是被動接受 reviewer 也不是無腦堅持。是好的協作姿態。
- M6 「順便修」做得對:第 1 輪只標 disabled / UX 衝突、Frontend 把它升格為實際可用按鈕(mailto + subject + body 帶 errorCode + technicalInfo);不只修「Reviewer 抓出來的」、還主動改善 UX。
- formatTechnicalInfo helper 抽出後三處共用:handleCopy /
<pre>/ handleContactSupport mailto body —— S5 不只解決 readability、實際讓「複製 vs 寄信」內容保證一致、是有實效的重構。 - i18n duration: 6000 配 §5.4 + lib/toast.ts 補 ToastOptions API:解 M4 同時把 toast util 介面 forward-compatible(未來其他 toast 也能調 duration),不是只 patch firmware-upgrade-dialog 一處。
- 9 個新 FirmwareErrorView tests 涵蓋完整:mailto href schema 驗證(
^mailto:)+ body decodeURIComponent 內容驗證、是測試到「行為 + 安全」兩面、不是只測 render。 - JSDoc 引用 Reviewer ticket 編號:
(Reviewer S5)/(Reviewer M1+M4)/(Reviewer S1)/(M3 釐清)— 修改可追溯回 review 條目、未來看 code 知道為什麼這樣寫。 - M3 處置成熟:沒急著統一 fallback 數值(避免行為 regression)、改用 JSDoc + test 註解雙重文件化「刻意設計」;是「文件化 over 程式變更」的好範例。
結論
Round 2 審查結果:✅ 通過、無需 Frontend 第 3 輪
第 1 輪 12 項應修 issue 全部到位、3 項刻意 follow-up 理由合理、1 項 (MJ3) 屬 Backend 範圍按計畫外延。MJ1 採方案 A 是正確選擇、Frontend pushback 邏輯成立、Reviewer 接受。第 2 輪新發現僅 2 個 Suggestion、都屬 nice-to-have、不阻擋。
是否阻擋 M9-5:否。本輪完全不阻擋 M9-5 三平台 KL520+KL720 實機 E2E。
是否需 Frontend 第 3 輪:否。除非 M9-5 階段 testing agent 發現新 issue、否則 M9-4 frontend 部分本輪結案。
剩餘待 follow-up(非阻擋):
- MJ3:Backend smoke test schema、由 Testing Agent / Backend Agent 在 M9-5 階段補
- S2:stage-percent 對照測試、Testing Agent M9-5
- S3:Dialog selector 優化、perf nice-to-have
- S4:FirmwareUpgradeDialog integration test、Testing M9-5 / M9-12
- R2-S1:mailto 地址抽常數、nice-to-have
- R2-S2:handleCopy / techOpen toggle 測試、nice-to-have
是否升 Security Auditor:否。ContactSupport mailto handler 已驗證安全(RFC 6068 scheme + encodeURIComponent + 不引入 XSS / open redirect / hardcoded secret)。