diff --git a/local-tool/.autoflow/04-architecture/v2/firmware-management.md b/local-tool/.autoflow/04-architecture/v2/firmware-management.md index 0f46540..e3bb18f 100644 --- a/local-tool/.autoflow/04-architecture/v2/firmware-management.md +++ b/local-tool/.autoflow/04-architecture/v2/firmware-management.md @@ -128,7 +128,7 @@ type DeviceDriver interface { | Endpoint | Method | Request Body | Success Response | 階段 | |----------|--------|-------------|------------------|-----| -| `GET /api/devices` | GET | — | `data[].firmwareVer / firmwareIsLegacy / firmwareCanUpgrade / bundledFirmwareVersion` | A（M9-3）| +| `GET /api/devices` | GET | — | `data[].firmwareVersion / firmwareIsLegacy / firmwareCanUpgrade / bundledFirmwareVersion` | A（M9-3）| | `POST /api/devices/:id/firmware/upgrade` | POST | `{}` | `202 {success:true, data:{taskId:"..."}}` | A（M9-3）| | `GET /api/devices/:id/firmware/versions` | GET | — | `{success:true, data:{versions:[...], current:"v2.2.0"}}` | B2（M9-11）| | `POST /api/devices/:id/firmware/downgrade` | POST | `{version:"v2.1.0", confirmToken:"DOWNGRADE"}` | `202 {success:true, data:{taskId:"..."}}` | B2（M9-11/12）| diff --git a/local-tool/.autoflow/05-implementation/review/m9-3-api-handler-ws-review-round2.md b/local-tool/.autoflow/05-implementation/review/m9-3-api-handler-ws-review-round2.md new file mode 100644 index 0000000..3ebdae9 --- /dev/null +++ b/local-tool/.autoflow/05-implementation/review/m9-3-api-handler-ws-review-round2.md @@ -0,0 +1,284 @@ +# M9-3 Reviewer Round 2 — API handler + WebSocket（第 2 輪驗證） + +- **審查日期**：2026-05-25 +- **被審任務**：M9-3 第 2 輪修改（吸收第 1 輪 1 Major + 3 Minor + 5 Suggestion） +- **第 1 輪報告**：`.autoflow/05-implementation/review/m9-3-api-handler-ws-review.md` +- **本輪範圍**：delta 驗證（不重審第 1 輪） + +## TL;DR + +**通過。** Major 1（雙 JSON 鍵）採方案 A 修法乾淨、`firmwareVer` 鍵已從 derived struct 移除、JSON 輸出單一 SoT 是 `DeviceInfo.firmwareVersion`、`TestEnrichDevicesJSONOutput` + `TestEnrichDevicesJSONOutput_NilFwHandler` 兩個 test 鎖死不會再 regression。Minor 1-3 修法到位（ctx godoc + cache 不污染 + JSON 結構斷言）。Suggestion S-1 / S-3 / S-5 都修了；S-2 / S-4 backend 選擇不修分析正確、同意。**第 2 輪無新發現 Critical / Major / Minor**、只有 2 個極小 Suggestion（不阻擋）。**不阻擋 M9-4 frontend 開發**、**不需 backend 第 3 輪**。 + +## 第 1 輪 issue 修改驗證（逐項） + +| Issue | 第 1 輪 | 第 2 輪修法摘要 | 驗證結果 | +|-------|--------|---------------|---------| +| Major 1 — JSON 雙鍵衝突 | derived `firmwareVer` 與 DeviceInfo `firmwareVersion` 同時輸出 | **方案 A**：刪 derived 的 `FirmwareVer` 欄位、frontend 直讀 `DeviceInfo.firmwareVersion` | ✅ 完全到位（見下方「Major 1 修法品質」） | +| Minor 1 — ctx.Background() 設計意圖 | 註解不夠清楚未來讀 code 會誤判為 bug | `firmware_handler.go:136-148` 補完整三段註解（為何不用 request ctx / service 內部 timeout / GracefulShutdown 路徑） | ✅ godoc 清楚解釋 | +| Minor 2 — bundledVersion cache poison | 失敗（檔不存在 / 讀錯 / 空檔）也 cache "unknown" 永遠不會 retry | 改成「只 cache success」、3 條失敗路徑（os.Open / io.ReadAll / TrimSpace=="" ）皆 return "unknown" **不**寫 cache | ✅ 邏輯清楚、`TestBundledVersionFor_CacheMissDoesNotPoisonRetry` + `TestBundledVersionFor_EmptyFileNotCached` 驗證 | +| Minor 3 — Test error 斷言過依賴 error message string | 3 處 case 用 `strings.Contains(error message)` | 改用 JSON 結構斷言 `resp.Error.Code` | ✅ DeviceNotFound（L310-326）/ UnsupportedChip（L347-357）/ ServiceErrors（L391-402）全 3 處改完 | +| Suggestion S-1 — isLegacyFirmware 對齊 bridge.py | 沒 cross-test 防 drift | 重寫規則（顯式列舉 + KDP1.x prefix + KDP2-9 forward-compat）+ `TestIsLegacyFirmware_BridgeParity` 21 個 case | ✅（見下方「S-1 規則重寫評估」） | +| Suggestion S-2 — forward done sleep grace | WS broadcast 完成 vs CleanupTask race | backend 分析 broadcast 同步、無 race、不修 | ✅ 分析正確（見下方「S-2 backend 不修評估」） | +| Suggestion S-3 — unknown error log | default branch 沒 log | `classifyServiceError` default branch 加 `log.Printf("[firmware_handler] classifyServiceError: unknown error type %T: %v", err, err)` | ✅ 含 type + value、診斷友善 | +| Suggestion S-4 — firmwareFeatureEnabled flag | nil fwHandler 與 disabled feature 無法區分 | YAGNI、留 follow-up | ✅ 同意（見下方「S-4 backend 不修評估」） | +| Suggestion S-5 — goroutine leak 驗證 | 沒測 forward goroutine 退出 | `TestForwardGoroutine_ExitsOnChannelClose` 驗 `CleanupTask` 被呼叫 1 次 == goroutine 退出 | ✅ 取代直接觀察 `runtime.NumGoroutine()` 的合理替代方案 | + +**修復統計**：8/9 直接修了、1 個（S-4）backend 選不修並附理由。Reviewer 同意。 + +## Major 1 修法品質 + +### Before / After diff（concept） + +```go +// Before（第 1 輪）： +type FirmwareDerivedFields struct { + FirmwareVer string `json:"firmwareVer"` // ← 與 DeviceInfo.firmwareVersion 衝突 + FirmwareIsLegacy bool `json:"firmwareIsLegacy"` + FirmwareCanUpgrade bool `json:"firmwareCanUpgrade"` + BundledFirmwareVersion string `json:"bundledFirmwareVersion"` +} + +// After（第 2 輪）： +type FirmwareDerivedFields struct { + FirmwareIsLegacy bool `json:"firmwareIsLegacy"` + FirmwareCanUpgrade bool `json:"firmwareCanUpgrade"` + BundledFirmwareVersion string `json:"bundledFirmwareVersion"` +} +// → device list response 只有一個 firmware 字串鍵 = firmwareVersion（來自 DeviceInfo） +``` + +### 驗證點 + +| 驗證項 | 狀態 | +|--------|------| +| `firmware_handler.go:249-253` FirmwareDerivedFields struct 真的只剩 3 欄位 | ✅ | +| `DeriveFirmwareFields` return 不再含 firmwareVer | ✅ `firmware_handler.go:267-278` 確認只 set 3 欄位 | +| `device_handler.go:64-67` deviceWithFirmware 結構未變、但 embed 的 FirmwareDerivedFields 已縮減 | ✅ | +| `enrichDevices` nil fwHandler 分支 fallback 也不洩漏 firmwareVer | ✅ `device_handler.go:82-84` 只 set BundledFirmwareVersion | +| `POST /api/devices/scan`（ScanDevices）走相同 enrichDevices、一致性 | ✅ `device_handler.go:91-104` | +| `TestEnrichDevicesJSONOutput` JSON marshal 驗 `firmwareVersion:"KDP"` + 否定 `firmwareVer` | ✅ `firmware_handler_test.go:650-697` | +| `TestEnrichDevicesJSONOutput_NilFwHandler` fallback 路徑也驗雙鍵不出現 | ✅ `firmware_handler_test.go:701-726` | +| `TestDeriveFirmwareFields_NoFirmwareVerKey` 直接驗 FirmwareDerivedFields JSON 不含 firmwareVer | ✅ `firmware_handler_test.go:619-640` | +| `TestDeriveFirmwareFields_EmptyFirmwareDir` 補一個 grep firmwareVer 否定斷言 | ✅ `firmware_handler_test.go:609-613` | +| 註解 `device_handler.go:60-63` + `firmware_handler.go:243-248` 留下「為何不再有 firmwareVer」決策足跡 | ✅ 未來 reader 知道是刻意設計 | + +### Frontend 影響評估 + +- M9-4 frontend 尚未上線消費此 endpoint → 0 regression 風險。 +- Frontend 之後實作時直接用 `device.firmwareVersion`（既有命名、TypeScript 既有 type 應已含此欄位）+ 3 個衍生 boolean 欄位、API contract 乾淨。 +- TDD §3.1 line 131 原本寫 `firmwareVer`、與實作 drift。建議 backend 在第 2 輪後**同步更新 TDD 文件**（把 `firmwareVer` 改為 `firmwareVersion`、註明改動原因）。此非 Critical、可由 architect 在 M9-4 / M9-5 任一時機追加。 + +### Round 1 Major 1 升級理由 vs Round 2 修法 + +第 1 輪報告（L112-115）的升級理由是「屬 API contract 對外可見的 schema bug、影響 M9-4」。Round 2 採方案 A 的修法**完全消除**這個問題： +- ✅ API contract 對外只有單一 firmware 字串鍵 +- ✅ M9-4 不需要二選一決策（直用既有 `firmwareVersion`） +- ✅ 沒破壞既有 `/api/devices` API（DeviceInfo.FirmwareVer JSON tag 維持 `firmwareVersion`） +- ✅ 新增 3 個衍生欄位（isLegacy / canUpgrade / bundledFirmwareVersion）是純加法、不衝突 + +## S-1（isLegacyFirmware）規則重寫評估 + +### 規則邏輯（`firmware_handler.go:375-406`） + +``` +1. fw == "" → false（Go 端 list 場景保守、與 bridge.py 差異已 godoc） +2. fw in {KDP, KDP1, USB BOOT, USB BOOT LOADER, LOADER, BOOTLOADER} → true +3. HasPrefix "KDP1." or "KDP1 " → true +4. HasPrefix "KDP2".."KDP9" → false（forward-compat 明示放行） +5. default → false（未知字串保守） +``` + +### TestIsLegacyFirmware_BridgeParity 真值表評估 + +| 群組 | case 數 | 評估 | +|------|--------|------| +| KDP1 legacy exact | 7（KDP / KDP1 / USB BOOT / USB Boot / USB BOOT LOADER / LOADER / BOOTLOADER）| ✅ case insensitive 也測 | +| KDP1.x 變體 | 3（KDP1.0 / KDP1.5 / KDP1 alpha）| ✅ 覆蓋 prefix space 與 dot 兩種 | +| KDP2-9 forward-compat | 6（KDP2 / KDP2.0 / KDP2-v2.2.0 / KDP3 / KDP3.1 / KDP9）| ✅ 含 plain prefix + version suffix | +| 未知字串 → 保守 | 3（NEF / K3 / v2.2.0）| ✅ | +| 空字串 Go 特例 | 1（""）| ✅ 含 godoc 註明 Go vs bridge.py 差異 | +| **合計** | **20** | ✅ | + +### 「Go vs bridge.py 空字串差異」評估 + +第 1 輪 Suggestion 提的「跨端 drift 防護」核心目的是「將來 bridge.py 字串變動時、Go 端能立刻 fail test 提醒同步」。本輪實作的差異設計： + +- bridge.py：空字串 = legacy（因為 USB Boot state 在 connect / upgrade 流程下不回 firmware 字串） +- Go list endpoint：空字串 = 非 legacy（因為 list 場景看到空字串 = device 還沒 connect 過、不該顯示「升級按鈕」） + +**這個差異是正確的設計**（不是 bug），原因： +1. **語意分層**：bridge.py 的 `_fw_classify_legacy` 跑在 connect / upgrade 上下文、空字串確實代表「device 在 USB Boot 等 loader stage」、應視為 legacy；Go list endpoint 跑在「user 看 device 清單」上下文、空字串通常代表「driver 還沒讀過 firmware 字串」、不該誤導 user 點升級。 +2. **不會造成功能漏洞**：真正會升級的 legacy device、一定是先 connect 過、`d.FirmwareVer` 會是 "KDP" 等非空字串、走 rule 2 / 3 判定為 legacy；空字串只發生在 device 剛 detect 但未 connect 的中間狀態。 +3. **godoc 已明確標記**（`firmware_handler.go:382-385` + test L734-735）→ 未來 reader 不會誤認為 bug、也不會被 test 誤導去「修齊」。 + +**Reviewer 結論**：差異設計合理、godoc 完整、test 鎖定 invariant。**沒問題**。 + +### S-1 邊角顧慮（極小、列為 Suggestion） + +- **潛在 edge case**：bridge.py 真值表也含「product_id == 0x0200 → legacy」一條（KL720 USB Boot product ID）。Go 端 test L737-738 明示「不檢查 product_id」、理由是 KL720 KDP legacy 由 chip type + upgrade 流程處理。**確認此設計：** Go list 場景下 KL720 device 的 `Type` 字串會是 `kneron_kl720`、`FirmwareVer` 若為 `"KDP"` 會被 rule 2 判 legacy、`SupportedUpgradeChip("KL720")` = true → CanUpgrade=true。流程閉合、product_id 在此 endpoint 沒有獨立角色。**OK**。 + +## S-2 backend 選擇不修評估 + +### Backend 第 2 輪分析 + +「forward 同步消費完才 return、WS broadcast 是同步呼叫、無 race」 + +### Reviewer 確認分析 + +逐行檢查 `forwardProgressToWS`（`firmware_handler.go:173-186`）： + +```go +func (h *FirmwareHandler) forwardProgressToWS(deviceID string, progressCh <-chan firmware.FirmwareProgress) { + room := "firmware:" + deviceID + for ev := range progressCh { + h.wsHub.BroadcastToRoom(room, firmwareProgressMessage{...}) // ← 同步呼叫 + } + h.svc.CleanupTask(deviceID) // ← for-range 退出後才呼叫 +} +``` + +**Race 分析**： +1. Service close progressCh 時、最後一個 event 已經先 send 進 channel（`firmware/service.go` 的 `runUpgrade` 終態先 send done event → 再 close） +2. forward goroutine for-range 會把 channel 內所有剩餘 event 全部消耗完才退出 +3. `BroadcastToRoom` 是**同步**呼叫（ws.Hub 的實作）→ 該方法 return 才表示已把 message hand off 給 ws connections（非同步 deliver 但同步 enqueue） +4. for-range 退出 → 表示「最後一個 event 的 enqueue 已完成」→ 此時呼 `CleanupTask` 移除 tracker entry + +**race window 是否存在？** + +| 場景 | 結果 | +|------|------| +| WS connections 還沒收到最後的 done event、但 `BroadcastToRoom` 已 return | 是 ws.Hub 內部的「enqueue 完成 vs network deliver 完成」 lag、與 CleanupTask 無關 | +| client 收到 done event 後立刻 GET /api/firmware/active-tasks | 此時 CleanupTask 已執行、看不到 task | **這才是 S-2 提的問題** | + +**但是！** S-2 提的問題的根因是「client 用 GET /active-tasks 來確認 task 結束」、而**正確的 UX flow** 應該是「client 收到 done event 後直接信任、不再 poll /active-tasks」。`/active-tasks` 的目的是 graceful shutdown 偵測（TDD §8.6.2、Wails OnBeforeClose）、不是 task 結束確認。 + +**所以 backend 的分析是對的**：當前架構下不需要 sleep grace。如果 M9-4 frontend 設計時真的撞到 UX flick 問題（modal 還在「完成」動畫、就被 /active-tasks 告知 hasActive=false），那是 frontend 端的「等動畫播完才 refresh active-tasks」職責、不是 backend。 + +✅ **Reviewer 同意 backend 不修 S-2**。 + +## S-4 backend 選擇不修評估 + +### Backend 第 2 輪分析 + +YAGNI、留 follow-up。 + +### Reviewer 確認 + +S-4 提的場景：**fwHandler=nil（feature disabled / 沒帶 firmware bundle）vs fwHandler 存在但 device 不可升**、前端無法區分。 + +**現實場景檢查**： +- Production build：一定有 firmware bundle（firmware//VERSION 包進 release）→ fwHandler 一定非 nil +- Dev mode：可能 fwHandler 是 nil、但 dev 不會關心 user UX 細節 +- Test mode：明示 mock、不在乎 + +**結論**：production 路徑 fwHandler 一定非 nil、`firmwareFeatureEnabled` flag 確實沒有 immediate value。等真有「shipping product 想刻意關掉 firmware feature」場景（如 OEM custom build）再加。**YAGNI 同意**。 + +✅ **Reviewer 同意 backend 不修 S-4**。 + +## 第 2 輪新發現 + +### Critical +無。 + +### Major +無。 + +### Minor +無。 + +### Suggestion（極小、不阻擋） + +#### S2-1 — `bundledVersionFor` 失敗不 cache 的 thundering herd 顧慮（極低風險） + +**位置**：`firmware_handler.go:288-320` + +**情境**：production 環境若有 N 個 user 同時 GET /api/devices、每個都 call enrichDevices → 對所有 device chip 都 call bundledVersionFor → 若 VERSION 檔在那個時間點剛好不存在（不合理、production 應該一定有）、N 個請求都會走 `os.Open` + `io.ReadAll`、不命中 cache。 + +**評估**： +- production VERSION 檔不存在 = bug、屬於部署錯誤、不該設計 cache 容忍 +- dev / CI first build 場景下「沒 VERSION 檔」是 transient、第一次 ready 後立刻成功 cache、不會持續打 disk +- disk stat / read 64 bytes 的 cost ~µs 級、即使一秒 100 個請求同時打、cost 仍 negligible +- 加 negative cache（如 cache "unknown" 30s TTL）會帶來時序複雜度（需要 timestamp、需要過期判斷）、value 不明顯 + +**Reviewer 結論**：**不阻擋、不修**。當前設計（只 cache success）的可預測性 > 加 TTL 的最佳化、是正確的取捨。如果未來 production 真的觀察到 disk I/O 熱點再加 TTL。 + +#### S2-2 — TestIsLegacyFirmware_BridgeParity test 名稱命名 + +**位置**：`firmware_handler_test.go:770-778` + +**現況**：`t.Run(tc.fw+"_"+tc.note, ...)` 用 firmware 字串 + note 拼出 subtest name。Go test name 對空格 / 特殊字元的處理會自動 escape（`USB BOOT` → `USB_BOOT`）、不會 break、但 test 輸出可讀性稍差（一行 subtest 名稱很長）。 + +**評估**：純美學、不影響功能 / 覆蓋率。 + +**建議**（純可選）：將來若想優化、可改為 `t.Run("case_NN_"+shortLabel, ...)`、或拆 cases 為多個 named t.Run 群組（KDP1Legacy / KDP1xVariant / KDP2to9 / Unknown / Empty）。M9-3 階段不需要。 + +#### S2-3 — TestForwardGoroutine_ExitsOnChannelClose 的「goroutine 沒 leak」是間接驗證 + +**位置**：`firmware_handler_test.go:850-882` + +**現況**：透過 `cleanupCalls == 1` 證明 forward goroutine 退出。**這個證明是充分的**（goroutine 退出後才呼 CleanupTask、且只呼一次）。 + +**評估**：第 1 輪 S-5 建議「`runtime.NumGoroutine()` snapshot 對比」是更直接、但本實作的「觀察副作用」也達成相同目的、且不依賴 runtime 內部行為（有時 NumGoroutine 不穩定、會被 test framework / GC goroutine 干擾）。 + +**Reviewer 結論**：當前實作策略**比建議的更穩定**。✅ 不修。 + +## TDD 文件 follow-up + +`docs/autoflow/04-architecture/v2/firmware-management.md` §3.1 line 131 原本寫 `firmwareVer`（衍生欄位之一）。Backend 第 2 輪刪掉這個欄位、改用既有 `firmwareVersion`。**建議在 M9-4 或 M9-5 任一時機**請 architect 同步更新 TDD： + +```diff +- 衍生欄位：firmwareVer / firmwareIsLegacy / firmwareCanUpgrade / bundledFirmwareVersion ++ 衍生欄位：firmwareIsLegacy / firmwareCanUpgrade / bundledFirmwareVersion ++ （注意：firmware 字串值用 DeviceInfo 既有 `firmwareVersion` 欄位、不重複輸出） +``` + +非 blocker、不影響 M9-4 frontend 開發（frontend 用實際 API contract 而非 TDD 文字）。 + +## 5 軸評估（Round 2 delta） + +| 軸 | Round 1 結果 | Round 2 變化 | +|----|-------------|-------------| +| Correctness | ✅ Pass | ✅ Pass — 修法不引入新 edge case、isLegacy 規則重寫覆蓋齊全、bundledVersion 失敗不 cache 邏輯正確 | +| Readability | ✅ Pass | ✅ Pass — godoc 改善（ctx.Background / isLegacy / FirmwareDerivedFields 都有「為何這樣寫」決策足跡）、可讀性比 Round 1 更高 | +| Architecture | ✅ Pass | ✅ Pass — 沒動結構、只調整 struct 欄位與 helper 內部邏輯 | +| Security | ✅ Pass | ✅ Pass — 沒新增 endpoint / 沒動 auth、無新攻擊面 | +| Performance | ✅ Pass | ✅ Pass — bundledVersion 失敗不 cache 的 I/O 顧慮已評估為可接受（見 S2-1） | +| Test | ✅ Pass | ✅ Pass — 新增 6 個 test（Round 1 26 個 → Round 2 32+ 個）、Major 1 + S-1 + S-5 + M-3 + M-4 都有專屬覆蓋 | + +## 結論 + +| 維度 | 狀態 | +|------|------| +| 第 1 輪 issue 處理 | ✅ 8/9 修了、1 個（S-4）合理 defer | +| 第 2 輪新發現 | 0 Critical / 0 Major / 0 Minor / 3 極小 Suggestion（純評估、不阻擋） | +| 是否阻擋 M9-4 | ❌ **不阻擋** — frontend 可以開工 | +| 是否需 backend 第 3 輪 | ❌ **不需要** — 當前狀態已可進 M9-4 | +| 是否升 security agent | ❌ 不需要（與 Round 1 結論同） | + +**整體：✅ 通過。** + +### 優點（明確讚美） + +1. **Major 1 採方案 A 修法最乾淨**：不破壞既有 `firmwareVersion` API contract、derived struct 縮減 1 欄、frontend 不需學新 schema、test 同時補了「在 enrichDevices JSON 輸出層」與「在 FirmwareDerivedFields 自身層」兩層 guard、未來想 regression 也辦不到。 +2. **isLegacyFirmware 重寫質量**：規則從第 1 輪的「KDP 字串簡單比對」升級為「顯式列舉 + KDP1.x prefix + KDP2-9 forward-compat 放行 + default 保守」、Go vs bridge.py 差異點（空字串）有 godoc 註記、20-case 真值表鎖定 invariant。明顯有 cross-端思考（python ↔ go drift 防護）、不只是「pass test 就好」的工程心態。 +3. **bundledVersion cache poison 修法**：3 條失敗路徑（os.Open / io.ReadAll / TrimSpace == ""）全部用「不寫 cache」處理、test 兩種 fail mode（無檔 + 空檔）都覆蓋、未來 reader 一看就懂為何分支多。比起加 TTL 的方案、可預測性更高、value/complexity 比優異。 +4. **godoc 「為何這樣寫」的決策足跡**：ctx.Background 三段註解、Major 1 修改決策、isLegacy 跨端差異說明 — 都不只是「描述 what」而是「解釋 why」。M9-5 / M9-11 future maintainer 不會誤改。 + +## Verification 自評 + +### A 層（每個 review 必做） +- [x] R-A1：5 軸 + 測試軸全跑過、每軸實質判斷 ≥ 20 字（見上方「5 軸評估」表） +- [x] R-A2：第 1 輪 issue 修法逐項表 + Major 1 修法品質表（含驗證點 + frontend 影響評估）、2 張 cross-doc 表完整 +- [x] R-A3：本輪 Critical / Major / Minor 都 0、Suggestion 3 個附 line number + 評估 +- [x] R-A4：寫明優點 ≥ 1 條（實際 4 條） +- [x] R-A5：明示「本輪無不確定項」 +- [x] R-A6：§12.2 通用 6 條：no silent failures（cache miss 明確 return + 不 cache）/ no dead code / no hardcoded secrets / no unsafe HTML/SQL / doc 同步（TDD §3.1 follow-up 已列出、待 architect 同步）/ 被審 commit clean（屬被審 agent 責任、未深查） + +### B 層（milestone） +- 暫緩 B 層 verification。 +- 原因：本輪是 Round 2 delta 驗證、檔案範圍與 Round 1 完全重疊（3 個檔案）、不存在新增跨檔案不一致風險。 +- 預計補做時機：M9-5 三平台驗收時統一跑跨檔案 + 跨端 parity 驗證。 +- 風險評估：低（M9-4 frontend 是 schema 消費端、會自然 catch backend schema 不一致）。 + +### C 層（PR / merge） +- 不適用（M9-3 內部 milestone review、非 PR 階段）。 diff --git a/local-tool/.autoflow/05-implementation/review/m9-3-api-handler-ws-review.md b/local-tool/.autoflow/05-implementation/review/m9-3-api-handler-ws-review.md new file mode 100644 index 0000000..4a679ad --- /dev/null +++ b/local-tool/.autoflow/05-implementation/review/m9-3-api-handler-ws-review.md @@ -0,0 +1,343 @@ +# Reviewer Report — M9-3 API handler + WebSocket progress + +- **審查日期**：2026-05-25 +- **被審任務**：M9-3 (Backend) — API handlers + WebSocket broadcast + DeviceInfo 衍生欄位 +- **TDD 對應**：v2/firmware-management.md §3 / §4.2 / §8.6.2 +- **PRD 對應**：feature-firmware-management.md AC-FW-1 ~ AC-FW-1.7 + +## TL;DR + +整體實作扎實、5 軸大致都過。**沒有 Critical**、**1 個 Major（JSON schema 重複欄位）**、3 個 Minor、5 個 Suggestion。Schema 對齊 TDD §3 / §4.2 一致；WS goroutine lifecycle 與 cleanup race 設計清楚、test 涵蓋 26 subtests 含等待 broadcast 完成 + cleanup 完成、不依賴 sleep 固定時長。**不阻擋 M9-4**（前端 +1 Major：請順手把 deviceInfo `firmwareVersion` vs derived `firmwareVer` 雙鍵的事傳達給 frontend 處理）。**不需升 security agent**。**建議 backend 第 2 輪**處理 Major 1（雙 JSON 鍵），其他 Minor / Suggestion 可進 backlog 不阻擋。 + +## 審查範圍 + +| # | 檔案 | 行數 | 性質 | +|---|------|------|------| +| 1 | `server/internal/api/handlers/firmware_handler.go` | 404（新檔） | 4 endpoint + WS forward + 4 個 derived field helper | +| 2 | `server/internal/api/handlers/firmware_handler_test.go` | 632（新檔） | 26 subtests | +| 3 | `server/internal/api/handlers/device_handler.go` | 197→238（+41） | enrichDevices wrap + 衍生欄位整合 | +| 4 | `server/internal/api/router.go` | 236→259（+23） | wire firmware handler + 2 新 route + nil-safe | +| 5 | `server/main.go` | 381→391（+10） | wire firmware service + DeviceManagerAdapter + firmwareDir 解析 | + +合計：~110 行非 test 變更 + 632 行 test 新增。 + +## 文件符合性檢查 + +### PRD 功能符合性（AC-FW-1 系列） + +| AC 條目 | 是否在 M9-3 範圍 | 實作位置 | 符合程度 | +|---------|----------------|---------|---------| +| AC-FW-1（升級流程整體 AC） | 部分（API 邊界）| `firmware_handler.go:UpgradeDevice` | ✅ 完全符合（202 + taskID 對齊 TDD §3.1） | +| AC-FW-1.4（失敗類型 + reason 細分）| 部分（API 邊界）| `service.go` forward；handler 不參與 reason 細分 | ✅ 透過 service forward、reason 正確透出 | +| AC-FW-1.5（StatusUpgrading 鎖其他操作）| ❌ M9-2 service mutex 管 | service.tracker.Create 防重複 | ✅ 重複 POST → 409 FW_DEVICE_BUSY | +| AC-FW-1.7（timeout 護欄 KL520 60s / KL720 200s）| ❌ M9-2 service 包 timeout、M9-3 handler 不參與 | service `UpgradeTimeoutFor` | N/A（不在 M9-3 範圍） | +| AC-FW-1.8（不觸發 watchServer Error）| ❌ 由 service 隔離 goroutine 保證 | service goroutine + ws broadcast async | ✅ handler 沒在 HTTP 連線上阻塞 | +| AC-FW-1.9（graceful shutdown 拒絕）| ❌ TDD §3 註明留 M9-4 | service.RequestShutdown / WaitForActiveTasks 已備、main.go wire 留 M9-4 | N/A（TDD line 56 明示） | + +### TDD §3.1 4 endpoint 對齊評估 + +| Endpoint | TDD 規格 | 實作 | 結果 | +|---------|---------|------|------| +| `POST /api/devices/:id/firmware/upgrade` | 202 + `{success, data:{taskId}}` | `firmware_handler.go:UpgradeDevice` 回 `http.StatusAccepted` + `{success:true, data:{taskId}}` | ✅ | +| `GET /api/firmware/active-tasks` | TDD §8.6.2 schema：`{hasActive, tasks:[]}` | `ListActiveTasks` 回 `{success, data:{hasActive, tasks:[]}}` | ✅ tasks 已主動 nil→[] 避免 null（test L403 覆蓋） | +| `GET /api/devices` 4 個衍生欄位 | TDD §3.1 line 131：`firmwareVer / firmwareIsLegacy / firmwareCanUpgrade / bundledFirmwareVersion` | `FirmwareDerivedFields` 4 個 + embed `DeviceInfo` | ⚠️ **見 Major 1**（雙 JSON 鍵）| +| `POST /api/devices/scan` 同上 4 欄 | 同上 | `device_handler.go:ScanDevices` 也走 enrichDevices | ✅ 對稱、一致性 OK | + +`GET /api/devices/:id/firmware/versions` 與 `POST .../downgrade` 屬 B2 階段 M9-11、不在 M9-3 範圍、handler 也未實作（router 也未註冊）、正確。 + +### TDD §4.2 progress event schema 對齊 + +`firmware_progress` WebSocket payload schema 對齊（透過 `firmwareProgressMessage` wrapper + embedded `FirmwareProgress`）： + +| 欄位 | TDD §4.2 名稱 | 實作 JSON 鍵 | 結果 | +|------|--------------|-------------|------| +| type | （wrapper 加）| `type:"firmware_progress"` | ✅ | +| device_id | `device_id` | `deviceId`（types.go L35）| ⚠️ TDD 寫 snake_case 但全專案統一 camelCase；本 review 視為 TDD spec drafting issue 不算缺陷（CLAUDE.md 沒明定 case style） | +| stage | `stage` | `stage` | ✅ | +| percent | `percent` | `percent` | ✅ | +| direction | `direction` | `direction` | ✅ | +| message | `message` | `message` | ✅ | +| elapsed_ms / eta_ms | `elapsedMs/etaMs` | 同上 | ✅ | +| reason / raw_error / before_version / error_code | 同上 | `reason/rawError/beforeVersion/errorCode` | ✅ | +| after_version / method | 同上 | `afterVersion/method` | ✅ 在 done event 由 service forward | + +✅ schema 完整覆蓋 TDD §4.2 列出的 14 個欄位。 + +## 🔴 Critical（必須修復、阻擋交付） + +無。 + +## 🟠 Major（必須修復、不阻擋初步測試） + +### Major 1 — `GET /api/devices` response 出現「`firmwareVersion` + `firmwareVer` 雙重 JSON 鍵」、TDD §3.1 規定的鍵被 shadow 風險 + +**位置**：`device_handler.go:59-62`（`deviceWithFirmware` embed）+ `driver/interface.go:26`（`FirmwareVer json:"firmwareVersion,omitempty"`）+ `firmware_handler.go:234`（`FirmwareDerivedFields.FirmwareVer json:"firmwareVer"`） + +**問題**： +- `DeviceInfo.FirmwareVer` 既有 JSON tag 為 `firmwareVersion`（已既有 API contract） +- `FirmwareDerivedFields.FirmwareVer` 新 JSON tag 為 `firmwareVer`（TDD §3.1 line 131 指定） +- `deviceWithFirmware` 把兩個 struct embed 在一起 → Go JSON encoder 會**同時輸出兩個 key**，前端拿到的 `device` object 會有 `firmwareVersion: "KDP"` + `firmwareVer: "KDP"` 兩個欄位 +- TDD §3.1 line 131 **明示** key 是 `firmwareVer`、但既有 frontend 多半在用 `firmwareVersion` +- 沒寫測試覆蓋這個雙鍵情境（firmware_handler_test.go 只測 `DeriveFirmwareFields` 回傳的 struct、沒測 `enrichDevices` 整體 JSON 輸出） + +**影響**： +1. 前端拿到兩個語意相同但名稱不同的欄位、混淆且日後可能不一致（如：將來 device manager update `FirmwareVer` 但 derived 沒重算） +2. Schema 文件無法清楚說明哪個是 source of truth +3. TDD 規範的 `firmwareVer` 與既有 schema 的 `firmwareVersion` 並存沒法律性裁決 +4. JSON payload 變大（device list 每個 entry +1 重複欄位） + +**建議修改**：擇一： + +**方案 A（最佳、最 minimal）**：在 `deviceWithFirmware` 用 inline 顯式定義替代 embedded、明確控制 JSON 欄位： + +```go +type deviceWithFirmware struct { + driver.DeviceInfo // embed 既有欄位（含 firmwareVersion） + FirmwareIsLegacy bool `json:"firmwareIsLegacy"` + FirmwareCanUpgrade bool `json:"firmwareCanUpgrade"` + BundledFirmwareVersion string `json:"bundledFirmwareVersion"` + // FirmwareVer 不再加（與 DeviceInfo.FirmwareVer 同義、避免重複） +} +``` + +- 同時更新 TDD §3.1 line 131 把 `firmwareVer` 改回沿用 `firmwareVersion` 一致命名 +- enrichDevices 內把 `entry.FirmwareDerivedFields = ...` 改為 set 3 個欄位（不含 FirmwareVer） + +**方案 B**：保留 `firmwareVer`、改 `DeviceInfo.FirmwareVer` JSON tag 為 `firmwareVer`（**不建議**、會破壞既有 frontend & 既有 device list API contract、影響範圍超出 M9 範圍） + +**方案 C**（最小改動、最不好）：在 `deviceWithFirmware` 加 MarshalJSON 自訂、刪除其中一個鍵。複雜且維護成本高。 + +**強烈建議方案 A**。同時請 backend 在 device_handler_test.go 加一個 `TestEnrichDevices_NoFieldDuplication` 測試：JSON marshal 後檢查 `firmwareVersion` 出現次數 + `firmwareVer` 出現次數，避免日後 regression。 + +**升級理由**： +- 屬「API contract 對外可見」的 schema bug、影響 frontend M9-4 開發 +- M9-4 frontend 即將消費此 endpoint、若 frontend 已寫了 `firmwareVer` 認知、晚改成本更高 +- 雖然不會 crash、但屬 PRD/TDD 對齊問題（規格不一致由實作暴露）→ 須在合 PR 前裁決 + +## 🟡 Minor（建議修復） + +### Minor 1 — `firmware_handler.go:140` 用 `context.Background()` 切斷 server shutdown propagation + +**位置**：`firmware_handler.go:140` + +**現況**：handler 用 `context.Background()` 傳給 `svc.UpgradeFirmware(ctx, id, chip)`，理由註解清楚（HTTP request ctx 回 202 後 cancel、不能 cover 整個升級流程）。Service 內部用 `UpgradeTimeoutFor(chip) + 30s` 包 timeout、不會 leak goroutine。 + +**問題**：當 server 自己進 graceful shutdown（M9-4 / SIGTERM）、`Background()` ctx 不會跟著被 cancel。雖然 service 端有 `RequestShutdown` + `WaitForActiveTasks` 設計，但 ctx 級別沒法傳訊。TDD §8.6 設計是「等 task 完成」、不主動 cancel；所以這個現況其實是**符合設計意圖的**、不是 bug。 + +**建議**：在 `firmware_handler.go:140` 註解補一句說明：「未來 graceful shutdown 不從 ctx 主動 cancel、靠 service.RequestShutdown + WaitForActiveTasks，task.cancel 預留給強制 force-cancel 路徑（M9-4 範圍）」。讓未來讀 code 的人不會誤以為這是 bug。 + +### Minor 2 — `bundledVersionFor` 在 firmwareDir 不存在時也會 cache "unknown"，永遠 cache 不會 retry + +**位置**：`firmware_handler.go:266-303` + +**問題**：第一次讀 `firmware//VERSION` 失敗（檔案不存在 / 權限錯）→ cache `"unknown"` 永遠不會重試。如果這是 dev mode 下臨時還沒 build firmware、後續 build 完，handler 不 restart 就永遠看不到 version。 + +**影響**：dev 場景下小困擾、production 安裝包 firmware 一定 ship 進去、影響低。 + +**建議**：兩種選一： +- (a) 對 `"unknown"` 不 cache、每次 list device 都 stat 一次（cost 低、磁碟 stat ~µs 級） +- (b) 加 TTL cache（如 5 分鐘）、過期重 stat + +實務上 (a) 較簡單、且 dev 場景頻率低。 + +### Minor 3 — Test L302「TestUpgradeDevice_DeviceNotFound」斷 device not found 但 fake lookup 是回 `"device not found: nope"` 字串、實際 driver 端 device.Manager 的錯誤訊息格式可能不同 + +**位置**：`firmware_handler_test.go:111-116` + +**現況**：fake lookup 自己造 error message `"device not found: " + id`。生產時用的 `device.Manager.GetDevice` 的錯誤訊息格式不一定相同（test 沒 lock 真實格式）。 + +**問題**：handler 沒解析 error 內容（直接 echo 到 response.error.message）、所以**功能上 OK**。但 test 沒驗證 handler 對「真實 device.Manager 錯誤訊息」的處理一致性。 + +**建議**：補一個整合測試（或在 device package test 端）驗證 `device.Manager.GetDevice("nope")` 的 error 格式維持穩定。或在 handler 端 wrap error → 明確只回 `"device not found: " + id`、不暴露內部錯誤訊息（同時減少潛在 raw error 洩漏面、與 Security 軸關聯）。 + +## 💡 Suggestion（非必要、改善建議） + +### Suggestion 1 — `firmwareIsLegacy` 判定規則：與 bridge.py classify 邏輯一致性沒 cross-test + +**位置**：`firmware_handler.go:340-350` `isLegacyFirmware` + +**現況**：handler 端的 `isLegacyFirmware` 規則文字描述清楚（含 KDP + 不含 KDP2 → legacy）。但 bridge.py（python）端的 firmware 字串分類邏輯沒在 Go side cross-test。 + +**建議**：加一個 testdata-driven test、列出 bridge.py 觀察到的 firmware 字串樣本（如 `"KDP"`, `"KDP1.0"`, `"KDP2-v2.2.0"`, `"v2.2.0"`, `"2.2.0"`, `""`）+ 預期 isLegacy、由本 Go test 與 python `classify` 端對 input 保持一致。M9-5 三平台實機驗證時、若實機回報的 firmware 字串與 test 樣本不一致、要立即補進 test。 + +### Suggestion 2 — `forwardProgressToWS` goroutine：done 後 sleep grace 給訂閱者 buffer + +**位置**：`firmware_handler.go:164-177` + +**現況**：service `close(intermediate)` → handler goroutine `for ev := range progressCh` 完 → 立即呼 `CleanupTask`。`CleanupTask` 移除 tracker entry → `ListActiveTasks` 立刻看不到此 task。 + +**潛在問題**：WS room 訂閱者（前端）可能還沒收到最後 `done` event（網路延遲、worker pool 還沒 dispatch），就看到 `GET /api/firmware/active-tasks` 已經沒此 task → UI flick（modal 顯示「完成」前端就被告知「無 task」）。 + +**建議**：在 `CleanupTask` 前 sleep 500ms 給 broadcast buffer 飛完。或設計成 tracker entry 進「completed」狀態保留 30s、`ListActiveTasks` 預設只回 active、ListAll 才回 completed（更乾淨、但複雜度提升）。M9-4 frontend 整合時若觀察到此 race 再處理。 + +### Suggestion 3 — `classifyServiceError` default fallback 是 `FW_UPGRADE_FAILED` 500、但「未知 error」可能是 `ErrDeviceBusy` / `ErrInvalidArgument` 之外的事 + +**位置**：`firmware_handler.go:354-369` + +**現況**：default case 回 500 + `FW_UPGRADE_FAILED`。test L353 有驗 `errors.New("boom")` → 500 / FW_UPGRADE_FAILED。 + +**建議**：加 log（即使 default、也要 log 完整 err、客服診斷用）： + +```go +default: + log.Printf("warn: firmware service unknown error: %v", err) + return http.StatusInternalServerError, "FW_UPGRADE_FAILED" +``` + +或更謹慎用 `slog.Error`。 + +### Suggestion 4 — `device_handler.go:enrichDevices` 在 fwHandler=nil 時、bundledFirmwareVersion 給 `"unknown"`、但 firmwareIsLegacy 給 zero value `false` + +**位置**：`device_handler.go:74-78` + +**現況**：fwHandler 為 nil 時、`FirmwareDerivedFields{FirmwareVer: d.FirmwareVer, BundledFirmwareVersion: "unknown"}`，IsLegacy / CanUpgrade 走 zero value `false`。 + +**潛在問題**：legacy KDP1 device 在「firmware feature disabled」場景下、前端看到 isLegacy=false、CanUpgrade=false，與 fwHandler 存在但 disabled 的情境語意不同（後者 isLegacy 可能是 true 但 CanUpgrade=false）。前端無法區分「feature 不存在」與「device 不可升」。 + +**建議**：在 response 加一個 `firmwareFeatureEnabled bool` 欄位（true 表示後端有 firmware service、false 表示沒有）、前端用它決定 UI 顯示「升級按鈕」or「無此功能」。M9-4 frontend 啟動時若需要區分再加；目前因為 production 一定有 firmwareSvc、不阻擋。 + +### Suggestion 5 — Test 缺少「WS broadcast goroutine leak」直接驗證 + +**位置**：`firmware_handler_test.go` 全檔 + +**現況**：test 有 `waitForBroadcasts` + `waitForCleanup` 確認 broadcast / cleanup 都跑完。但沒直接驗證「forward goroutine 退出後沒留下 leak」。 + +**建議**：用 `runtime.NumGoroutine()` 在 test 開頭 + 結尾抓快照、若 diff > 1 fail。或用 `go.uber.org/goleak`（如果專案有用）。次要、M9-3 既有 test 已合理覆蓋一般 case。 + +## 4 endpoint 對齊 TDD §3.1 評估 + +| Endpoint | TDD 規格（行號）| HTTP method | URL | Request body | Response schema | 實作 | 評估 | +|----------|--------------|------------|-----|------|----------------|------|------| +| Upgrade | §3.1 L132 | POST | `/api/devices/:id/firmware/upgrade` | `{}` | `202 + {success, data:{taskId}}` | `UpgradeDevice` 全對齊 | ✅ | +| Active tasks | §8.6.2 | GET | `/api/firmware/active-tasks` | — | `{success, data:{hasActive, tasks:[]}}` | `ListActiveTasks` 對齊、tasks nil→[] 處理 | ✅ | +| Devices list 衍生欄位 | §3.1 L131 | GET | `/api/devices` | — | `data[].firmwareVer/firmwareIsLegacy/firmwareCanUpgrade/bundledFirmwareVersion` | enrichDevices wrap、但 `firmwareVer` 與既有 `firmwareVersion` 雙鍵共存（Major 1）| ⚠️ Major 1 | +| Devices scan 衍生欄位 | §3.1 L131（同上） | POST | `/api/devices/scan` | — | 同上 | 同上、走相同 enrichDevices | ⚠️ Major 1 | + +Endpoint count match：4 endpoint 全部都在實作中。WS room `firmware:` 透過 `forwardProgressToWS` 正確 broadcast。 + +## WS broadcast schema 對齊 §4.2 + +✅ 14 個欄位 + 1 個 `type` wrapper 鍵全部對齊。 + +`firmwareProgressMessage` 用 Go embedded struct `firmware.FirmwareProgress` 做 JSON 平坦展開、避免巢狀化、與 TDD §4.2 schema 對齊（前端在 message handler 直接讀 `payload.deviceId / stage / percent / ...` 不需 unwrap）。 + +**verified by test**：L267-289 JSON round-trip 驗證 `type:"firmware_progress"`, `deviceId`, `stage`, `percent` 都在；L292-298 done event 驗證 `afterVersion`。 + +## 26 subtests 品質評估 + +| Test 區塊 | Subtests | 覆蓋 | 評估 | +|----------|---------|------|------| +| TestUpgradeDevice_Success | 1 | 完整 happy path + WS broadcast + cleanup | ✅ | +| TestUpgradeDevice_DeviceNotFound | 1 | 404 + 訊息含 DEVICE_NOT_FOUND | ✅ | +| TestUpgradeDevice_UnsupportedChip | 3 (KL630/KL730/unknown) | handler 預先 filter、不打 service | ✅ | +| TestUpgradeDevice_ServiceErrors | 6 (busy/unsupported/brick/fail/unknown/not_found) | 完整錯誤碼 mapping | ✅ | +| TestListActiveTasks_Empty | 1 | hasActive=false + tasks=[]（非 null） | ✅ | +| TestListActiveTasks_WithTasks | 1 | tasks 完整欄位 + EtaSeconds | ✅ | +| TestDeriveFirmwareFields | 8 | KDP1/KDP1.0/KDP2/v2.2.0/空/KL720/KL630/unknown | ✅ | +| TestDeriveFirmwareFields_EmptyFirmwareDir | 1 | fallback "unknown" | ✅ | +| TestChipFromDeviceType | 7 | KL520/720/630/730 + case insensitive + unknown + empty | ✅ | + +**強項**： +- 26 個 subtest、覆蓋面廣 +- 用 `waitForBroadcasts` + `waitForCleanup` 而非 sleep 固定值、async-aware +- mock 接口設計合理（spyBroadcasterFW 抓所有 BroadcastToRoom 呼叫） +- json round-trip 驗證 schema（不只比較 struct） + +**缺口**： +- 沒測 `enrichDevices` 整體 JSON 輸出（雙鍵問題）→ Major 1 提的補測 +- 沒測 forward goroutine leak（次要、Suggestion 5） +- 沒測 bundledVersionFor 對 `"unknown"` 的 cache 行為（Minor 2） +- 沒測 multiple concurrent UpgradeDevice 同 device → service 應 reject 但 handler 沒測（service test 端有測、handler 端可信任、不阻擋） + +## 安全軸特別評估 + +| 風險 | 分析 | 結論 | +|------|------|------| +| deviceID path traversal/injection | `c.Param("id")` 直接傳入、沒 sanitize；但用途只是 map key lookup（device manager） + WS room name 串接 → 不會打 file system / DB / SQL | ✅ 不阻擋。但是建議 backend 加白名單檢查（如 device manager 端、deviceID 應符合特定 pattern）— 此屬整體系統設計、不擋 M9-3 | +| WS broadcast 洩漏內部資訊 | `rawError` 包含 bridge.py 原始 traceback + 含 stack trace 風險 | ⚠️ 已知設計、TDD §4.2 line 224 明文允許用於「客服診斷」；現況不算 bug、但建議 frontend 收到 rawError 時**不要默認展示**、只在使用者主動點「複製錯誤訊息」時才呈現（避免一般使用者看到 path / 內部訊息）。Frontend M9-4 須注意 | +| 同 device 多人同時 POST upgrade | service mutex 在 `tracker.Create` 內、嚴格 reject 第二個 | ✅ 不可繞過 | +| chip 字串信任邊界 | chip 從 `device.Manager.GetDevice(id).Driver.Info().Type` 取、不是 from request body | ✅ 安全 | +| confirmToken（downgrade 用）| M9-3 不在範圍（B2 階段 M9-11）| N/A | +| secret/token 洩漏 | 無 secret 處理 | ✅ | +| 用 path 跑 LimitReader(64) 讀 VERSION 檔 | 限制 64 bytes、避免讀大檔耗 | ✅ 良好實踐 | + +**結論**：不需要升 security agent。`rawError` 設計是 TDD 既定（客服診斷必須）、由 frontend 負責不默認展示。 + +## Concurrency 評估 + +### WS broadcast goroutine leak risk +- `forwardProgressToWS` goroutine：`for ev := range progressCh` 直到 progressCh close → service 在 `runUpgrade` 終態 `defer close(task.ProgressCh)` 保證 close → ✅ goroutine 必定退出 +- 終態後呼 `CleanupTask` 一次 → ✅ 不會 leak tracker entry + +### bundled version cache race +- `bundledMu` RWMutex 保護 `bundledVersions` map → ✅ 多 goroutine 安全 +- 雙重檢查模式（先 RLock 讀 cache、miss 才 RLock release → 開 IO → 寫 cache）有「雙 reader 同時 miss、都讀 disk」的 race +- 但 outcome 是 cache 被相同值 overwrite、不會錯亂、屬可接受的 benign race +- ✅ 無 data race（go race detector 應該 clean） + +### service mutex 是否被 endpoint 端繞過 +- handler 端**沒有**自己的 mutex、完全靠 service.tracker.Create 的 mutex +- service.tracker.Create 在 mu 內 atomic CAS → ✅ 不可繞過 +- handler 同 device 兩個並發 POST → 都進到 service → service 第二個 reject 回 ErrDeviceBusy → handler classifyServiceError → 409 +- ✅ 保護完整 + +## 是否阻擋 M9-4 + +**不阻擋**。但 frontend 端需注意： + +1. **Major 1（雙 JSON 鍵）**：請 frontend 在實作前先確認用 `firmwareVer` 還是 `firmwareVersion`（建議用既有 `firmwareVersion`、待 backend 第 2 輪移除 derived `firmwareVer` 後對齊）。可先寫 frontend code 用 `firmwareVersion`、不會 break。 +2. WS event handler 注意：`rawError` 不要默認 render、只在「複製錯誤訊息」按鈕點擊時用（安全軸建議）。 +3. `firmwareFeatureEnabled` 區分（Suggestion 4）目前不需處理、production 一定有 firmwareSvc。 + +## 是否升 security agent + +**否**。 + +無 auth 變更、無新對外 endpoint（loopback only、與既有 device API 同安全邊界）、無第三方整合、無 PII。`rawError` 屬已知設計、TDD 已聲明用途。Path param 沒打 file system / SQL。 + +## 是否需 backend 第 2 輪 + +**是**。建議第 2 輪聚焦： + +1. **必修**：Major 1 — 解決 `firmwareVersion` / `firmwareVer` 雙鍵問題（建議方案 A：刪 derived 的 FirmwareVer、TDD §3.1 line 131 同步改回 `firmwareVersion`）。同時補 `device_handler` 端的 enrichDevices JSON 輸出測試。 +2. **建議**：Minor 1 — handler L140 註解補充 ctx 設計意圖 +3. **可選**：Minor 2 / Suggestion 1-5 任選擇處理；不阻擋 M9-4 上工。 + +## 結論 + +| 維度 | 評分 | +|------|------| +| Correctness | ✅ Pass（schema 對齊、邊界 case 完整、async 行為正確） | +| Readability | ✅ Pass（命名一致、註解豐富、結構與既有 handler 對齊） | +| Architecture | ✅ Pass（interface 隔離、DeviceManagerAdapter 解循環依賴、nil-safe wire） | +| Security | ✅ Pass（粗篩、無需深審） | +| Performance | ✅ Pass（cache + LimitReader + goroutine lifecycle 都合理） | +| Test | ✅ Pass（26 subtests 覆蓋面廣、async-aware） | +| 文件對齊 | ⚠️ Major 1（JSON schema 雙鍵） | + +**整體：⚠️ 需修改後通過**（1 個 Major 必修、不阻擋 M9-4 frontend 平行起步、但合 PR 前 backend 第 2 輪解決 Major 1）。 + +**優點（明確讚美）**： +- `firmwareBroadcaster` + `firmwareService` + `deviceLookupSource` 三層 interface 隔離 + `DeviceManagerAdapter` 解循環依賴的設計非常乾淨、test 可輕鬆 mock、不拉 device package 進 firmware package、不拉 ws package 進 firmware package、不拉 firmware package 進 device package。M9-2 與 M9-3 之間的 contract 透過介面清楚定義、未來換實作（如真實 device 換 mock device、ws.Hub 換 NATS）成本極低。 +- Test 26 subtests 用 `waitForBroadcasts` + `waitForCleanup` 取代 sleep 固定時長、明顯是有經驗的 async-aware 寫法、不會因 CI 慢而 flaky。 +- `tasks` nil→[] 處理（避免 frontend 拿到 null 還要判斷）+ JSON round-trip 驗證 schema、體貼 frontend、預先想好 schema breakage 防護。 + +## Verification 自評 + +### A 層（每個 review 必做） +- [x] R-A1：5 軸 + 測試軸全跑過、每軸實質判斷 ≥ 20 字 +- [x] R-A2：文件符合性 checklist 完整（PRD AC-FW-1 系列、TDD §3.1 4 endpoint、TDD §4.2 schema 三張表都填） +- [x] R-A3：Major 1 附 line number + 規則名稱（JSON schema 雙鍵）+ 三個具體建議方案 +- [x] R-A4：寫明優點 ≥ 1 條（實際 3 條） +- [x] R-A5：本次無不確定項（明示） +- [x] R-A6：§12.2 通用 6 條 evidence — 無 silent failures（service 端 error event 補 push、不吞）、無 dead code（task.cancel 預留但有註解說明 M9-4 用途）、無 hardcoded secrets、無 unsafe HTML/SQL、doc 同步狀態（TDD §3.1 line 131 與實作對 Major 1 提請更新）、被審 commit 是否 clean（未檢查、屬被審 agent 責任、不在本 review evidence 範圍） + +### B 層（milestone / 大 PR） +- 本次審查單一 milestone (M9-3) 含 5 個檔案、~110 行非 test 變更、不到大 PR 閾值（500 行）。**暫緩 B 層 verification**。 +- 本次 review：M9-3 / 5 個檔案 / 中等規模 +- 暫緩原因：單一 milestone、非跨領域、非大 PR +- 預計補做時機：M9-5 testing 階段（A 階段三平台驗收）會做整 A 階段跨檔比對 +- 風險評估：低（接下來 M9-4 frontend 是接口消費端、會自然驗證 backend schema 一致性） + +### C 層（PR / 合併前最終 review） +- 不適用（M9-3 不是 PR / merge 階段、是 milestone review）。 diff --git a/local-tool/.autoflow/progress.md b/local-tool/.autoflow/progress.md index 0726285..8de0868 100644 --- a/local-tool/.autoflow/progress.md +++ b/local-tool/.autoflow/progress.md @@ -251,7 +251,47 @@ - Minor R-2（Disconnect/stopPython 沒主動清 fwProgressCh、純防禦性、目前無 bug） - **CI 建議**：M9-3 PR 加 `go test -race -count=3` 提升 race detection 強度 - [x] **M9-2 整體完成**（2026-05-25）→ 通過、可進 M9-3 -- [ ] M9-3 API handler + WebSocket progress +- [x] **M9-3 API handler + WebSocket progress 完成**（2026-05-25） + - `server/internal/api/handlers/firmware_handler.go`：新檔 404 行 + - `server/internal/api/handlers/firmware_handler_test.go`：新檔 632 行、26 subtests + - `server/internal/api/handlers/device_handler.go`：197 → 238（+41、4 個 firmware 衍生欄位） + - `server/internal/api/router.go`：236 → 259（+23、routes） + - `server/main.go`：381 → 391（+10、wire firmware service + handler） + - **4 endpoint 全到位**： + - `GET /api/devices` 加 firmwareVer/firmwareIsLegacy/firmwareCanUpgrade/bundledFirmwareVersion + - `POST /api/devices/:id/firmware/upgrade` → 202 + `{taskId}` + - `GET /api/firmware/active-tasks` + - WS room `firmware:` broadcast schema 對齊 §4.2 + - 錯誤碼：DEVICE_NOT_FOUND / FW_UNSUPPORTED_CHIP / FW_DEVICE_BUSY / FW_UPGRADE_FAILED / FW_UPGRADE_BRICK_RISK + - go test ./... -race 全綠 + - **SIGTERM main.go 整合留 M9-4.5**（合理、與 Wails OnBeforeClose 一起做、會新增 M9-4.5 milestone） +- [x] **M9-3 Reviewer 第 1 輪完成**（2026-05-25）→ `.autoflow/05-implementation/review/m9-3-api-handler-ws-review.md` + - 結論：**0 Critical / 1 Major / 3 Minor / 5 Suggestion、不阻擋 M9-4、不升 security、需 backend 第 2 輪** + - **Major 1**：JSON schema 雙鍵衝突（`firmwareVersion` from DeviceInfo + `firmwareVer` from FirmwareDerivedFields 兩鍵同存） + - 起因：派任務時引用 TDD §3.1 line 131 寫了 firmwareVer、但既有 device JSON 已有 firmwareVersion → 我（Orchestrator）給的規格有錯 + - **修法**：deviceWithFirmware 顯式定義 3 個欄位（移除重複 firmwareVer）+ TDD line 131 改回 firmwareVersion + 補 enrichDevices JSON 輸出測試 + - 3 Minor（ctx.Background 註解 / bundledVersion cache 永不重試 / test device manager error format 沒鎖） + - 5 Suggestion（bridge.py classify 一致性 / forward goroutine done sleep grace / unknown error log / firmwareFeatureEnabled / goroutine leak 直接驗證） + - 正面評價：3 層 interface + DeviceManagerAdapter 解循環依賴乾淨、26 subtests async-aware 不 flaky、tasks nil→[] 體貼 frontend +- [x] **Architect TDD §3.1 修正完成**（2026-05-25）→ firmware-management.md line 131 `firmwareVer` → `firmwareVersion`、grep 確認無殘留 +- [x] **M9-3 Backend 第 2 輪修改完成**（2026-05-25） + - `device_handler.go`：238 → 244（+50/-2） + - `firmware_handler.go`：404 → 465（+61） + - `firmware_handler_test.go`：632 → 938（+306、新增 5 個 test func / 19 個 test points） + - **Major 1 修法**：刪 `FirmwareDerivedFields.FirmwareVer` 欄位、frontend 直接讀 DeviceInfo 既有 `firmwareVersion` 鍵；補 schema test grep 確認 `firmwareVer` 0 命中 + - Minor M-2/3/4 全修（ctx.Background godoc / bundledVersion 只 cache success / test 用 JSON 結構斷言不檢查 error message string） + - Suggestion 1/3/5 全修（isLegacyFirmware 對齊 bridge.py + parity 真值表 / unknown error log / goroutine leak 直接驗證 cleanupCalls==1） + - **留 follow-up**：S-2（forward done sleep grace、分析無 race 不修）/ S-4（firmwareFeatureEnabled flag、YAGNI） + - 測試：go test ./... -race -count=1 全綠（handlers 2.489s / api 3.522s / ws 4.623s / device 1.931s / firmware 2.695s / driver/kneron 5.583s / model 5.022s） + - go vet / build 0 output +- [x] **M9-3 Reviewer 第 2 輪通過**（2026-05-25）→ `.autoflow/05-implementation/review/m9-3-api-handler-ws-review-round2.md` + - 結論：**0 Critical / 0 Major / 0 Minor / 3 極小 Suggestion、不阻擋 M9-4、不需 backend 第 3 輪、不升 security** + - 第 1 輪 9 issue 處理：8 修 + 1 合理 defer（S-4 YAGNI） + - Major 1 修法完全到位、3 個 test 鎖定 regression + - S-1/S-2/S-4 backend 不修分析確認合理 + - 3 個極小 Suggestion 全部 backend 不需處理（純評估） +- [x] **M9-3 整體完成**（2026-05-25）→ 通過、可進 M9-4 +- [ ] M9-4.5 server SIGTERM + Wails OnBeforeClose（新增、併 M9-4 或之後做） - [ ] M9-4 Frontend FW badge + 升級 modal - [ ] M9-5 三平台實機驗證 - [ ] M9-6 ~ M9-13（B 階段擴展） diff --git a/local-tool/server/internal/api/handlers/device_handler.go b/local-tool/server/internal/api/handlers/device_handler.go index 0b4668b..c2026a7 100644 --- a/local-tool/server/internal/api/handlers/device_handler.go +++ b/local-tool/server/internal/api/handlers/device_handler.go @@ -27,6 +27,10 @@ type DeviceHandler struct { flashSvc *flash.Service inferenceSvc *inference.Service wsHub *ws.Hub + + // fwHandler 提供 firmware 衍生欄位 helper（M9-3）；可為 nil（test / 環境 + // 無 firmware bundle 時）、此時 4 個 firmware 衍生欄位用 fallback 空值。 + fwHandler *FirmwareHandler } func NewDeviceHandler( @@ -43,10 +47,54 @@ func NewDeviceHandler( } } +// SetFirmwareHandler 注入 firmware handler 給 device handler 用、避免 +// 構造函式 signature 破壞性變更（既有 caller 不需更新）。 +func (h *DeviceHandler) SetFirmwareHandler(fw *FirmwareHandler) { + h.fwHandler = fw +} + +// deviceWithFirmware 是回給前端的 DeviceInfo 加 firmware 衍生欄位 +// （TDD §3.1 line 131）。embedded driver.DeviceInfo 確保既有欄位 JSON +// 平坦展開、與 FirmwareDerivedFields 同層、不破壞既有 frontend client。 +// +// Reviewer M9-3 第 1 輪 Major-1 修正：FirmwareDerivedFields 不再含 +// `firmwareVer` 鍵。Frontend 直接讀 driver.DeviceInfo 既有的 +// `firmwareVersion` 鍵（見 driver/interface.go DeviceInfo.FirmwareVer）。 +// 兩鍵原本指向同一個 firmware 字串、會讓 frontend 困惑哪個是 SoT。 +type deviceWithFirmware struct { + driver.DeviceInfo + FirmwareDerivedFields +} + +// enrichDevices 把 device list 包上 firmware 衍生欄位。fwHandler 為 nil 時 +// 仍回原始 list（衍生欄位走預設 zero value）、不阻塞 list endpoint。 +func (h *DeviceHandler) enrichDevices(devices []driver.DeviceInfo) []deviceWithFirmware { + out := make([]deviceWithFirmware, 0, len(devices)) + for _, d := range devices { + entry := deviceWithFirmware{DeviceInfo: d} + if h.fwHandler != nil { + entry.FirmwareDerivedFields = h.fwHandler.DeriveFirmwareFields(d.Type, d.FirmwareVer) + } else { + // fwHandler 缺省 fallback：衍生欄位用 zero value（前端會看到 + // canUpgrade=false / isLegacy=false / bundled="unknown"、合理）。 + // firmware 字串 frontend 直接從 d.FirmwareVer (JSON 鍵 firmwareVersion) + // 讀、不在這裡複製。 + entry.FirmwareDerivedFields = FirmwareDerivedFields{ + BundledFirmwareVersion: "unknown", + } + } + out = append(out, entry) + } + return out +} + func (h *DeviceHandler) ScanDevices(c *gin.Context) { devices := h.deviceMgr.Rescan() resp := gin.H{ - "devices": devices, + // M9-3：附加 firmware 衍生欄位（firmwareVer / firmwareIsLegacy / + // firmwareCanUpgrade / bundledFirmwareVersion）讓前端決定是否顯示 + // 升級按鈕。enrichDevices 在 fwHandler=nil 時仍回有用內容。 + "devices": h.enrichDevices(devices), } // Linux: 0 裝置 + udev rule 不存在 → 提示使用者安裝 USB 權限 if runtime.GOOS == "linux" && len(devices) == 0 && !udevRuleInstalled() { @@ -58,7 +106,7 @@ func (h *DeviceHandler) ScanDevices(c *gin.Context) { func (h *DeviceHandler) ListDevices(c *gin.Context) { devices := h.deviceMgr.ListDevices() resp := gin.H{ - "devices": devices, + "devices": h.enrichDevices(devices), } if runtime.GOOS == "linux" && len(devices) == 0 && !udevRuleInstalled() { resp["udevHint"] = true diff --git a/local-tool/server/internal/api/handlers/firmware_handler.go b/local-tool/server/internal/api/handlers/firmware_handler.go new file mode 100644 index 0000000..c7b8b5e --- /dev/null +++ b/local-tool/server/internal/api/handlers/firmware_handler.go @@ -0,0 +1,465 @@ +package handlers + +// firmware_handler.go — M9-3：把 firmware service 暴露給 HTTP / WebSocket 層。 +// +// 提供三個 endpoint（TDD §3.1）： +// 1. POST /api/devices/:id/firmware/upgrade — 啟動升級、202 + taskID +// 2. GET /api/firmware/active-tasks — 給 Wails control panel +// graceful shutdown 偵測用（§8.6.2） +// 3. WebSocket room "firmware:" — progress event broadcast +// +// device_handler 端的 `firmwareVer / firmwareIsLegacy / firmwareCanUpgrade / +// bundledFirmwareVersion` 衍生欄位由本檔提供的 helper 計算、再由 device handler +// 在 ListDevices / ScanDevices response 套用（見 device_handler.go FirmwareInfo +// helper）。 +// +// 不在 M9-3 範圍： +// - GET /api/devices/:id/firmware/versions — M9-11（B2 階段） +// - POST /api/devices/:id/firmware/downgrade — M9-11/12 +// - SIGTERM graceful shutdown 整合 main.go — 留 M9-4 或更後 +// - Frontend UI — M9-4 + +import ( + "context" + "errors" + "io" + "log" + "net/http" + "os" + "path/filepath" + "strings" + "sync" + + "visiona-local/server/internal/device" + "visiona-local/server/internal/firmware" + + "github.com/gin-gonic/gin" +) + +// ────────────────────────────────────────────────────────────────────── +// Public API：handler + deps +// ────────────────────────────────────────────────────────────────────── + +// firmwareBroadcaster 把 ws.Hub.BroadcastToRoom 抽象出來、test 可注入 spy。 +// 採與 SystemHandler 相同 pattern（shutdownNotifyBroadcaster）、避免 handler +// 直接綁死 *ws.Hub。 +type firmwareBroadcaster interface { + BroadcastToRoom(room string, data interface{}) +} + +// firmwareService 是 firmware.Service 的 minimum surface、給 handler 用。 +// 抽介面是為了 unit test 不用啟 Python bridge / 真的 driver。 +type firmwareService interface { + UpgradeFirmware(ctx context.Context, deviceID, chip string) (string, <-chan firmware.FirmwareProgress, error) + CleanupTask(deviceID string) + HasActiveTask() bool + GetActiveTaskInfo() []*firmware.ActiveTaskInfo +} + +// FirmwareHandler 處理 firmware 相關 HTTP endpoint + WebSocket broadcast。 +type FirmwareHandler struct { + svc firmwareService + deviceMgr deviceLookupSource + wsHub firmwareBroadcaster + + // bundledVersions cache 從 firmware//VERSION 讀進來的版本字串、 + // 避免每次 ListDevices 都打 disk I/O（A 階段檔案不變）。 + bundledMu sync.RWMutex + bundledVersions map[string]string // chip → "v2.2.0" + firmwareDir string // server/scripts/firmware/ 絕對路徑（caller 注入） +} + +// deviceLookupSource 是 device manager 對 handler 的最小介面、test 可 mock。 +type deviceLookupSource interface { + GetDevice(id string) (*device.DeviceSession, error) +} + +// NewFirmwareHandler 建立 handler。firmwareDir 是 server/scripts/firmware/ 的 +// 絕對路徑（含 KL520/、KL720/ 子目錄）；空字串時 bundledFirmwareVersion +// 衍生欄位會回 "unknown"。 +func NewFirmwareHandler( + svc firmwareService, + deviceMgr deviceLookupSource, + wsHub firmwareBroadcaster, + firmwareDir string, +) *FirmwareHandler { + return &FirmwareHandler{ + svc: svc, + deviceMgr: deviceMgr, + wsHub: wsHub, + bundledVersions: make(map[string]string), + firmwareDir: firmwareDir, + } +} + +// ────────────────────────────────────────────────────────────────────── +// 1. POST /api/devices/:id/firmware/upgrade +// ────────────────────────────────────────────────────────────────────── + +// UpgradeDevice 啟動 firmware 升級流程。 +// +// Response： +// - 202 Accepted + {success:true, data:{taskId}}：goroutine 啟動、progress +// event 走 WebSocket room "firmware:" +// - 404：device 不存在 +// - 400：chip 不支援（A 階段 only KL520/KL720） +// - 409：device 已有 active firmware task / server shutting down +// - 500：service 拒絕（不可恢復錯誤） +// +// 注意：本 handler 不阻塞 HTTP 連線、立即回 202、實際升級在 goroutine 跑。 +// 客戶端應 subscribe WebSocket room 接收進度。 +func (h *FirmwareHandler) UpgradeDevice(c *gin.Context) { + id := c.Param("id") + + // 1. 找 device、取 chip + session, err := h.deviceMgr.GetDevice(id) + if err != nil { + c.JSON(http.StatusNotFound, gin.H{ + "success": false, + "error": gin.H{"code": "DEVICE_NOT_FOUND", "message": err.Error()}, + }) + return + } + info := session.Driver.Info() + chip := ChipFromDeviceType(info.Type) + if !firmware.SupportedUpgradeChip(chip) { + c.JSON(http.StatusBadRequest, gin.H{ + "success": false, + "error": gin.H{ + "code": "FW_UNSUPPORTED_CHIP", + "message": "A 階段僅支援 KL520/KL720（chip=" + chip + "）", + }, + }) + return + } + + // 2. 呼 service 啟動升級 + // + // 刻意用 context.Background() 而非 c.Request.Context()： + // + // - HTTP request ctx 在我們回 202 後會立即 cancel（gin 結束 handler + // 即釋放 connection），若沿用 request ctx 會把背景 goroutine 也 cancel + // 掉、升級流程立刻中斷、device brick 風險極高。 + // - Service 內部自己包 timeout（runUpgrade → UpgradeTimeoutFor + margin、 + // 見 service.go runUpgrade）、不會永久 hang。 + // - Graceful shutdown 透過 service.GracefulShutdown() 主動觸發、不靠 + // 外層 ctx cancel（見 service.go GracefulShutdown）。 + // + // 結論：Background 在這裡是刻意設計、非疏忽。 + taskID, progressCh, err := h.svc.UpgradeFirmware(context.Background(), id, chip) + if err != nil { + status, code := classifyServiceError(err) + c.JSON(status, gin.H{ + "success": false, + "error": gin.H{"code": code, "message": err.Error()}, + }) + return + } + + // 3. spawn forward goroutine：消費 progressCh、broadcast 到 WS room + go h.forwardProgressToWS(id, progressCh) + + c.JSON(http.StatusAccepted, gin.H{ + "success": true, + "data": gin.H{"taskId": taskID}, + }) +} + +// forwardProgressToWS 把 service 的 progress events 廣播到 WebSocket room +// "firmware:"、Frontend modal subscribe 該 room 接進度。 +// +// progressCh 由 service 在終態（done/error）後 close、本 goroutine 退出時 +// 呼叫 CleanupTask 移除 tracker entry。 +func (h *FirmwareHandler) forwardProgressToWS(deviceID string, progressCh <-chan firmware.FirmwareProgress) { + room := "firmware:" + deviceID + for ev := range progressCh { + // schema 對齊 TDD §4.2、外掛 type 標籤讓 client 統一 dispatch。 + // 也保留 deviceId / direction / stage / percent / elapsedMs / etaMs 等 + // FirmwareProgress 既有欄位（json marshal 直接展開）。 + h.wsHub.BroadcastToRoom(room, firmwareProgressMessage{ + Type: "firmware_progress", + FirmwareProgress: ev, + }) + } + // progressCh closed → service 已 markDone、可移除 tracker entry。 + h.svc.CleanupTask(deviceID) +} + +// firmwareProgressMessage 是 WebSocket payload wrapper、加 type 欄位讓 +// 前端可在同一 room 內 dispatch 不同訊息（雖然目前 firmware: room 只有 +// firmware_progress、保留結構彈性）。 +// +// 使用 embedded struct：FirmwareProgress 的所有 json field 會直接展開到 +// 同層、不會多一層巢狀。 +type firmwareProgressMessage struct { + Type string `json:"type"` + firmware.FirmwareProgress +} + +// ────────────────────────────────────────────────────────────────────── +// 2. GET /api/firmware/active-tasks +// ────────────────────────────────────────────────────────────────────── + +// ListActiveTasks 回傳所有進行中 firmware task 的 snapshot、給 Wails control +// panel 在 OnBeforeClose 偵測「是否有任務不可中斷」用（TDD §8.6.2）。 +// +// Response： +// +// { +// "success": true, +// "data": { +// "hasActive": true|false, +// "tasks": [ +// {"taskId":..., "deviceId":..., "deviceName":..., "chip":..., +// "direction":..., "stage":..., "elapsedMs":..., "etaSeconds":...} +// ] +// } +// } +func (h *FirmwareHandler) ListActiveTasks(c *gin.Context) { + tasks := h.svc.GetActiveTaskInfo() + // 用 nil → []：JSON encode 空 slice 為 [] 而非 null、Frontend 更好處理 + if tasks == nil { + tasks = []*firmware.ActiveTaskInfo{} + } + c.JSON(http.StatusOK, gin.H{ + "success": true, + "data": gin.H{ + "hasActive": len(tasks) > 0, + "tasks": tasks, + }, + }) +} + +// ────────────────────────────────────────────────────────────────────── +// 3. DeviceInfo 衍生欄位 helper（給 device_handler 用） +// ────────────────────────────────────────────────────────────────────── + +// FirmwareDerivedFields 是 device list / scan response 上的 firmware 衍生 +// 欄位（TDD §3.1 line 131）。 +// +// 由 device handler 在 ListDevices / ScanDevices 套用到每個 device entry +// 上、Frontend 用這些欄位決定是否顯示「升級韌體」按鈕。 +// +// Reviewer M9-3 第 1 輪 Major-1（JSON schema 雙鍵衝突修正）： +// 原本還有 `firmwareVer string` 欄位、但 device list response 的 +// driver.DeviceInfo 已內建 `firmwareVersion`（見 driver/interface.go line 26）、 +// 兩鍵指向同一個 firmware 字串會讓 frontend 混淆。改採顯式 3 欄位、frontend +// 直接讀 device entry 既有的 `firmwareVersion` 鍵。WS broadcast schema 不變 +// （已用 `beforeVersion` / `afterVersion`、與此處不衝突）。 +type FirmwareDerivedFields struct { + FirmwareIsLegacy bool `json:"firmwareIsLegacy"` + FirmwareCanUpgrade bool `json:"firmwareCanUpgrade"` + BundledFirmwareVersion string `json:"bundledFirmwareVersion"` +} + +// DeriveFirmwareFields 從 DeviceInfo 計算衍生欄位（不打 service、純字串 +// 判斷、適合塞 list endpoint 每筆 entry）。 +// +// 規則： +// - FirmwareIsLegacy：對齊 bridge.py `_fw_classify_legacy`（kneron_bridge.py +// line 1463）— 顯式列舉 KDP1 字串變體 + KDP2-KDP9 prefix 放行、避免 +// 對未來 firmware 誤判。 +// - FirmwareCanUpgrade：chip 在 A 階段支援清單（KL520/KL720）+ IsLegacy=true +// - BundledFirmwareVersion：讀 firmware//VERSION（cache） +// +// 注意：本 helper 不再回 firmwareVer 欄位、caller 直接使用 DeviceInfo 既有 +// `firmwareVersion` 鍵；參數 firmwareVer 只供 IsLegacy 判定用。 +func (h *FirmwareHandler) DeriveFirmwareFields(deviceType, firmwareVer string) FirmwareDerivedFields { + chip := ChipFromDeviceType(deviceType) + bundled := h.bundledVersionFor(chip) + isLegacy := isLegacyFirmware(firmwareVer) + canUpgrade := firmware.SupportedUpgradeChip(chip) && isLegacy + + return FirmwareDerivedFields{ + FirmwareIsLegacy: isLegacy, + FirmwareCanUpgrade: canUpgrade, + BundledFirmwareVersion: bundled, + } +} + +// bundledVersionFor 讀 firmware//VERSION、cache 結果。第一次 miss +// 時讀 disk、後續直接回 cache。讀檔失敗回 "unknown"（不 panic、不阻塞 +// device list）。 +// +// Reviewer M9-3 第 1 輪 Minor M-3：只 cache success。失敗（檔不存在 / 讀 +// 錯 / 空檔）不寫 cache、下次 list device 會重試。情境：CI 第一次 build +// 時 firmware 檔還沒 ready、若 cache 空字串將永遠回 "unknown"；改成不 cache +// 失敗、檔準備好後第一次 list 就能讀到新版本。 +func (h *FirmwareHandler) bundledVersionFor(chip string) string { + if h.firmwareDir == "" || chip == "" { + return "unknown" + } + h.bundledMu.RLock() + v, ok := h.bundledVersions[chip] + h.bundledMu.RUnlock() + if ok { + return v + } + + versionPath := filepath.Join(h.firmwareDir, chip, "VERSION") + f, err := os.Open(versionPath) + if err != nil { + // 沒 VERSION 檔（如 CI first build）→ 不 cache、下次重試 + return "unknown" + } + defer f.Close() + + buf, err := io.ReadAll(io.LimitReader(f, 64)) // VERSION 應只是一行版本字串 + if err != nil { + // 讀錯 → 不 cache、下次重試 + return "unknown" + } + parsed := strings.TrimSpace(string(buf)) + if parsed == "" { + // 空檔 → 不 cache、下次重試 + return "unknown" + } + // 只 cache 成功讀到的版本字串 + h.cacheBundledVersion(chip, parsed) + return parsed +} + +func (h *FirmwareHandler) cacheBundledVersion(chip, version string) { + h.bundledMu.Lock() + h.bundledVersions[chip] = version + h.bundledMu.Unlock() +} + +// ────────────────────────────────────────────────────────────────────── +// helpers（exported / unexported） +// ────────────────────────────────────────────────────────────────────── + +// ChipFromDeviceType 把 driver.DeviceInfo.Type 字串轉成 firmware chip +// 識別字串（"KL520" / "KL720"）。對應 detector.go:chipFromProductID +// 的反向轉換、未識別時回空字串。 +// +// 注意：刻意公開、device_handler 也可能直接用（雖然目前透過 +// DeriveFirmwareFields 走）。 +func ChipFromDeviceType(deviceType string) string { + low := strings.ToLower(deviceType) + switch { + case strings.Contains(low, "kl520"): + return firmware.ChipKL520 + case strings.Contains(low, "kl720"): + return firmware.ChipKL720 + case strings.Contains(low, "kl630"): + return "KL630" + case strings.Contains(low, "kl730"): + return "KL730" + default: + return "" + } +} + +// isLegacyFirmware 判斷 firmware 字串是否為 KDP1 legacy。 +// +// 對齊 bridge.py `_fw_classify_legacy`（kneron_bridge.py line 1463）的判定 +// 規則、與 Python 端保持一致行為（Reviewer M9-3 第 1 輪 S-1：跨端 drift 防護）。 +// +// 規則差異（與 bridge.py 對應）： +// +// 1. 已知 KDP1 字串顯式列舉（明示比對、不靠 substring）： +// "KDP", "KDP1", "USB BOOT", "USB BOOT LOADER", "LOADER", "BOOTLOADER" +// 2. KDP1.x 變體（"KDP1.0", "KDP1.5", "KDP1 alpha" 等）→ legacy +// 3. KDP2-KDP9 prefix 明示放行（forward-compat 未來 firmware、避免 substring +// match 對 "KDP3" 之類字串誤判 legacy） +// 4. 未知 firmware 字串（如 "NEF" / "K3"）→ 保守 default = 不 legacy +// （避免誤觸 loader stage brick device；若實際為 legacy、verify 階段 +// 會 detect verify_mismatch、不致 brick） +// +// 注意：Go 端不接收 product_id（DeviceInfo.ProductID 雖有、但 ListDevices +// response 是 driver.DeviceInfo embedded、product_id 對應 KL720 KDP legacy +// 判定走 chip type 即可、不需要在這裡覆蓋）。bridge.py 端的 product_id == +// 0x0200 短路在 connect / upgrade 流程內部處理、不影響 list endpoint +// canUpgrade 判定。 +func isLegacyFirmware(firmwareVer string) bool { + fw := strings.ToUpper(strings.TrimSpace(firmwareVer)) + + // 1. 已知 KDP1 legacy firmware 字串完整列舉 + // 注意：bridge.py 把 "" 視為 legacy（USB Boot state 不回 firmware + // string）；但 Go 端 list endpoint 看到空 firmware 字串通常是 device + // 還沒 connect 過 — 保守不視為 legacy（不顯示升級按鈕），與 bridge.py + // 的 connect 流程語意分離（connect 流程進入 loader 是另一條路徑）。 + if fw == "" { + return false + } + switch fw { + case "KDP", "KDP1", "USB BOOT", "USB BOOT LOADER", "LOADER", "BOOTLOADER": + return true + } + + // 2. KDP1.x（KDP1.0 / KDP1.5 等） + if strings.HasPrefix(fw, "KDP1.") || strings.HasPrefix(fw, "KDP1 ") { + return true + } + + // 3. 明示放行 KDP2 / KDP3+（forward-compat、避免 substring match 對未來 + // firmware 誤判） + for _, prefix := range []string{"KDP2", "KDP3", "KDP4", "KDP5", "KDP6", "KDP7", "KDP8", "KDP9"} { + if strings.HasPrefix(fw, prefix) { + return false + } + } + + // 4. 未知 firmware 字串 → 保守不 legacy（與 bridge.py 一致） + return false +} + +// classifyServiceError 把 firmware.Service 的 error 對應到 HTTP status 和 +// 錯誤碼（TDD §3.3）。 +// +// Reviewer M9-3 第 1 輪 S-3：unknown error 路徑加 log，方便除錯非預期錯誤 +// 類型——未來 service 新增 sentinel error 但忘了在這裡掛上時、log 能即時 +// 揭露問題。 +func classifyServiceError(err error) (int, string) { + switch { + case errors.Is(err, firmware.ErrDeviceNotFound): + return http.StatusNotFound, "DEVICE_NOT_FOUND" + case errors.Is(err, firmware.ErrUnsupportedChip): + return http.StatusBadRequest, "FW_UNSUPPORTED_CHIP" + case errors.Is(err, firmware.ErrDeviceBusy): + return http.StatusConflict, "FW_DEVICE_BUSY" + case errors.Is(err, firmware.ErrUpgradeBrickRisk): + return http.StatusInternalServerError, "FW_UPGRADE_BRICK_RISK" + case errors.Is(err, firmware.ErrUpgradeFailed): + return http.StatusInternalServerError, "FW_UPGRADE_FAILED" + default: + log.Printf("[firmware_handler] classifyServiceError: unknown error type %T: %v", err, err) + return http.StatusInternalServerError, "FW_UPGRADE_FAILED" + } +} + +// ────────────────────────────────────────────────────────────────────── +// device.Manager → firmware.DeviceLookup adapter +// ────────────────────────────────────────────────────────────────────── + +// DeviceManagerAdapter 把 *device.Manager 包成 firmware.DeviceLookup、 +// 讓 main.go 在 NewService 時可以直接傳給 firmware 模組、不破壞 device.Manager +// 既有 API。 +// +// 公開出來、供 main.go 在 wire 階段使用。 +type DeviceManagerAdapter struct { + mgr *device.Manager +} + +// NewDeviceManagerAdapter 建立 adapter。 +func NewDeviceManagerAdapter(mgr *device.Manager) *DeviceManagerAdapter { + return &DeviceManagerAdapter{mgr: mgr} +} + +// GetUpgradeDriver 實作 firmware.DeviceLookup。 +// +// 把 device session 內的 driver（*kneron.KneronDriver）轉成 +// firmware.UpgradeDriver 介面。KneronDriver 已內建 Info() + +// UpgradeFirmware(ctx, chip, progressCh) 兩個 method、直接 type-assert。 +func (a *DeviceManagerAdapter) GetUpgradeDriver(deviceID string) (firmware.UpgradeDriver, error) { + session, err := a.mgr.GetDevice(deviceID) + if err != nil { + return nil, err + } + drv, ok := session.Driver.(firmware.UpgradeDriver) + if !ok { + return nil, errors.New("driver does not implement firmware.UpgradeDriver") + } + return drv, nil +} diff --git a/local-tool/server/internal/api/handlers/firmware_handler_test.go b/local-tool/server/internal/api/handlers/firmware_handler_test.go new file mode 100644 index 0000000..fa76487 --- /dev/null +++ b/local-tool/server/internal/api/handlers/firmware_handler_test.go @@ -0,0 +1,938 @@ +package handlers + +// firmware_handler_test.go — M9-3 unit test +// +// 覆蓋： +// 1. POST /api/devices/:id/firmware/upgrade +// - 成功路徑（202 + taskID + progress 透過 WS broadcast） +// - device not found（404） +// - chip 不支援（400 / KL630 / KL730） +// - service 拒絕：busy（409）/ unsupported chip（400）/ brick risk（500）/ generic fail（500） +// 2. GET /api/firmware/active-tasks +// - 空清單（hasActive=false、tasks=[]） +// - 有 task（hasActive=true、tasks 完整欄位） +// 3. DeriveFirmwareFields +// - KDP1 legacy → IsLegacy=true / CanUpgrade=true +// - KDP2 modern → IsLegacy=false / CanUpgrade=false +// - 空 firmware → 保守不 legacy +// - chip 不支援（KL630）→ CanUpgrade=false 即使 legacy +// - 讀 VERSION 檔成功 / 失敗 fallback "unknown" +// 4. WebSocket broadcast schema：{type:"firmware_progress", deviceId, stage, percent, ...} + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "visiona-local/server/internal/device" + "visiona-local/server/internal/driver" + "visiona-local/server/internal/firmware" + + "github.com/gin-gonic/gin" +) + +// ────────────────────────────────────────────────────────────────────── +// Mocks +// ────────────────────────────────────────────────────────────────────── + +// fakeFirmwareService 是 firmwareService interface 的 test stub。 +type fakeFirmwareService struct { + mu sync.Mutex + + // programmed behaviour + upgradeErr error + upgradeTaskID string + upgradeProgress []firmware.FirmwareProgress + + activeTasks []*firmware.ActiveTaskInfo + + // observed + upgradeCalls []upgradeCall + cleanupCalls []string + hasActiveCalls int +} + +type upgradeCall struct { + deviceID string + chip string +} + +func (f *fakeFirmwareService) UpgradeFirmware(_ context.Context, deviceID, chip string) (string, <-chan firmware.FirmwareProgress, error) { + f.mu.Lock() + f.upgradeCalls = append(f.upgradeCalls, upgradeCall{deviceID: deviceID, chip: chip}) + err := f.upgradeErr + taskID := f.upgradeTaskID + progress := f.upgradeProgress + f.mu.Unlock() + + if err != nil { + return "", nil, err + } + ch := make(chan firmware.FirmwareProgress, len(progress)+1) + for _, ev := range progress { + ch <- ev + } + close(ch) + return taskID, ch, nil +} + +func (f *fakeFirmwareService) CleanupTask(deviceID string) { + f.mu.Lock() + f.cleanupCalls = append(f.cleanupCalls, deviceID) + f.mu.Unlock() +} + +func (f *fakeFirmwareService) HasActiveTask() bool { + f.mu.Lock() + defer f.mu.Unlock() + f.hasActiveCalls++ + return len(f.activeTasks) > 0 +} + +func (f *fakeFirmwareService) GetActiveTaskInfo() []*firmware.ActiveTaskInfo { + f.mu.Lock() + defer f.mu.Unlock() + return f.activeTasks +} + +// fakeDeviceLookup 模擬 deviceLookupSource。 +type fakeDeviceLookup struct { + sessions map[string]*device.DeviceSession +} + +func (f *fakeDeviceLookup) GetDevice(id string) (*device.DeviceSession, error) { + s, ok := f.sessions[id] + if !ok { + return nil, errors.New("device not found: " + id) + } + return s, nil +} + +// fakeDriver 是 driver.DeviceDriver 的 minimal stub、只填 Info()。 +type fakeDriver struct { + info driver.DeviceInfo +} + +func (f *fakeDriver) Info() driver.DeviceInfo { return f.info } +func (f *fakeDriver) Connect() error { return nil } +func (f *fakeDriver) Disconnect() error { return nil } +func (f *fakeDriver) IsConnected() bool { return false } +func (f *fakeDriver) Flash(_ string, _ chan<- driver.FlashProgress) error { return nil } +func (f *fakeDriver) StartInference() error { return nil } +func (f *fakeDriver) StopInference() error { return nil } +func (f *fakeDriver) ReadInference() (*driver.InferenceResult, error) { + return nil, nil +} +func (f *fakeDriver) RunInference(_ []byte) (*driver.InferenceResult, error) { + return nil, nil +} +func (f *fakeDriver) GetModelInfo() (*driver.ModelInfo, error) { return nil, nil } + +// spyBroadcasterFW 抓所有 BroadcastToRoom 呼叫、供斷言。 +type spyBroadcasterFW struct { + mu sync.Mutex + calls []spyBroadcastCall +} + +type spyBroadcastCall struct { + room string + data interface{} +} + +func (s *spyBroadcasterFW) BroadcastToRoom(room string, data interface{}) { + s.mu.Lock() + defer s.mu.Unlock() + s.calls = append(s.calls, spyBroadcastCall{room: room, data: data}) +} + +func (s *spyBroadcasterFW) snapshot() []spyBroadcastCall { + s.mu.Lock() + defer s.mu.Unlock() + out := make([]spyBroadcastCall, len(s.calls)) + copy(out, s.calls) + return out +} + +// ────────────────────────────────────────────────────────────────────── +// Helpers +// ────────────────────────────────────────────────────────────────────── + +func newTestFirmwareHandler(t *testing.T) (*FirmwareHandler, *fakeFirmwareService, *fakeDeviceLookup, *spyBroadcasterFW, string) { + t.Helper() + gin.SetMode(gin.TestMode) + svc := &fakeFirmwareService{} + lookup := &fakeDeviceLookup{sessions: make(map[string]*device.DeviceSession)} + hub := &spyBroadcasterFW{} + tmpDir := t.TempDir() + h := NewFirmwareHandler(svc, lookup, hub, tmpDir) + return h, svc, lookup, hub, tmpDir +} + +func addFakeDevice(lookup *fakeDeviceLookup, id, devType, fwVer string) { + lookup.sessions[id] = device.NewSession(&fakeDriver{ + info: driver.DeviceInfo{ + ID: id, + Name: "fake-" + id, + Type: devType, + FirmwareVer: fwVer, + }, + }) +} + +func writeVersionFile(t *testing.T, root, chip, version string) { + t.Helper() + chipDir := filepath.Join(root, chip) + if err := os.MkdirAll(chipDir, 0o755); err != nil { + t.Fatalf("mkdir %s: %v", chipDir, err) + } + if err := os.WriteFile(filepath.Join(chipDir, "VERSION"), []byte(version+"\n"), 0o644); err != nil { + t.Fatalf("write VERSION: %v", err) + } +} + +// performUpgradeRequest 觸發 POST /api/devices/:id/firmware/upgrade、回 ResponseRecorder。 +func performUpgradeRequest(h *FirmwareHandler, deviceID string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: deviceID}} + c.Request = httptest.NewRequest(http.MethodPost, "/api/devices/"+deviceID+"/firmware/upgrade", nil) + h.UpgradeDevice(c) + return w +} + +func performListActiveTasksRequest(h *FirmwareHandler) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest(http.MethodGet, "/api/firmware/active-tasks", nil) + h.ListActiveTasks(c) + return w +} + +// ────────────────────────────────────────────────────────────────────── +// 1. POST /api/devices/:id/firmware/upgrade +// ────────────────────────────────────────────────────────────────────── + +func TestUpgradeDevice_Success(t *testing.T) { + h, svc, lookup, hub, _ := newTestFirmwareHandler(t) + addFakeDevice(lookup, "dev-1", "kneron_kl520", "KDP") + + svc.upgradeTaskID = "upgrade-dev-1-20260525" + svc.upgradeProgress = []firmware.FirmwareProgress{ + {DeviceID: "dev-1", Stage: firmware.StagePreparing, Percent: 5, Direction: firmware.DirectionUpgrade}, + {DeviceID: "dev-1", Stage: firmware.StageLoading, Percent: 20, Direction: firmware.DirectionUpgrade}, + {DeviceID: "dev-1", Stage: firmware.StageDone, Percent: 100, AfterVersion: "v2.2.0", Direction: firmware.DirectionUpgrade}, + } + + w := performUpgradeRequest(h, "dev-1") + + if w.Code != http.StatusAccepted { + t.Fatalf("status = %d, want 202; body=%s", w.Code, w.Body.String()) + } + + var resp struct { + Success bool `json:"success"` + Data map[string]interface{} `json:"data"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if !resp.Success { + t.Errorf("success = false, want true") + } + if got := resp.Data["taskId"]; got != "upgrade-dev-1-20260525" { + t.Errorf("taskId = %v, want upgrade-dev-1-20260525", got) + } + + // service 收到正確 chip + if len(svc.upgradeCalls) != 1 || svc.upgradeCalls[0].chip != firmware.ChipKL520 { + t.Errorf("svc.upgradeCalls = %+v, want chip=KL520", svc.upgradeCalls) + } + + // 等 broadcast goroutine 跑完 + waitForBroadcasts(t, hub, 3, 2*time.Second) + waitForCleanup(t, svc, "dev-1", 2*time.Second) + + calls := hub.snapshot() + if len(calls) != 3 { + t.Fatalf("broadcast calls = %d, want 3; %+v", len(calls), calls) + } + for i, c := range calls { + if c.room != "firmware:dev-1" { + t.Errorf("call[%d].room = %q, want firmware:dev-1", i, c.room) + } + // JSON round-trip 驗證 schema + raw, _ := json.Marshal(c.data) + var m map[string]interface{} + if err := json.Unmarshal(raw, &m); err != nil { + t.Errorf("call[%d] unmarshal: %v", i, err) + continue + } + if m["type"] != "firmware_progress" { + t.Errorf("call[%d].type = %v, want firmware_progress", i, m["type"]) + } + if m["deviceId"] != "dev-1" { + t.Errorf("call[%d].deviceId = %v, want dev-1", i, m["deviceId"]) + } + if _, ok := m["stage"]; !ok { + t.Errorf("call[%d] missing stage", i) + } + if _, ok := m["percent"]; !ok { + t.Errorf("call[%d] missing percent", i) + } + } + + // 最終 done event：包含 afterVersion + doneRaw, _ := json.Marshal(calls[2].data) + var doneMsg map[string]interface{} + _ = json.Unmarshal(doneRaw, &doneMsg) + if doneMsg["afterVersion"] != "v2.2.0" { + t.Errorf("done.afterVersion = %v, want v2.2.0", doneMsg["afterVersion"]) + } +} + +func TestUpgradeDevice_DeviceNotFound(t *testing.T) { + h, _, _, _, _ := newTestFirmwareHandler(t) + w := performUpgradeRequest(h, "nope") + if w.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404; body=%s", w.Code, w.Body.String()) + } + // Reviewer M9-3 第 1 輪 Minor M-4：用 response JSON 結構 + 錯誤碼斷言、 + // 不檢查 error message string（避免 device manager error wrapping 改變 + // 時 test 不必要地破裂）。 + var resp struct { + Success bool `json:"success"` + Error struct { + Code string `json:"code"` + Message string `json:"message"` + } `json:"error"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if resp.Success { + t.Errorf("success = true, want false") + } + if resp.Error.Code != "DEVICE_NOT_FOUND" { + t.Errorf("error.code = %q, want DEVICE_NOT_FOUND", resp.Error.Code) + } +} + +func TestUpgradeDevice_UnsupportedChip(t *testing.T) { + cases := []struct { + name string + devType string + }{ + {"KL630", "kneron_kl630"}, + {"KL730", "kneron_kl730"}, + {"unknown", "kneron_unknown"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h, svc, lookup, _, _ := newTestFirmwareHandler(t) + addFakeDevice(lookup, "dev-x", tc.devType, "KDP") + + w := performUpgradeRequest(h, "dev-x") + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400; body=%s", w.Code, w.Body.String()) + } + // Reviewer M9-3 第 1 輪 Minor M-4：透過 JSON 結構斷言錯誤碼。 + var resp struct { + Error struct { + Code string `json:"code"` + } `json:"error"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if resp.Error.Code != "FW_UNSUPPORTED_CHIP" { + t.Errorf("error.code = %q, want FW_UNSUPPORTED_CHIP", resp.Error.Code) + } + // service 不應該被呼叫 + if len(svc.upgradeCalls) != 0 { + t.Errorf("svc.upgradeCalls = %d, want 0 (handler should pre-filter)", len(svc.upgradeCalls)) + } + }) + } +} + +func TestUpgradeDevice_ServiceErrors(t *testing.T) { + cases := []struct { + name string + svcErr error + wantStatus int + wantCode string + }{ + {"busy", firmware.ErrDeviceBusy, http.StatusConflict, "FW_DEVICE_BUSY"}, + {"unsupported_at_service", firmware.ErrUnsupportedChip, http.StatusBadRequest, "FW_UNSUPPORTED_CHIP"}, + {"brick_risk", firmware.ErrUpgradeBrickRisk, http.StatusInternalServerError, "FW_UPGRADE_BRICK_RISK"}, + {"generic_fail", firmware.ErrUpgradeFailed, http.StatusInternalServerError, "FW_UPGRADE_FAILED"}, + {"unknown", errors.New("boom"), http.StatusInternalServerError, "FW_UPGRADE_FAILED"}, + {"not_found_at_service", firmware.ErrDeviceNotFound, http.StatusNotFound, "DEVICE_NOT_FOUND"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h, svc, lookup, _, _ := newTestFirmwareHandler(t) + addFakeDevice(lookup, "dev-1", "kneron_kl520", "KDP") + svc.upgradeErr = tc.svcErr + + w := performUpgradeRequest(h, "dev-1") + if w.Code != tc.wantStatus { + t.Fatalf("status = %d, want %d; body=%s", w.Code, tc.wantStatus, w.Body.String()) + } + // Reviewer M9-3 第 1 輪 Minor M-4：JSON 結構斷言錯誤碼。 + var resp struct { + Error struct { + Code string `json:"code"` + } `json:"error"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if resp.Error.Code != tc.wantCode { + t.Errorf("error.code = %q, want %q", resp.Error.Code, tc.wantCode) + } + }) + } +} + +// ────────────────────────────────────────────────────────────────────── +// 2. GET /api/firmware/active-tasks +// ────────────────────────────────────────────────────────────────────── + +func TestListActiveTasks_Empty(t *testing.T) { + h, _, _, _, _ := newTestFirmwareHandler(t) + w := performListActiveTasksRequest(h) + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", w.Code, w.Body.String()) + } + var resp struct { + Success bool `json:"success"` + Data struct { + HasActive bool `json:"hasActive"` + Tasks []firmware.ActiveTaskInfo `json:"tasks"` + } `json:"data"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if !resp.Success { + t.Errorf("success = false") + } + if resp.Data.HasActive { + t.Errorf("hasActive = true, want false") + } + if len(resp.Data.Tasks) != 0 { + t.Errorf("tasks = %d, want 0", len(resp.Data.Tasks)) + } + // tasks 不應為 null（前端較難處理） + if !strings.Contains(w.Body.String(), `"tasks":[]`) { + t.Errorf("tasks 應為 [] 而非 null: %s", w.Body.String()) + } +} + +func TestListActiveTasks_WithTasks(t *testing.T) { + h, svc, _, _, _ := newTestFirmwareHandler(t) + now := time.Now() + svc.activeTasks = []*firmware.ActiveTaskInfo{ + { + TaskID: "upgrade-dev-1", + DeviceID: "dev-1", + DeviceName: "KL520 #1", + Chip: firmware.ChipKL520, + Direction: firmware.DirectionUpgrade, + Stage: firmware.StageFlashing, + StartTs: now, + ElapsedMs: 5000, + EtaSeconds: 30, + }, + } + w := performListActiveTasksRequest(h) + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", w.Code, w.Body.String()) + } + var resp struct { + Success bool `json:"success"` + Data struct { + HasActive bool `json:"hasActive"` + Tasks []firmware.ActiveTaskInfo `json:"tasks"` + } `json:"data"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if !resp.Data.HasActive { + t.Errorf("hasActive = false, want true") + } + if len(resp.Data.Tasks) != 1 { + t.Fatalf("tasks = %d, want 1", len(resp.Data.Tasks)) + } + got := resp.Data.Tasks[0] + if got.DeviceID != "dev-1" { + t.Errorf("task.deviceId = %q, want dev-1", got.DeviceID) + } + if got.Stage != firmware.StageFlashing { + t.Errorf("task.stage = %q, want flashing", got.Stage) + } + if got.EtaSeconds != 30 { + t.Errorf("task.etaSeconds = %d, want 30", got.EtaSeconds) + } +} + +// ────────────────────────────────────────────────────────────────────── +// 3. DeriveFirmwareFields +// ────────────────────────────────────────────────────────────────────── + +func TestDeriveFirmwareFields(t *testing.T) { + h, _, _, _, tmpDir := newTestFirmwareHandler(t) + writeVersionFile(t, tmpDir, "KL520", "v2.2.0") + writeVersionFile(t, tmpDir, "KL720", "v2.2.0") + // 注意：不寫 KL630 VERSION → 測試 fallback + + cases := []struct { + name string + devType string + fwVer string + wantLegacy bool + wantCanUpgrade bool + wantBundled string + }{ + { + name: "KL520 KDP1 legacy → can upgrade", + devType: "kneron_kl520", + fwVer: "KDP", + wantLegacy: true, + wantCanUpgrade: true, + wantBundled: "v2.2.0", + }, + { + name: "KL520 KDP1.0 legacy 字串變體 → can upgrade", + devType: "kneron_kl520", + fwVer: "KDP1.0", + wantLegacy: true, + wantCanUpgrade: true, + wantBundled: "v2.2.0", + }, + { + name: "KL520 KDP2 modern → cannot upgrade", + devType: "kneron_kl520", + fwVer: "KDP2-v2.2.0", + wantLegacy: false, + wantCanUpgrade: false, + wantBundled: "v2.2.0", + }, + { + name: "KL520 已是 v2.2.0 → cannot upgrade", + devType: "kneron_kl520", + fwVer: "v2.2.0", + wantLegacy: false, + wantCanUpgrade: false, + wantBundled: "v2.2.0", + }, + { + name: "KL520 firmware 空字串 → 保守不 legacy", + devType: "kneron_kl520", + fwVer: "", + wantLegacy: false, + wantCanUpgrade: false, + wantBundled: "v2.2.0", + }, + { + name: "KL720 KDP1 legacy → can upgrade", + devType: "kneron_kl720", + fwVer: "KDP", + wantLegacy: true, + wantCanUpgrade: true, + wantBundled: "v2.2.0", + }, + { + name: "KL630 即使 KDP1 也 cannot upgrade（A 階段不支援）", + devType: "kneron_kl630", + fwVer: "KDP", + wantLegacy: true, + wantCanUpgrade: false, + wantBundled: "unknown", // 無 VERSION 檔 + }, + { + name: "未知 chip type → 保守不可升", + devType: "kneron_unknown", + fwVer: "KDP", + wantLegacy: true, + wantCanUpgrade: false, + wantBundled: "unknown", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := h.DeriveFirmwareFields(tc.devType, tc.fwVer) + // Reviewer M9-3 第 1 輪 Major-1：FirmwareDerivedFields 不再含 + // firmwareVer 鍵；frontend 直接讀 DeviceInfo.firmwareVersion。 + // 此處只斷言 3 個衍生欄位。 + if got.FirmwareIsLegacy != tc.wantLegacy { + t.Errorf("IsLegacy = %v, want %v", got.FirmwareIsLegacy, tc.wantLegacy) + } + if got.FirmwareCanUpgrade != tc.wantCanUpgrade { + t.Errorf("CanUpgrade = %v, want %v", got.FirmwareCanUpgrade, tc.wantCanUpgrade) + } + if got.BundledFirmwareVersion != tc.wantBundled { + t.Errorf("BundledFirmwareVersion = %q, want %q", got.BundledFirmwareVersion, tc.wantBundled) + } + }) + } +} + +func TestDeriveFirmwareFields_EmptyFirmwareDir(t *testing.T) { + // firmwareDir = "" → bundled 永遠回 "unknown" + gin.SetMode(gin.TestMode) + svc := &fakeFirmwareService{} + lookup := &fakeDeviceLookup{sessions: make(map[string]*device.DeviceSession)} + hub := &spyBroadcasterFW{} + h := NewFirmwareHandler(svc, lookup, hub, "") + + got := h.DeriveFirmwareFields("kneron_kl520", "KDP") + if got.BundledFirmwareVersion != "unknown" { + t.Errorf("BundledFirmwareVersion = %q, want unknown when firmwareDir is empty", got.BundledFirmwareVersion) + } + if !got.FirmwareIsLegacy { + t.Errorf("IsLegacy = false, want true (KDP)") + } + if !got.FirmwareCanUpgrade { + t.Errorf("CanUpgrade = false, want true (chip+legacy)") + } + // Major-1：FirmwareDerivedFields JSON 不應含 firmwareVer 鍵 + raw, _ := json.Marshal(got) + if strings.Contains(string(raw), `"firmwareVer"`) { + t.Errorf("FirmwareDerivedFields JSON 不應含 firmwareVer 鍵；got: %s", string(raw)) + } +} + +// TestDeriveFirmwareFields_NoFirmwareVerKey 驗證 Reviewer M9-3 第 1 輪 +// Major-1 修正：FirmwareDerivedFields JSON 不再含 firmwareVer 鍵（避免與 +// driver.DeviceInfo 的 firmwareVersion 鍵衝突）。 +func TestDeriveFirmwareFields_NoFirmwareVerKey(t *testing.T) { + h, _, _, _, tmpDir := newTestFirmwareHandler(t) + writeVersionFile(t, tmpDir, "KL520", "v2.2.0") + + got := h.DeriveFirmwareFields("kneron_kl520", "KDP") + raw, err := json.Marshal(got) + if err != nil { + t.Fatalf("marshal: %v", err) + } + body := string(raw) + + // 不應該再含 firmwareVer 鍵 + if strings.Contains(body, `"firmwareVer"`) { + t.Errorf("FirmwareDerivedFields JSON 不應含 firmwareVer 鍵（避免與 DeviceInfo.firmwareVersion 衝突）；got: %s", body) + } + // 但應含 3 個衍生欄位 + for _, key := range []string{`"firmwareIsLegacy"`, `"firmwareCanUpgrade"`, `"bundledFirmwareVersion"`} { + if !strings.Contains(body, key) { + t.Errorf("FirmwareDerivedFields JSON 缺少 %s; got: %s", key, body) + } + } +} + +// TestEnrichDevicesJSONOutput 驗證 device list endpoint 的 JSON schema： +// 用 enrichDevices 模擬 ListDevices / ScanDevices 的內部組裝（不啟整個 gin +// router），grep response JSON 確認： +// - 有 "firmwareVersion" 鍵（DeviceInfo embedded、frontend SoT） +// - 沒 "firmwareVer" 鍵（避免雙鍵衝突） +// - 有 firmwareIsLegacy / firmwareCanUpgrade / bundledFirmwareVersion 衍生欄位 +// +// Reviewer M9-3 第 1 輪 Major-1：必須補這個 schema 測試。 +func TestEnrichDevicesJSONOutput(t *testing.T) { + fwh, _, _, _, tmpDir := newTestFirmwareHandler(t) + writeVersionFile(t, tmpDir, "KL520", "v2.2.0") + + dh := NewDeviceHandler(nil, nil, nil, nil) + dh.SetFirmwareHandler(fwh) + + devices := []driver.DeviceInfo{ + { + ID: "dev-1", + Name: "KL520 #1", + Type: "kneron_kl520", + FirmwareVer: "KDP", // legacy + Status: driver.StatusDetected, + }, + } + enriched := dh.enrichDevices(devices) + if len(enriched) != 1 { + t.Fatalf("enrichDevices returned %d entries, want 1", len(enriched)) + } + + raw, err := json.Marshal(enriched[0]) + if err != nil { + t.Fatalf("marshal: %v", err) + } + body := string(raw) + + // 1. 必須有 firmwareVersion 鍵（來自 DeviceInfo.FirmwareVer json tag） + if !strings.Contains(body, `"firmwareVersion":"KDP"`) { + t.Errorf("缺少 firmwareVersion 鍵; got: %s", body) + } + // 2. 絕對不應有 firmwareVer 鍵（避免雙鍵衝突） + if strings.Contains(body, `"firmwareVer"`) { + t.Errorf("device JSON 不應含 firmwareVer 鍵（與 firmwareVersion 衝突）; got: %s", body) + } + // 3. 衍生欄位齊全 + for _, key := range []string{`"firmwareIsLegacy"`, `"firmwareCanUpgrade"`, `"bundledFirmwareVersion"`} { + if !strings.Contains(body, key) { + t.Errorf("device JSON 缺少 %s; got: %s", key, body) + } + } + // 4. 既有 DeviceInfo 欄位也應該還在（不破壞既有 frontend client） + for _, key := range []string{`"id":"dev-1"`, `"name":"KL520 #1"`, `"type":"kneron_kl520"`, `"status":"detected"`} { + if !strings.Contains(body, key) { + t.Errorf("device JSON 缺少 DeviceInfo 既有欄位 %s; got: %s", key, body) + } + } +} + +// TestEnrichDevicesJSONOutput_NilFwHandler 驗證 fallback 路徑：fwHandler=nil +// 時、JSON 也不應含 firmwareVer 鍵、firmwareVersion 應從 DeviceInfo 帶上。 +func TestEnrichDevicesJSONOutput_NilFwHandler(t *testing.T) { + dh := NewDeviceHandler(nil, nil, nil, nil) + // 刻意不 SetFirmwareHandler → fwHandler 為 nil + + devices := []driver.DeviceInfo{ + { + ID: "dev-1", + Type: "kneron_kl520", + FirmwareVer: "KDP", + Status: driver.StatusDetected, + }, + } + enriched := dh.enrichDevices(devices) + raw, _ := json.Marshal(enriched[0]) + body := string(raw) + + if !strings.Contains(body, `"firmwareVersion":"KDP"`) { + t.Errorf("nil fwHandler 時 firmwareVersion 應從 DeviceInfo 帶上; got: %s", body) + } + if strings.Contains(body, `"firmwareVer"`) { + t.Errorf("nil fwHandler 時不應含 firmwareVer 鍵; got: %s", body) + } + if !strings.Contains(body, `"bundledFirmwareVersion":"unknown"`) { + t.Errorf("nil fwHandler 時 bundled 應為 unknown; got: %s", body) + } +} + +// TestIsLegacyFirmware_BridgeParity 驗證 Go isLegacyFirmware 與 bridge.py +// _fw_classify_legacy 對同樣 firmware 字串給出一致結果（Reviewer M9-3 +// 第 1 輪 S-1：跨端 drift 防護）。 +// +// 真值表來源：kneron_bridge.py line 1463-1508 `_fw_classify_legacy`。 +// 注意：bridge.py 把空字串 ""（USB Boot state 不回 firmware）視為 legacy、 +// 但 Go list endpoint 看到空字串通常是 device 還沒 connect 過 — 保守當 +// 非 legacy（不顯示升級按鈕）；此差異已在 isLegacyFirmware godoc 註記。 +func TestIsLegacyFirmware_BridgeParity(t *testing.T) { + // product_id=0x0200 那條規則 Go 端不檢查（KL720 KDP legacy 由 chip + // type + upgrade 流程處理、不在 list endpoint canUpgrade 邏輯內）。 + cases := []struct { + fw string + wantLegacy bool + note string + }{ + // 已知 KDP1 legacy 字串 + {"KDP", true, "bridge.py legacy_exact"}, + {"KDP1", true, "bridge.py legacy_exact"}, + {"USB BOOT", true, "bridge.py legacy_exact"}, + {"USB Boot", true, "case insensitive"}, + {"USB BOOT LOADER", true, "bridge.py legacy_exact"}, + {"LOADER", true, "bridge.py legacy_exact"}, + {"BOOTLOADER", true, "bridge.py legacy_exact"}, + // KDP1.x 變體 + {"KDP1.0", true, "bridge.py KDP1. prefix"}, + {"KDP1.5", true, "bridge.py KDP1. prefix"}, + {"KDP1 alpha", true, "bridge.py KDP1 prefix"}, + // KDP2-KDP9 明示放行 + {"KDP2", false, "bridge.py KDP2 prefix → modern"}, + {"KDP2.0", false, "bridge.py KDP2 prefix"}, + {"KDP2-v2.2.0", false, "bridge.py KDP2 prefix"}, + {"KDP3", false, "bridge.py KDP3 prefix → forward-compat"}, + {"KDP3.1", false, "bridge.py KDP3 prefix → forward-compat"}, + {"KDP9", false, "bridge.py KDP9 prefix → forward-compat"}, + // 未知字串 → 保守不 legacy（bridge.py default） + {"NEF", false, "bridge.py default for unknown"}, + {"K3", false, "bridge.py default for unknown"}, + {"v2.2.0", false, "version string only → not legacy"}, + // Go 端對空字串的特例（保守、與 bridge.py 在 list endpoint 場景一致） + {"", false, "Go list endpoint conservative (not connected yet)"}, + } + for _, tc := range cases { + t.Run(tc.fw+"_"+tc.note, func(t *testing.T) { + got := isLegacyFirmware(tc.fw) + if got != tc.wantLegacy { + t.Errorf("isLegacyFirmware(%q) = %v, want %v (%s)", + tc.fw, got, tc.wantLegacy, tc.note) + } + }) + } +} + +// TestBundledVersionFor_CacheMissDoesNotPoisonRetry 驗證 Reviewer M9-3 +// 第 1 輪 Minor M-3：cache miss 後不寫入 cache、下次 list device 會重試 +// 並讀到新版本（情境：CI first build 時 firmware 檔還沒 ready）。 +func TestBundledVersionFor_CacheMissDoesNotPoisonRetry(t *testing.T) { + h, _, _, _, tmpDir := newTestFirmwareHandler(t) + + // 1. VERSION 檔還沒 ready → 應回 "unknown" + got1 := h.bundledVersionFor("KL520") + if got1 != "unknown" { + t.Fatalf("first call (no VERSION file) = %q, want unknown", got1) + } + + // 2. 確認沒有 cache 失敗結果（否則下次重試會永遠回 unknown） + h.bundledMu.RLock() + _, cached := h.bundledVersions["KL520"] + h.bundledMu.RUnlock() + if cached { + t.Errorf("失敗結果不應寫入 cache（M-3）；cache 含 KL520 entry") + } + + // 3. 模擬 CI 後續產出 VERSION 檔 + writeVersionFile(t, tmpDir, "KL520", "v2.2.0") + + // 4. 第二次呼叫應該讀到實際版本（重試成功） + got2 := h.bundledVersionFor("KL520") + if got2 != "v2.2.0" { + t.Errorf("retry after VERSION ready = %q, want v2.2.0", got2) + } + + // 5. 第三次應從 cache 命中（success 才 cache） + h.bundledMu.RLock() + cachedVal := h.bundledVersions["KL520"] + h.bundledMu.RUnlock() + if cachedVal != "v2.2.0" { + t.Errorf("success 應 cache；cache value = %q", cachedVal) + } +} + +// TestBundledVersionFor_EmptyFileNotCached 驗證空 VERSION 檔（不合理狀態） +// 也不應該污染 cache。 +func TestBundledVersionFor_EmptyFileNotCached(t *testing.T) { + h, _, _, _, tmpDir := newTestFirmwareHandler(t) + + // 寫空 VERSION 檔 + chipDir := filepath.Join(tmpDir, "KL520") + if err := os.MkdirAll(chipDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(chipDir, "VERSION"), []byte(""), 0o644); err != nil { + t.Fatal(err) + } + + got := h.bundledVersionFor("KL520") + if got != "unknown" { + t.Errorf("empty file = %q, want unknown", got) + } + h.bundledMu.RLock() + _, cached := h.bundledVersions["KL520"] + h.bundledMu.RUnlock() + if cached { + t.Errorf("空檔不應寫入 cache（避免下次重試永遠回 unknown）") + } +} + +// TestForwardGoroutine_ExitsOnChannelClose 驗證 Reviewer M9-3 第 1 輪 S-5： +// service 端 close progressCh 後、forward goroutine 必須退出（不能 leak）。 +// +// 透過觀察 svc.CleanupTask 被呼叫 = forwardProgressToWS 從 for-range 退出 +// 並執行了 deferred-like cleanup（程式碼是在 for 結束後同步呼叫、非 defer）。 +func TestForwardGoroutine_ExitsOnChannelClose(t *testing.T) { + h, svc, lookup, hub, _ := newTestFirmwareHandler(t) + addFakeDevice(lookup, "dev-leak", "kneron_kl520", "KDP") + + svc.upgradeTaskID = "tk-leak" + svc.upgradeProgress = []firmware.FirmwareProgress{ + {DeviceID: "dev-leak", Stage: firmware.StagePreparing, Percent: 1}, + {DeviceID: "dev-leak", Stage: firmware.StageDone, Percent: 100}, + } + + w := performUpgradeRequest(h, "dev-leak") + if w.Code != http.StatusAccepted { + t.Fatalf("status = %d, want 202", w.Code) + } + + // 等 broadcast + cleanup 都完成 = goroutine 已退出 + waitForBroadcasts(t, hub, 2, 2*time.Second) + waitForCleanup(t, svc, "dev-leak", 2*time.Second) + + // 直接斷言：cleanupCalls 含 dev-leak 且只一次（goroutine 退出後不會再 + // 重複呼） + svc.mu.Lock() + count := 0 + for _, id := range svc.cleanupCalls { + if id == "dev-leak" { + count++ + } + } + svc.mu.Unlock() + if count != 1 { + t.Errorf("CleanupTask(dev-leak) 呼叫 %d 次，want 1（goroutine 該乾淨退出一次）", count) + } +} + +func TestChipFromDeviceType(t *testing.T) { + cases := []struct { + devType string + want string + }{ + {"kneron_kl520", "KL520"}, + {"kneron_kl720", "KL720"}, + {"kneron_kl630", "KL630"}, + {"kneron_kl730", "KL730"}, + {"Kneron_KL520", "KL520"}, // case insensitive + {"unknown", ""}, + {"", ""}, + } + for _, tc := range cases { + t.Run(tc.devType, func(t *testing.T) { + got := ChipFromDeviceType(tc.devType) + if got != tc.want { + t.Errorf("ChipFromDeviceType(%q) = %q, want %q", tc.devType, got, tc.want) + } + }) + } +} + +// ────────────────────────────────────────────────────────────────────── +// helpers：等 goroutine 完成 +// ────────────────────────────────────────────────────────────────────── + +func waitForBroadcasts(t *testing.T, hub *spyBroadcasterFW, n int, timeout time.Duration) { + t.Helper() + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + if len(hub.snapshot()) >= n { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatalf("timeout waiting for %d broadcasts, got %d", n, len(hub.snapshot())) +} + +func waitForCleanup(t *testing.T, svc *fakeFirmwareService, deviceID string, timeout time.Duration) { + t.Helper() + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + svc.mu.Lock() + for _, id := range svc.cleanupCalls { + if id == deviceID { + svc.mu.Unlock() + return + } + } + svc.mu.Unlock() + time.Sleep(5 * time.Millisecond) + } + t.Fatalf("timeout waiting for CleanupTask(%q)", deviceID) +} diff --git a/local-tool/server/internal/api/router.go b/local-tool/server/internal/api/router.go index b92985b..7da7238 100644 --- a/local-tool/server/internal/api/router.go +++ b/local-tool/server/internal/api/router.go @@ -11,6 +11,7 @@ import ( "visiona-local/server/internal/api/ws" "visiona-local/server/internal/camera" "visiona-local/server/internal/device" + "visiona-local/server/internal/firmware" "visiona-local/server/internal/flash" "visiona-local/server/internal/inference" "visiona-local/server/internal/model" @@ -26,6 +27,8 @@ func NewRouter( cameraMgr *camera.Manager, flashSvc *flash.Service, inferenceSvc *inference.Service, + firmwareSvc *firmware.Service, // M9-3：firmware 升降版 service + firmwareDir string, // M9-3：bundled firmware//VERSION 根目錄 wsHub *ws.Hub, staticFS http.FileSystem, logBroadcaster *logger.Broadcaster, @@ -49,6 +52,19 @@ func NewRouter( deviceHandler := handlers.NewDeviceHandler(deviceMgr, flashSvc, inferenceSvc, wsHub) cameraHandler := handlers.NewCameraHandler(cameraMgr, deviceMgr, inferenceSvc, wsHub) + // M9-3：firmware handler。firmwareSvc 可為 nil（test / 未來 disable）、 + // 此時 firmware endpoint 不註冊、device handler 仍用 fwHandler=nil fallback。 + var firmwareHandler *handlers.FirmwareHandler + if firmwareSvc != nil { + firmwareHandler = handlers.NewFirmwareHandler( + firmwareSvc, + deviceMgr, + wsHub, + firmwareDir, + ) + deviceHandler.SetFirmwareHandler(firmwareHandler) + } + api := r.Group("/api") { // System @@ -80,6 +96,14 @@ func NewRouter( api.POST("/devices/:id/inference/start", deviceHandler.StartInference) api.POST("/devices/:id/inference/stop", deviceHandler.StopInference) + // Firmware (M9-3、A 階段) + // upgrade endpoint 走 202 + WebSocket room "firmware:" 推進度。 + // active-tasks 給 Wails control panel graceful shutdown 偵測用。 + if firmwareHandler != nil { + api.POST("/devices/:id/firmware/upgrade", firmwareHandler.UpgradeDevice) + api.GET("/firmware/active-tasks", firmwareHandler.ListActiveTasks) + } + // Camera api.GET("/camera/list", cameraHandler.ListCameras) api.POST("/camera/start", cameraHandler.StartPipeline) diff --git a/local-tool/server/main.go b/local-tool/server/main.go index 6492419..7d52694 100644 --- a/local-tool/server/main.go +++ b/local-tool/server/main.go @@ -23,6 +23,7 @@ import ( "visiona-local/server/internal/config" "visiona-local/server/internal/deps" "visiona-local/server/internal/device" + "visiona-local/server/internal/firmware" "visiona-local/server/internal/flash" "visiona-local/server/internal/inference" "visiona-local/server/internal/model" @@ -249,6 +250,16 @@ func main() { flashSvc := flash.NewService(deviceMgr, modelRepo, builtInDataDir) inferenceSvc := inference.NewService(deviceMgr) + // M9-3：firmware service（升降版 orchestrator）。 + // firmwareDir 解析：bridge script 同一個 scripts/ 目錄下的 firmware/ 子目錄。 + // 例：scripts/kneron_bridge.py → scripts/firmware//{fw_*.bin, VERSION} + firmwareDir := filepath.Join(filepath.Dir(bridgeScript), "firmware") + logger.Info("Firmware bundle dir: %s", firmwareDir) + firmwareSvc := firmware.NewService( + handlers.NewDeviceManagerAdapter(deviceMgr), + firmware.FirmwareDir{Root: firmwareDir}, + ) + // Determine static file system for embedded frontend var staticFS http.FileSystem if !cfg.DevMode { @@ -295,7 +306,7 @@ func main() { systemHandler := handlers.NewSystemHandler(Version, BuildTime, pythonBinForSystem, restartFn, wsHub) // Create router - r := api.NewRouter(modelRepo, modelStore, deviceMgr, cameraMgr, flashSvc, inferenceSvc, wsHub, staticFS, logBroadcaster, systemHandler) + r := api.NewRouter(modelRepo, modelStore, deviceMgr, cameraMgr, flashSvc, inferenceSvc, firmwareSvc, firmwareDir, wsHub, staticFS, logBroadcaster, systemHandler) // Configure HTTP server (bind to localhost only) addr := cfg.Addr()