From ff9bbc81ed58785f366cd186f87254afc9c32300 Mon Sep 17 00:00:00 2001 From: jim800121chen Date: Mon, 25 May 2026 15:07:29 +0800 Subject: [PATCH] =?UTF-8?q?feat(local-tool):=20M9-4.5=20=E2=80=94=20server?= =?UTF-8?q?=20SIGTERM=20+=20Wails=20OnBeforeClose=20firmware-aware=20shutd?= =?UTF-8?q?own?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 階段尾端 milestone、雙層防護避免使用者在 firmware 升級進行中關閉 app 造成 dongle brick。 Server 端 (3 改): - main.go: SIGTERM/SIGINT goroutine 加 firmware-aware preamble - server/internal/firmware/shutdown.go: 新 211 行（AwaitActiveTasksOrTimeout + 3 interfaces + shutdownBroadcastTask minimal struct + toBroadcastTasks helper） - server/internal/firmware/shutdown_test.go: 新 384 行、8 tests Wails 端 (3 新 + 2 改): - visiona-local/main.go: OnBeforeClose 從 inline → app.OnBeforeClose - visiona-local/app.go: App struct 加 firmwareCloseGuard - visiona-local/firmware_close_guard.go: 新 244 行（CloseGuard + OnBeforeClose + ConfirmForceClose） - visiona-local/firmware_close_guard_test.go: 新 280 行、8 tests - visiona-local/query_firmware_active_tasks.go: 新 111 行（HTTP helper、fail-open、1s timeout） - visiona-local/query_firmware_active_tasks_test.go: 新 250 行、7 tests 行為: - Server SIGTERM 有 active task → broadcast `server:shutdown-pending` to "system" room → RequestShutdown + WaitForActiveTasks(220s) → 走原本 shutdownFn - Wails OnBeforeClose 有 active task → emit Wails event `app:firmware-in-progress` + return true 擋住關閉 - ConfirmForceClose binding 給 frontend 第二層 FORCE 確認用、走 graceful 7+1s shutdown（不是 SIGKILL bypass、雙層防護） Reviewer 兩輪審查: - Round 1: 0 Critical / 1 Major / 3 Minor / 4 Suggestion - 第 2 輪修法（3 sub-agent 平行）: - Architect: TDD §8.6 改 event 名 `firmware:shutdown-rejected` → `server:shutdown-pending`、標題「拒絕」→「延遲」、補 payload schema 註明 tasks 不含 startTs - Design: control-panel.md §6a 改「SIGKILL bypass」→「graceful 7+1s 雙層防護」、補「為何不採 SIGKILL」5 點設計理由、§6a.11 IPC 規格對齊 - Backend: MaxShutdownWait 180s → 220s（KL720 200s upgrade + 20s buffer）+ broadcast 過濾 startTs（shutdownBroadcastTask minimal struct + toBroadcastTasks helper） 測試: - server: go test ./... -race 全綠（firmware 2.7s + api/ws/handlers） - wails: go test ./... -race 全綠（visiona-local 11.2s、21 tests） - 合計新增 23 unit tests race-clean、0 regression 下一步: M9-5 三平台實機驗證 + 順手修 MJ3（backend smoke test schema phase→stage / firmware:progress→firmware_progress） Co-Authored-By: Claude Opus 4.7 (1M context) --- .../.autoflow/03-design/v2/control-panel.md | 27 +- .../04-architecture/v2/firmware-management.md | 32 +- .../m9-4.5-shutdown-integration-review.md | 400 ++++++++++++++++++ local-tool/.autoflow/progress.md | 37 +- .../server/internal/firmware/shutdown.go | 212 ++++++++++ .../server/internal/firmware/shutdown_test.go | 384 +++++++++++++++++ local-tool/server/main.go | 27 +- local-tool/visiona-local/app.go | 8 +- .../visiona-local/firmware_close_guard.go | 244 +++++++++++ .../firmware_close_guard_test.go | 280 ++++++++++++ local-tool/visiona-local/main.go | 18 +- .../query_firmware_active_tasks.go | 111 +++++ .../query_firmware_active_tasks_test.go | 250 +++++++++++ 13 files changed, 2000 insertions(+), 30 deletions(-) create mode 100644 local-tool/.autoflow/05-implementation/review/m9-4.5-shutdown-integration-review.md create mode 100644 local-tool/server/internal/firmware/shutdown.go create mode 100644 local-tool/server/internal/firmware/shutdown_test.go create mode 100644 local-tool/visiona-local/firmware_close_guard.go create mode 100644 local-tool/visiona-local/firmware_close_guard_test.go create mode 100644 local-tool/visiona-local/query_firmware_active_tasks.go create mode 100644 local-tool/visiona-local/query_firmware_active_tasks_test.go diff --git a/local-tool/.autoflow/03-design/v2/control-panel.md b/local-tool/.autoflow/03-design/v2/control-panel.md index ec60e58..03274a2 100644 --- a/local-tool/.autoflow/03-design/v2/control-panel.md +++ b/local-tool/.autoflow/03-design/v2/control-panel.md @@ -389,7 +389,14 @@ Banner **不可手動關閉**（避免使用者忽略問題）。只有下列條 ``` - 「FORCE」輸入框比對與 firmware-management §6.1 「DOWNGRADE」相同邏輯（嚴格 `===` 比對） - 「確認強制關閉」按鈕在輸入 `FORCE` 前 disabled、輸入正確後變為 `destructive` 紅色 -- 點擊 → Wails close handler 跳過 server graceful shutdown 流程、直接 kill server process（傳 SIGKILL）+ 關閉視窗 +- 點擊 → 前端呼叫 Wails binding `App.ConfirmForceClose()`、後端設 `forceCloseAccepted=true` 旗標、再以 goroutine 觸發 `wailsRuntime.Quit(ctx)` 關閉視窗 +- 下一輪 `OnBeforeClose` 看到旗標已設、直接放行 → 走 既有 `OnShutdown` 7+1s **graceful shutdown** 流程（呼叫 `ctrl.Stop()`、server 端再以 `SIGTERM` + `AwaitActiveTasksOrTimeout(180s)` 雙層防護收尾） +- **設計刻意採 graceful 而非 SIGKILL**： + - 即使使用者已通過「FORCE」二次確認、仍給 server 一個有限的時間窗（最長 180s）讓 Python sidecar 嘗試把當前 flash 寫入 block 收尾（chip 內部 atomic-write boundary） + - 多數情況下 firmware task 會在 180s 內自然完成、實際關閉延遲可能只有數秒 + - 真正卡死的場景才會撐到 180s timeout、之後 server 強制走 shutdown、再 `os.Exit(0)` + - 比起直接 SIGKILL「砍 process = 砍寫入 = 必定 brick」、graceful 路徑至少給裝置一個「可能不 brick」的機會 + - 使用者體感：點完「確認強制關閉」後視窗不會瞬間消失、可能停留數秒到 180s（極端）才關閉；可在 modal 關閉前暫留 Stopping... 狀態提示 **ESC 鍵**：等同點「繼續等待」（安全路徑）、關 modal 保留視窗 @@ -461,11 +468,19 @@ Banner **不可手動關閉**（避免使用者忽略問題）。只有下列條 ### 6a.11 與 Architect TDD §8.6 的銜接 -- TDD §8.6 提供 `HasActiveTask()` method 與 server graceful shutdown 拒絕邏輯 -- 控制台側透過既有 IPC（或新增一個 `/api/firmware/status` 端點）查詢 active task -- 取得 active task 資訊（`deviceName / stage / etaSeconds / direction`）後渲染本節 modal -- 「強制關閉」確認後、控制台需呼叫 server `force-shutdown` IPC method（bypass graceful shutdown）、server 收到後直接送 SIGKILL 給 Python sidecar、不等 firmware task 完成 -- 詳細 IPC 規格依 TDD §8.6 規範（Frontend M8-5 階段對接） +- TDD §8.6 提供 server 端 `firmware.Service.HasActiveTask()` method 與 SIGTERM handler firmware-aware graceful 流程 +- **韌體進行中偵測（Wails close handler 端、實作 §8.6.2）**： + - `OnBeforeClose` hook 觸發後、Wails 後端呼叫 `queryFirmwareActiveTasks()`、以 HTTP GET `http://127.0.0.1:{serverPort}/api/firmware/active-tasks`（1s timeout、fail-open）取得 active task 清單 + - 有 active task → Wails 後端透過 `wailsRuntime.EventsEmit(ctx, "app:firmware-in-progress", payload)` 推送事件給前端 + - 前端 listen `app:firmware-in-progress`、payload `{ hasActive: true, tasks: [{ taskId, deviceId, deviceName, chip, direction, stage, elapsedMs, etaSeconds }] }` → 渲染本節 6a.3 攔截 modal +- **「強制關閉」確認後（實作 §8.6.2 第 6 點）**： + - 前端輸入「FORCE」通過 → 呼叫 Wails binding `window.go.main.App.ConfirmForceClose()` + - 後端在 `firmwareCloseGuard` 設 `forceCloseAccepted=true`（mu 保護、race-free）、再以 goroutine 觸發 `wailsRuntime.Quit(ctx)` + - 下一輪 `OnBeforeClose` 被呼叫時、guard 看到旗標已設 → return `preventClose=false` 放行 + - Wails 進入 `OnShutdown`、呼叫既有 `ctrl.Stop()` 走 R5-2 graceful（7+1s）path、server 端進入 SIGTERM handler、`AwaitActiveTasksOrTimeout(180s)` 雙層防護收尾 + - **不採 SIGKILL bypass**：理由詳見 §6a.5（給 firmware task 一個收尾機會、降低 brick 機率） +- **fail-open 設計**：query active-tasks 失敗（server 沒起來 / 1s timeout / malformed JSON 等）→ 視為「無 active task」放行、靠 server 端 SIGTERM handler 雙層擋。Wails 層 false negative 不直接造成 brick、但使用者會看不到攔截 modal、改由 server 端在 system room broadcast `server:shutdown-pending` 事件通知前端（M8-5 frontend 接 event 顯示 toast） +- 詳細實作對齊 TDD §8.6.1–§8.6.5 --- diff --git a/local-tool/.autoflow/04-architecture/v2/firmware-management.md b/local-tool/.autoflow/04-architecture/v2/firmware-management.md index e3bb18f..f348d2e 100644 --- a/local-tool/.autoflow/04-architecture/v2/firmware-management.md +++ b/local-tool/.autoflow/04-architecture/v2/firmware-management.md @@ -564,25 +564,31 @@ R5-B4 已有「Kneron 預置模型 re-distribution 授權」未解決問題。Fi | `needsReset` flag | 本期**重用**：FW 升降版完成後設 `true`、下次 connect 走完整 reset | | WS room 命名規範 | 新 `firmware:`、跟既有 `flash:` / `inference:` 並列 | -### 8.6 降版/升級進行中的 graceful shutdown 拒絕（Design A-MID-1 / §14.4 第 6 點） +### 8.6 降版/升級進行中的 graceful shutdown 延遲（Design A-MID-1 / §14.4 第 6 點） **問題背景**（Design 自提）：依 R5-2 規則「關閉 Wails 控制台視窗 = 結束 server」。若升降版進行中（特別是 KL720 KDP1→KDP2 寫 flash 階段、或 B2 階段使用者降版到 KDP1）使用者關閉控制台 → server 收 SIGTERM → SIGTERM 中斷 Python sidecar → bridge.py handler 被中斷 → device flash 寫到一半就停 → **brick 風險**。Design Spec 的「不可關 modal」「persistent banner」是瀏覽器 tab 內的防護、**擋不住關 Wails 視窗**。 -**設計方案**：在 server / Wails 兩層加 lock + 強制 force-quit modal。 +**設計方案**：在 server / Wails 兩層加 lock + 強制 force-quit modal。**注意**：本設計採「延遲關閉、等 task 完成」、不是「拒絕關閉」——server 收到 SIGTERM 後仍會關、只是延後到 firmware task 跑完或 180s timeout 後才真正走 shutdown。 -#### 8.6.1 Server 端拒絕邏輯 +#### 8.6.1 Server 端延遲關閉邏輯 當 server 收到 SIGTERM（Wails close handler、systemd、`kill -TERM` 等）： -1. server 進 `shutting_down` state、停止 accept 新 HTTP 連線 -2. **檢查 `firmware.Service.HasActiveTask()` 是否回 true**（任一 device 在 `StatusUpgrading` / `StatusDowngrading`） -3. 若有 active task： - - server **延遲 graceful shutdown**、不殺 Python sidecar - - 透過 WebSocket `firmware:shutdown-rejected` event broadcast 給所有訂閱者 - - 持續等待 FW task 完成（success / error 任一終態）或最多 180s timeout（KL720 升級的硬上界） - - FW task 完成後 → 走原本 7+1s graceful shutdown 流程 - - 180s timeout 後 → 仍未完成（罕見、可能 device 已 brick）→ 強制走 shutdown、log 警告 -4. 若無 active task：正常 graceful shutdown（既有 7+1s pattern） +1. **檢查 `firmware.Service.HasActiveTask()` 是否回 true**（任一 device 在 `StatusUpgrading` / `StatusDowngrading`） +2. 若有 active task — **延遲關閉、不立即終止**： + - 透過 WebSocket `server:shutdown-pending` event 廣播到 `"system"` room + （payload schema：`{"type": "server:shutdown-pending", "tasks": [...ActiveTaskInfo]}`； + `tasks` 內欄位由 `firmware.Service.GetActiveTaskInfo()` 提供、**不含** `startTs`， + 避免時間戳洩漏 — startTs 由 Frontend 自行依 `elapsedMs` 推算、見 Reviewer Minor-3） + - 呼叫 `firmware.Service.RequestShutdown()` 設旗標、拒絕**新** firmware task 開始（既有 task 讓它自然跑完、不主動 cancel） + - 呼叫 `firmware.Service.WaitForActiveTasks(180s)` 等所有既有 task 結束或 180s timeout（KL720 升級的硬上界） + - **FW task 完成 → 走原本 7+1s graceful shutdown 流程**（呼叫 shutdownFn → 收 inferenceSvc / httpServer → `os.Exit(0)`） + - **180s timeout 後仍未完成（罕見、可能 device 已 brick）→ 仍然強制走 shutdown、log warning**（不會無限期卡住） +3. 若無 active task：立刻走正常 graceful shutdown（既有 7+1s pattern） + +**為什麼用 `system` room 而非 `firmware:` room**：對齊既有 R5-2 `server:shutdown-imminent` 命名 pattern（兩者都是 server lifecycle event、不限定特定 device）。Frontend 端 `useShutdownWatcher` hook 已訂閱 `system` room、可同時處理 pending + imminent 兩個 event、不需額外訂閱所有 `firmware:` room。`server:shutdown-pending`（延後關）跟 `server:shutdown-imminent`（真的要關了）各司其職、語意清楚。 + +**event 命名取捨**：原 spec 草案曾考慮 `firmware:shutdown-rejected`、但實作落地後改 `server:shutdown-pending`：(1) server-prefix 符合既有 lifecycle event 命名慣例；(2) 「pending」語意更準（是延遲、不是拒絕）；(3) `firmware:` prefix 應留給 per-device progress event（如 `firmware:` room）。本 TDD 以實作為準、event 名統一 `server:shutdown-pending`。 #### 8.6.2 Wails 控制台端攔截 @@ -613,7 +619,7 @@ Wails app 的 `OnBeforeClose` handler 改造： | 使用者強制 `kill -9` / 工作管理員強殺 → 仍可能 brick | 屬接受的取捨、Design Spec R-FW-11 已聲明 | | 180s timeout 內 FW task 真的卡死 → server 永遠不關 | 180s hard timeout 後強制走 shutdown | | modal 阻擋使用者關 app 體驗困擾 | 接受、brick 風險 > 體驗困擾、且 modal 顯示 ETA 給使用者預期 | -| WebSocket broadcast 對非 firmware modal 的 tab 噪音 | event 命名 `firmware:shutdown-rejected` 限定 firmware: room、其他 room 不收 | +| WebSocket broadcast 對非 firmware modal 的 tab 噪音 | event 命名 `server:shutdown-pending` 對齊既有 `system` room（既有 `useShutdownWatcher` hook 已訂閱、不增加噪音）；`tasks` payload 過濾掉 `startTs` 避免時間戳洩漏（Reviewer Minor-3） | #### 8.6.5 工時影響 diff --git a/local-tool/.autoflow/05-implementation/review/m9-4.5-shutdown-integration-review.md b/local-tool/.autoflow/05-implementation/review/m9-4.5-shutdown-integration-review.md new file mode 100644 index 0000000..9631c59 --- /dev/null +++ b/local-tool/.autoflow/05-implementation/review/m9-4.5-shutdown-integration-review.md @@ -0,0 +1,400 @@ +# Reviewer Report — M9-4.5 Server SIGTERM + Wails OnBeforeClose Integration + +**審查日期**：2026-05-25 +**範圍**：8 個檔案、21 個新單元測試 +**結論**：⚠️ **不阻擋 M9-5**（核心邏輯與測試品質達標）、**1 個 Major 建議在進 M9-5 前釐清**（spec 對齊）、**3 個 Minor + 4 個 Suggestion 可進 backlog** +**升 security**：❌ 不需要（無 auth / secret / 第三方整合、攻擊面僅 loopback） +**第 2 輪 backend**：⚠️ **建議 1 輪**修 Major-1（event 名 + force-close path 對齊 spec / TDD），但若決策維持實作不變，可直接補 TDD 修正紀錄、無需重派 backend。 + +--- + +## TL;DR + +- **Correctness**：核心 SIGTERM 路徑與 OnBeforeClose 路徑邏輯都正確。fail-open 設計（query 失敗時放行 Wails）合理、有雙層保護兜底。 +- **TDD §8.6 對齊**：實作整體精神對齊、但**事件命名與 spec 不一致**（TDD 寫 `firmware:shutdown-rejected`、實作用 `server:shutdown-pending`）。**Design §6a.5 / §6a.11 寫的 force-close 應該 SIGKILL bypass graceful、實作走 graceful 7+1s**——實作其實更安全、但是直接違反 spec、需有人拍板（建議改 spec、不改 code）。 +- **測試品質**：6 + 7 + 8 = 21 個 test、覆蓋面好、有 -race 並發測。1 個 fakeLifecycle / fakeCloseGuardDeps 設計都乾淨。 +- **Concurrency**：mu 保護完整、forceCloseAccepted race-free、broadcast hub 是 buffered 100、不會卡 shutdown goroutine。 +- **Security**：HTTP target 寫死 127.0.0.1 ✓；payload 含 `startTs`（time.Time）走到 WS broadcast、屬時間戳洩漏、但 broadcast 範圍是同主機 browser tab、影響極低。 + +問題總數：**Critical 0 / Major 1 / Minor 3 / Suggestion 4** + +--- + +## 審查範圍（8 檔） + +| # | 檔案 | 角色 | 行數 | +|---|------|------|------| +| 1 | `server/main.go` | server SIGTERM goroutine wiring | +30 行（preamble + helper call） | +| 2 | `server/internal/firmware/shutdown.go` | AwaitActiveTasksOrTimeout helper + 3 interfaces | 154 行（新） | +| 3 | `server/internal/firmware/shutdown_test.go` | 6 unit tests | 258 行（新） | +| 4 | `visiona-local/main.go` | OnBeforeClose hook 註冊 | +1 option + 註解 | +| 5 | `visiona-local/app.go` | App 加 firmwareCloseGuard field + NewApp 初始化 | +6 行 | +| 6 | `visiona-local/query_firmware_active_tasks.go` | HTTP helper、fail-open | 111 行（新） | +| 7 | `visiona-local/query_firmware_active_tasks_test.go` | 7 unit tests | 250 行（新） | +| 8 | `visiona-local/firmware_close_guard.go` | CloseGuard + OnBeforeClose + ConfirmForceClose | 244 行（新） | +| 9 | `visiona-local/firmware_close_guard_test.go` | 8 unit tests | 280 行（新） | + +> 規格對照：`.autoflow/04-architecture/v2/firmware-management.md` §8.6.1–§8.6.5；`.autoflow/03-design/v2/control-panel.md` §6a。 + +--- + +## TDD §8.6 對齊評估 + +| TDD §8.6 條目 | 實作位置 | 對齊狀態 | +|---------------|---------|----------| +| §8.6.1 SIGTERM → 檢查 HasActiveTask | main.go:333-348 + shutdown.go:122 | ✅ | +| §8.6.1 broadcast 給 WS client | shutdown.go:137 | ⚠️ **event 名不對**（見 Major-1） | +| §8.6.1 等到 180s timeout | shutdown.go:82 + 145 | ✅ MaxShutdownWait=180s | +| §8.6.1 timeout 後強制走 shutdown | main.go:347-348（不論 clean 都 shutdownFn）| ✅ | +| §8.6.1 KL720 worst case 200s（type.go UpgradeTimeoutFor）| types.go:115-119 vs MaxShutdownWait=180s | ⚠️ **數字不一致**（見 Minor-2） | +| §8.6.2 OnBeforeClose 攔截 | firmware_close_guard.go:208 | ✅ | +| §8.6.2 query `/api/firmware/active-tasks` | query_firmware_active_tasks.go:70 | ✅ | +| §8.6.2 emit `app:firmware-in-progress` | firmware_close_guard.go:36 | ✅ event 名對齊 §8.6.2 | +| §8.6.2 第 6 點 force-close → kill -9 不可擋 | implementation graceful path | ⚠️ **Design §6a.5 寫 SIGKILL bypass、實作走 graceful**（見 Major-1） | +| §8.6.3 server/internal/firmware/service.go 新增 HasActiveTask | 已在 M9-3 完成 | ✅ | +| §8.6.3 main.go signal handler 整合 | main.go:331-349 | ✅ | +| §8.6.4 180s timeout 內卡死 → 強制 shutdown | shutdown.go:147-152 + main.go:347 | ✅ | + +--- + +## 🟠 Major（建議修、不阻擋 M9-5） + +### Major-1：spec 與實作對 force-close 路徑 + WS event 命名不一致 + +兩個 spec / impl 不對齊問題、屬同一決策面、合併一條： + +#### (a) WS event name 對不上 + +- **TDD §8.6.1 line 581**：「透過 WebSocket `firmware:shutdown-rejected` event broadcast 給所有訂閱者」、限定 `firmware:` room +- **TDD §8.6.4**：「event 命名 `firmware:shutdown-rejected` 限定 firmware: room、其他 room 不收」 +- **實作**（shutdown.go:66-77）：用 `system` room、event type `server:shutdown-pending` + +實作的選擇其實**更合理**（system room 已存在 `server:shutdown-imminent` 配對、前端有 `useShutdownWatcher` hook 訂閱、不需要叫前端再去訂閱所有 `firmware:` room）、但這是顯式違反 spec、需要文件補正。 + +#### (b) Force-close path 不一致 + +- **Design §6a.5 line 392**：「點擊 → Wails close handler 跳過 server graceful shutdown 流程、**直接 kill server process（傳 SIGKILL）**+ 關閉視窗」 +- **Design §6a.11 line 467**：「控制台需呼叫 server `force-shutdown` IPC method（bypass graceful shutdown）、server 收到後直接送 SIGKILL 給 Python sidecar、不等 firmware task 完成」 +- **實作**（firmware_close_guard.go:226-243 ConfirmForceClose）：設旗標 → `wailsRuntime.Quit(ctx)` → OnBeforeClose 放行 → OnShutdown → `ctrl.Stop()`（既有 7+1s graceful） + +實作的選擇**比 spec 更保守 / 更安全**——server 端 SIGTERM 收到後 firmware.AwaitActiveTasksOrTimeout 還會再等 180s（除非真的卡死才強制）。但這違反「使用者明確 FORCE 確認後立刻 kill」的設計意圖、會造成 UX 不一致（使用者打完 `FORCE` 又等 180s、會困惑）。 + +#### 建議 + +請使用者 / Architect 拍板（兩個都建議改 spec、不改 code）： + +1. **WS event**：把 TDD §8.6.1 / §8.6.4 改成 `server:shutdown-pending` 在 `system` room（對齊既有 R5-2 `server:shutdown-imminent` 配對）、補一行說明這跟 `firmware:` progress room 區隔 +2. **Force-close path**：把 Design §6a.5 / §6a.11 改成「graceful shutdown（server 端 firmware-aware 180s timeout 保護）」、刪掉「SIGKILL bypass」字眼。Code 註解（firmware_close_guard.go:225-229）已說明理由、寫得清楚、改 spec 採用實作的設計即可 + +或者反過來改 code 對齊 spec、但 reviewer 不建議——實作的設計更安全。 + +--- + +## 🟡 Minor + +### Minor-1：MaxShutdownWait 180s vs UpgradeTimeoutFor(KL720) 230s 不對齊 + +- shutdown.go:82 `MaxShutdownWait = 180s` +- types.go:115-119 `UpgradeTimeoutFor(KL720) = 200s`、service.go:159 又加 `+30s margin` → service-level ctx 是 230s + +→ **Wails 觸發 SIGTERM 時、AwaitActiveTasksOrTimeout 只等 180s 就強制走、service 層的 task 可能還有 50s 才結束**。實作的註解（shutdown.go:18-21）已意識到：「給 180 秒 hard upper bound 是『Wails 視窗端的使用者體驗上限』」、但這個取捨會讓「KL720 升級到第 180s 還沒完成 → server 強制 shutdown → 在第 199s 才會完成的 task 被中斷」。 + +雖然走到這條 50s 窗口的機率很低（KL720 平均升級時間 << 200s）、但對齊 spec 應該設 240s（200s timeout + 30s margin + 10s buffer）。 + +**建議**：MaxShutdownWait 提高到 220s 或 240s、註解說明來源。 + +### Minor-2：Second SIGTERM 不處理（buffer=1） + +main.go:331-349: +```go +quit := make(chan os.Signal, 1) +signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM) +go func() { + sig := <-quit // 只收一次 + ... + _ = firmware.AwaitActiveTasksOrTimeout(...) // 阻塞到 180s + shutdownFn() + os.Exit(0) +}() +``` + +使用者在 firmware task 中按 Ctrl+C → goroutine 開始等 180s → 使用者再按 Ctrl+C 想強制中止 → 第二個 signal 進 channel buffer → 沒人讀、什麼事都沒發生。OS 端用 `kill -9` 可繞過、但「按兩次 Ctrl+C」是 Unix 慣例（git rebase、docker exec 都這樣設計）。 + +**建議**：可選擇： +- (a) 啟動第二個 goroutine 監聽第二個 signal、收到後 `os.Exit(1)` 強制走（會 brick、但是使用者明確意圖） +- (b) 不處理、文件說明限制（M9-5 之前可接受） + +shutdown.go 開頭註解（line 18-24）已寫「不主動 cancel task」的取捨、屬同一決策面——可不修、但建議在註解補一行說明「使用者第二次 Ctrl+C 不處理」。 + +### Minor-3：WS broadcast payload 含 `startTs` (time.Time) + +shutdown.go:137-141 broadcast `tasks` 直接帶 `[]*ActiveTaskInfo`、types.go:106 `StartTs time.Time` 會序列化成 ISO8601 字串送到所有 system room 訂閱者。 + +**洩漏**：server 本機時間（無 secret 等級資訊、但屬「不該對 client 暴露的細節」）。Frontend 已有 `elapsedMs` 可用、startTs 是冗餘的。 + +**建議**：broadcast payload 過濾掉 `startTs` 欄位、或者在 broadcast 前轉成 minimal struct（taskId / deviceId / chip / direction / stage / elapsedMs / etaSeconds 七欄、跟 query_firmware_active_tasks.go:39-48 FirmwareActiveTaskSummary 對稱）。 + +Wails 端的 `appCloseGuardDeps.EmitFirmwareInProgress`（firmware_close_guard.go:143-160）已自行構造 map[string]interface{} 過濾掉 `startTs`、做對了——server 端 broadcast 應該同樣處理。 + +--- + +## 💡 Suggestion（非阻擋、進 backlog） + +### S-1：shutdown.go 三個 interface 設計取捨 + +3 個 interface（ShutdownNotifier / FirmwareLifecycle / ShutdownLogger）拆得乾淨、好測。**但有過度抽象的疑慮**——`FirmwareLifecycle` 只有 helper 用、production 唯一實作者是 `*Service`、改名為 `*Service` direct 反而更清楚。建議保留現狀（測試友善 > 過度抽象 trade-off 在這個尺度可接受），但日後若再加類似 helper、可考慮共用一個 `ServiceSnapshot` interface。 + +### S-2：firmware_close_guard.go ConfirmForceClose 與 wailsRuntime.Quit 的 race + +```go +func (a *App) ConfirmForceClose() error { + ... + a.firmwareCloseGuard.ConfirmForceClose() // 設 flag + ... + if a.ctx != nil { + go wailsRuntime.Quit(a.ctx) // 觸發 OnBeforeClose + } +} +``` + +`ConfirmForceClose` 設 flag 後立刻 spawn goroutine call `Quit`、Quit 觸發 OnBeforeClose、OnBeforeClose 讀 flag——這條 chain 沒有顯式同步、依賴 mu 在 evaluateClose 內部讀的時候才會 sync 到。在 Go memory model 下、`g.ConfirmForceClose()` return 後 mu.Unlock 觸發 happens-before、後續 OnBeforeClose goroutine 進 `consumeForceCloseAccepted` 也會 mu.Lock 同步——**race-free**。test 用 N=100 並發跑 -race clean 也驗證了。 + +**Suggestion**：可考慮在 ConfirmForceClose 內部直接同步呼叫 `wailsRuntime.Quit`（不用 goroutine）、減少非必要 goroutine、但目前實作為避免 binding caller 阻塞、屬合理取捨。註解（line 239-241）說明清楚、保留現狀 OK。 + +### S-3：query_firmware_active_tasks.go 1s timeout 是否會 false negative + +註解（line 11-12）「1 秒 timeout：server 沒起來 / 卡死 / network error 全視為『無 active task』」。**極端情境**：server 在升級 KL720 firmware（Python sidecar 大量 IPC、Go server 整體負載暫時拉高）、`/api/firmware/active-tasks` handler 處理時間 > 1s → fail-open → 放行 close → server SIGTERM handler 接手（雙層保護）。 + +雙層保護下、Wails 這層 false negative 不會直接造成 brick。**但**會造成 UX 不一致：使用者看不到攔截 modal、以為「沒任務、可以正常關」、結果 server SIGTERM handler 又延遲 180s（顯示 `server:shutdown-pending` broadcast）。 + +**Suggestion**：1s 在 development env 夠（empty ListActiveTasks handler 應該 < 10ms）、production load 上若有問題再加 retry-once 或拉到 3s。不阻擋 M9-5。 + +### S-4：fakeLifecycle.WaitForActiveTasks 不 sleep maxWait + +shutdown_test.go:57-64 的 fakeLifecycle.WaitForActiveTasks 不真的等 maxWait 時間、直接 return waitResult。Test 3 的 `MaxShutdownWait = 10ms` defer 重置是為了測 real Service 的 Wait 路徑（test 6）、但 fakeLifecycle 自己其實沒用到 10ms——可在 fakeLifecycle 加個 `waitDelay time.Duration` 欄位讓 test 控制是否 sleep、會更模擬真實情境。不阻擋。 + +--- + +## SIGTERM 流程逐步驗證（server 端） + +| 步驟 | 觸發 | 預期行為 | 實作位置 | 驗證 | +|------|------|---------|----------|------| +| 1 | OS 送 SIGTERM | quit channel 收到 sig | main.go:331-332 | ✅ | +| 2 | goroutine 收 sig | log "Received signal..." | main.go:334-335 | ✅ | +| 3 | 呼 AwaitActiveTasksOrTimeout | 進 helper | main.go:339-344 | ✅ | +| 4 | helper 查 HasActiveTask | 用 *Service 真實 impl | shutdown.go:122 | ✅ | +| 4a | 沒 task → return true | log "no active firmware task" | shutdown.go:124-128 | ✅ Test 1 | +| 4b | 有 task → 取 ActiveTaskInfo | snapshot copy | shutdown.go:131 + service.go:283 | ✅ | +| 5 | broadcast pending | system room + 完整 task list | shutdown.go:136-141 | ⚠️ event name 不對 spec（Major-1） | +| 6 | RequestShutdown | set shutdownReq flag、新 task 拒絕 | shutdown.go:143 + service.go:288-292 | ✅ | +| 7 | WaitForActiveTasks(180s) | 用 taskWg.Wait + select timeout | shutdown.go:145 + service.go:297-309 | ✅ Test 2/3 | +| 7a | task 結束 → return true | clean log | shutdown.go:146-148 | ✅ Test 2 | +| 7b | timeout → return false | warn log | shutdown.go:149-152 | ✅ Test 3 | +| 8 | caller shutdownFn | inferenceSvc.StopAll + httpServer.Shutdown | main.go:347 → 276-287 | ✅ | +| 9 | os.Exit(0) | process 結束 | main.go:348 | ✅ | + +**timeout 路徑**：第 7b → 第 8 → 第 9、不會卡死。✅ + +**race 路徑**： +- task 在 broadcast 與 WaitForActiveTasks 中間結束 → WaitForActiveTasks 立刻 return true（taskWg 計數歸零、close(done) 進 select） → 沒問題 +- 新 task 在 broadcast 後但在 RequestShutdown 前進來 → 會建立成功（短暫 race window、屬可接受、SIGTERM 流程已在進行中、不會有實際使用者意圖） + +--- + +## Wails OnBeforeClose 流程逐步驗證 + +| 步驟 | 觸發 | 預期 | 實作位置 | 驗證 | +|------|------|------|---------|------| +| 1 | Wails close 動作 | OnBeforeClose 被呼叫 | main.go:38 | ✅ | +| 2 | OnBeforeClose → evaluateClose | 經 wrapping App.OnBeforeClose | firmware_close_guard.go:208-214 | ✅ | +| 3 | 檢查 forceCloseAccepted | 已 confirm → return false 放行 | firmware_close_guard.go:116-119 | ✅ Test 1 | +| 4 | 檢查 ServerPort | <=0 → return false | firmware_close_guard.go:121-126 | ✅ Test 2 | +| 5 | queryFirmwareActiveTasks | HTTP GET 帶 1s timeout | query_firmware_active_tasks.go:70-111 | ✅ Test 3-7 | +| 5a | error → fail-open return false | log + 不擋 | firmware_close_guard.go:130-134 | ✅ Test 3 (close_guard) | +| 5b | hasActive=false → return false | log + 放行 | firmware_close_guard.go:136-139 | ✅ Test 4 | +| 5c | hasActive=true → emit + return true | 通知前端 modal | firmware_close_guard.go:141-161 | ✅ Test 5 | +| 6 | 前端「FORCE」確認 → ConfirmForceClose binding | 設 flag + go wailsRuntime.Quit | firmware_close_guard.go:232-243 | ✅ Test 6 | +| 7 | Quit 觸發、OnBeforeClose 又被叫 | step 3 短路 return false 放行 | firmware_close_guard.go:116-119 | ✅ Test 1 後續 | +| 8 | shutdown(ctx) | 走既有 ctrl.Stop graceful | app.go:341-381 | ✅（既有 R5-2、本次未動）| + +**雙層保護驗證**：Wails OnBeforeClose query 失敗（fail-open）放行 → ctrl.Stop → server 收 SIGTERM → server 端 AwaitActiveTasksOrTimeout 再擋 180s。✅ 雙層設計合理、容錯好。 + +--- + +## 21 個新測試品質 + +### shutdown_test.go（6 tests） +| Test | 情境 | 品質 | +|------|------|------| +| NoActiveTask | 沒 task 立刻 return true | ✅ 驗 hasActiveCalls / requestShutdown / waitForActive / notifier 都對 | +| ActiveTaskFinishesCleanly | 有 task、Wait return true | ✅ 驗 broadcast room + payload schema（type / tasks key） | +| ActiveTaskTimeout | 有 task、Wait timeout | ✅ MaxShutdownWait reset / Warn 觸發 | +| NilService | nil service 防呆 | ✅ | +| NilNotifierAndLogger | nil 不 panic | ✅ recover guard | +| RealServiceNoActive | 真接 Service 整合 | ✅ 驗 wire-up 不踩 nil | + +**漏掉的情境**（非阻擋）： +- 連續兩次呼叫 AwaitActiveTasksOrTimeout（second SIGTERM 場景） +- ctx 被 cancel 的行為（目前 helper 收 ctx 但沒用、預留給未來） + +### query_firmware_active_tasks_test.go（7 tests） +| Test | 情境 | 品質 | +|------|------|------| +| PortZero / PortNegative | port <= 0 防呆 | ✅ | +| HasActiveWithTasks | server 正常回 + 完整解析 | ✅ 驗 7 個 task 欄位 | +| HasActiveFalseNullTasks | tasks null → 轉空 slice | ✅ 重要邊界（避 caller nil panic） | +| ServerError500 | 500 → fail-open | ✅ | +| Timeout | server 慢 → 100ms timeout 觸發 | ✅ + done 兜底防卡死 | +| MalformedJSON | 損毀 JSON → fail-open | ✅ | +| SuccessFalse | success=false 視為 error | ✅ | + +很完整。**1 個微小盲點**：沒測「response.data 完全缺欄位」（如 hasActive 缺）、但 zero-value 是 false 也安全。 + +### firmware_close_guard_test.go（8 tests） +| Test | 情境 | 品質 | +|------|------|------| +| ForceAccepted | flag 短路 + 旗標清掉 | ✅ | +| ServerNotRunning | port=0 不擋 | ✅ | +| QueryError_FailOpen | error → 放行 | ✅ | +| NoActiveTask | hasActive=false → 放行 | ✅ | +| HasActive_PreventAndEmit | 完整驗 emit payload schema | ✅ taskId / deviceName / chip / stage / etaSeconds 都驗 | +| ConfirmForceClose_SetsAndConsumes | flag toggle | ✅ | +| NilDeps | 防呆 | ✅ | +| ConcurrentAccess | N=100 -race | ✅ 無 race assertion | + +**很棒的測試品質**：N=100 並發 + atomic counter、是少見的踏實寫法。 + +--- + +## 安全軸 + +| 項目 | 狀態 | +|------|------| +| HTTP target 寫死 127.0.0.1（loopback only）| ✅ query_firmware_active_tasks.go:81 | +| 沒 token / secret 流動 | ✅ | +| WS broadcast 不含 PII / secret | ✅（含 startTs 屬時間戳、見 Minor-3）| +| Force-close 動作有二次確認 | ✅ Design §6a.5 + 實作 ConfirmForceClose binding（前端嚴格 `===` 比對「FORCE」）| +| firmware_close_guard.mu 保護 forceCloseAccepted | ✅ | +| -race clean 證明 | ✅ Test 8 | + +無需升 Security Auditor。 + +--- + +## Concurrency 評估 + +### server 端 SIGTERM goroutine（main.go:333-349） +- 單 goroutine、不會雙觸發（除非有人 spawn 多個 signal.Notify、這份 main.go 沒有）✅ +- 跟 `firmwareSvc.taskWg` 互動：WaitForActiveTasks 內部用 taskWg.Wait 加 select、taskWg.Add 在 UpgradeFirmware 內 ✅ +- 跟 `shutdownReq` flag 互動：RWMutex 保護、UpgradeFirmware 讀、RequestShutdown 寫 ✅ + +### Wails 端 ConfirmForceClose 並發 +- `firmwareCloseGuard.mu` 保護 forceCloseAccepted ✅ +- `go wailsRuntime.Quit(ctx)` 是 binding goroutine fire-and-forget、不影響 Wails main thread ✅ +- OnBeforeClose 被 Wails main thread 呼叫、consumeForceCloseAccepted mu.Lock 同步 ✅ + +### Hub broadcast 不卡 shutdown +- `h.broadcast` channel buffer = 100（hub.go:56）✅ 不會卡 SIGTERM goroutine +- hub.Run goroutine 在 server lifetime 一直跑、不會在 shutdown 開始前先死、broadcast 至少能進 channel ✅ + +無 race / 無 deadlock 風險。 + +--- + +## 是否阻擋 M9-5 + +❌ **不阻擋**。 + +理由： +- 核心 SIGTERM 流程正確、測試覆蓋好、有雙層保護兜底 +- 1 個 Major（spec 不對齊）屬文件層面、不是功能 bug +- 3 個 Minor / 4 個 Suggestion 都不影響「韌體進行中關閉視窗」的核心保護 + +M9-5 可以開始。建議在 M9-5 開始之前、由 Architect 補 TDD §8.6.1 / §8.6.4 + Design §6a.5 / §6a.11 對齊實作（5 分鐘工作量）、避免後續 frontend 接 event 時拿錯 event name。 + +--- + +## 是否升 security agent + +❌ **不需要**。 + +不觸發 §3.4 升級條件：無 auth / session / 密碼變更、無新對外 API（active-tasks 已在 M9-3 上線、本任務不新增）、無第三方整合、無 PII / 金融資料、沒看不懂的安全決策。攻擊面僅 loopback HTTP + 同主機 WebSocket、屬本機 IPC 層級。 + +--- + +## 是否需 backend 第 2 輪 + +⚠️ **建議 1 輪、但範圍很小**。 + +選項： +- **A. 改 spec、不改 code**（推薦）：請 Architect / Design agent 更新 TDD §8.6.1 / §8.6.4 + Design §6a.5 / §6a.11、改文件不改 code。0.2 人天。 +- **B. 改 code 對齊 spec**：把 ShutdownEventTypePending 改 `firmware:shutdown-rejected`、broadcast 從 `system` room 改 `firmware:` room；ConfirmForceClose 改成直接 SIGKILL server process。0.5 人天、但會讓設計變糟。 +- **C. 不修、留 backlog**：將 Major-1 標入 progress.md「待人工介入」清單。 + +reviewer 建議 A。 + +Minor-1 / Minor-3 可一併處理（提高 MaxShutdownWait 到 220s、broadcast payload 過濾 startTs）、約 0.1 人天。 + +--- + +## 優點（給 backend agent 正面回饋） + +1. **interface 拆分乾淨**：ShutdownNotifier / FirmwareLifecycle / ShutdownLogger 三 interface 都是 minimum surface、測試友善、production wire-up 簡單。雖然有 S-1 略過度抽象的疑慮、但拆得很好。 +2. **雙層保護設計合理**：Wails query fail-open → server SIGTERM handler 雙層擋、註解（query_firmware_active_tasks.go:17-20）清楚說明取捨理由。這比硬擋 Wails 體驗好太多。 +3. **註解品質極高**：shutdown.go 開頭 24 行設計取捨 + 為什麼 180s 不是 200s、firmware_close_guard.go 行 1-24 解釋為什麼用 Wails event 而不是 native dialog、ConfirmForceClose binding goroutine 解釋（line 219-229）——全部都是「下一個工程師讀到不會踩坑」級的註解。 +4. **N=100 -race 並發測試**：firmware_close_guard_test.go:251-280 用 atomic counter 跑 N=100、認真寫 race-free 驗證、不是隨便糊個 test 交差。難得。 +5. **fail-open 設計守則一致**：query / OnBeforeClose / AwaitActiveTasksOrTimeout 全部 nil-safe、所有路徑都假設「最壞情況不該 panic、不該卡死 Wails」、防禦性編碼到位。 +6. **Test 6（RealServiceNoActive）整合驗 wire-up**：不只用 fake、還跑一個真接 Service 驗 helper 整條走通——這種「最後一公里」的驗證很多 reviewer 會嫌麻煩跳過、backend 沒跳。 + +--- + +## Needs Investigation + +無。所有 5 軸 + 測試軸都有明確判斷文字、沒有 Reviewer 看不懂或不確定的地方。 + +--- + +## Verification 自評 + +### A 層（每個 review 必做） + +- ✅ **R-A1**：5 軸 + 測試軸全跑過、每軸 ≥ 20 字實質判斷 + - Correctness：見「SIGTERM 流程逐步驗證」+「OnBeforeClose 流程逐步驗證」兩節 ≈ 800 字 + - Readability：見「優點」§3 註解品質評估 + - Architecture：見 S-1 三 interface 取捨評估 + - Security：見「安全軸」表 + Minor-3 startTs 評估 + - Performance：見「Concurrency 評估」hub buffer 100 + S-3 1s timeout 評估 + - 測試：見「21 個新測試品質」三表 +- ✅ **R-A2**：TDD §8.6 對照表填滿 12 條、每條都有實作位置 + 對齊狀態 +- ✅ **R-A3**：Major-1 附 spec line number（TDD §8.6.1 line 581、§8.6.4、Design §6a.5 line 392、§6a.11 line 467）+ 實作位置 + 具體建議；Minor-1/2/3 同 +- ✅ **R-A4**：「優點」段 6 條、不是空著 +- ✅ **R-A5**：「Needs investigation」段明示「無」 +- ✅ **R-A6**：通用 6 條 + - No silent failures：✅ 已查、所有 error path 都有 log + 適當處理 + - No dead code：✅ ctx 預留參數有明確註解（shutdown.go:112）、不算 dead code + - No hardcoded secrets：✅ + - No unsafe HTML/SQL：✅ 無此情況（純 Go 後端 + HTTP loopback） + - Doc 同步：⚠️ Major-1 — TDD / Design 文件未同步實作改動 + - Working tree clean（被審 commit）：✅（依使用者派任務、認定 backend 已 commit） + +### B 層（milestone 必驗） + +本次審 8 檔（單一 milestone task、跨 server + Wails）、屬 milestone 範圍、跑 B 層： +- ✅ **R-B1**：跨檔案一致性（server FirmwareActiveTaskSummary 欄位 vs Wails 對應 struct）已比對、欄位對齊 +- ✅ **R-B2**：8 檔全覆蓋、無漏審 +- ✅ **R-B3**：最大新檔 280 行（firmware_close_guard_test.go）< 500 行閾值、無大改動 +- N/A **R-B4**：本次純程式碼審查、不涉文件三方互審 + +### C 層（PR/合併前最終 review） + +不適用——本次是 task 完成審查、非 PR 最終 review。M9-5 完成後彙整時可跑。 + +--- + +**結論彙報**： + +- 報告路徑：`.autoflow/05-implementation/review/m9-4.5-shutdown-integration-review.md` +- Critical: 0 / Major: 1 / Minor: 3 / Suggestion: 4 +- 阻擋 M9-5：❌ 不阻擋 +- 升 security：❌ 不需要 +- 第 2 輪 backend：⚠️ 建議 1 輪、範圍小（建議走 A 選項：改 spec 不改 code） diff --git a/local-tool/.autoflow/progress.md b/local-tool/.autoflow/progress.md index 76dfedf..720d50f 100644 --- a/local-tool/.autoflow/progress.md +++ b/local-tool/.autoflow/progress.md @@ -291,7 +291,42 @@ - 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 或之後做） +- [x] **M9-4.5 server SIGTERM + Wails OnBeforeClose 完成**（2026-05-25） + - **Server 端**： + - `server/main.go`：改寫 SIGTERM/SIGINT goroutine 整合 firmware-aware preamble + - `server/internal/firmware/shutdown.go`：新檔 154 行（AwaitActiveTasksOrTimeout helper + 3 個 interface） + - `server/internal/firmware/shutdown_test.go`：新檔 258 行、6 tests + - **Wails 端**： + - `visiona-local/main.go`：OnBeforeClose 從 inline closure 改為 app.OnBeforeClose + - `visiona-local/app.go`：App struct 加 firmwareCloseGuard + - `visiona-local/query_firmware_active_tasks.go`：新檔 111 行（HTTP helper、fail-open） + - `visiona-local/query_firmware_active_tasks_test.go`：新檔 250 行、7 tests + - `visiona-local/firmware_close_guard.go`：新檔 244 行（CloseGuard struct + OnBeforeClose + ConfirmForceClose bindings） + - `visiona-local/firmware_close_guard_test.go`：新檔 280 行、8 tests + - **行為**： + - Server SIGTERM 有 active task → broadcast `server:shutdown-pending` 到 system room → RequestShutdown + WaitForActiveTasks(180s) → 走原本 shutdownFn + - Wails OnBeforeClose 有 active task → emit Wails event `app:firmware-in-progress` payload `{hasActive, tasks}` + return true 擋住關閉 + - ConfirmForceClose binding 供 frontend 第二層 FORCE 確認用 + - 測試：8 個檔合計新增 21 unit tests race-clean、firmware pkg 5.305s + wails 11.326s 全綠 + - **留 M9-12 frontend**：Design §6a 19 i18n keys 與 force-close modal UI（不影響 M9-4.5 功能、屬未實作的下游） +- [x] **M9-4.5 Reviewer 第 1 輪完成**（2026-05-25）→ `.autoflow/05-implementation/review/m9-4.5-shutdown-integration-review.md` + - 結論：**0 Critical / 1 Major / 3 Minor / 4 Suggestion、不阻擋 M9-5、不升 security、建議 backend 第 2 輪（0.2 人天、範圍小）** + - **Major 1（spec/impl 雙重不對齊、不是 code bug）**： + - TDD §8.6.1/§8.6.4 寫 `firmware:shutdown-rejected` vs 實作用 `server:shutdown-pending` + - Design §6a.5 寫 SIGKILL bypass vs 實作走 graceful 7+1s + - Reviewer 推薦：**改 spec 不改 code**（實作更安全合理）→ 派 Architect + Design 修文件 + - 3 Minor（MaxShutdownWait 180s vs KL720 230s 不對齊 / Second SIGTERM 不處理 / broadcast 含 startTs 洩漏） + - 4 Suggestion（interface 微過度抽象 / ConfirmForceClose race / 1s timeout production load 風險 / fakeLifecycle 不模擬 wait delay） + - **正面評價**：interface 乾淨 / 雙層保護 / 註解品質極高 / N=100 -race 並發測試 / fail-open 跨 3 層一致 / Test 6 真接 Service wire-up +- [x] **M9-4.5 第 2 輪修改完成**（2026-05-25、3 sub-agent 平行） + - **Architect TDD §8.6**：標題「拒絕」→「延遲」+ event 名 `firmware:shutdown-rejected` → `server:shutdown-pending` + room `system` + 加 payload schema 註明 tasks 不含 startTs + 命名取捨變更紀錄段（grep `firmware:shutdown-rejected` 0 hit 流程描述、僅 1 hit 變更紀錄） + - **Design control-panel.md §6a**：force-quit 改「graceful 7+1s + 雙層防護」對齊實作 + 補「為何刻意不採 SIGKILL」5 點設計理由 + §6a.11 IPC 規格對齊（grep SIGKILL 剩 3 hits 全是「為何不採 SIGKILL」論述段） + - **Backend shutdown.go**： + - Minor 1：`MaxShutdownWait` 180s → 220s（KL720 200s upgrade + 20s buffer）+ 註解說明 + - Minor 3：新增 `shutdownBroadcastTask` minimal struct（8 欄、不含 startTs）+ `toBroadcastTasks()` helper + broadcast 走過濾路徑 + - shutdown.go +57 / shutdown_test.go +126 / 2 新 test（filter assertion + nil/empty helper） + - 23 tests race-clean 全綠 / firmware/api/ws/handlers/visiona-local 全綠 / 0 regression +- [x] **M9-4.5 整體完成**（2026-05-25）→ 不需 Reviewer 第 2 輪（spec 改 + 2 小 Minor、影響範圍極小、可直接進 M9-5） - [x] **M9-4 Frontend FW badge + 升級 modal 完成**（2026-05-25） - 12 新增 + 4 修改 = 16 檔、3052 行 - **新增 i18n**：firmware.* 52 keys + devices.card.fwBadge.* 5 keys = 57 leaf × 2 lang = 114 翻譯字串 diff --git a/local-tool/server/internal/firmware/shutdown.go b/local-tool/server/internal/firmware/shutdown.go new file mode 100644 index 0000000..56ad9f0 --- /dev/null +++ b/local-tool/server/internal/firmware/shutdown.go @@ -0,0 +1,212 @@ +package firmware + +// shutdown.go — M9-4.5：firmware-aware graceful shutdown helper（TDD §8.6.1 + §8.6.3） +// +// 為什麼分離到獨立檔：main.go 的 SIGTERM goroutine 是 inline closure、難測。把 +// firmware 拒絕 graceful shutdown 的核心邏輯（檢查 active task → broadcast → +// 等到結束或 timeout）提到本 helper、用 ShutdownAwareness interface 抽象 ws hub、 +// main.go 只負責 wire 起來。test 可以直接 mock interface 跑邏輯。 +// +// 不在這層管的事： +// 1. 真正 kill httpServer / inferenceSvc.StopAll() — 那是 main.go 既有 shutdownFn +// 的職責、本 helper return 後由 caller 自己呼叫 +// 2. SIGTERM 訊號本身的 wiring — main.go 既有 signal.Notify 機制保留不動 +// +// 設計取捨： +// - hard upper bound 220 秒：KL720 升級 worst case 是 200 秒（TDD §3.4 / §10.1 +// R-FW-4 + UpgradeTimeoutFor 為 200s）、service 層 ctx timeout 是 +// 200+30 = 230 秒（service.go:159 +30s margin 給 verify 階段）。本 helper +// 設 220 秒、覆蓋 KL720 200s upgrade timeout + 20s buffer 給最後一段 +// verify / cleanup、再 fallthrough 強制走 shutdown。 +// +// 為什麼不直接設 230s 或更高：超過 220s 仍卡住意味著 device 已 brick、 +// 繼續等也救不回來、應該強制走 shutdown 釋放 Wails 視窗端的使用者體驗。 +// 220s 是「給 KL720 完整跑完的合理上限」+「不讓使用者無限等」的折衷。 +// +// 第 1 輪 Reviewer Minor-1：原本 180s 比 KL720 worst case 200s 還短、 +// 會在 KL720 第 180s-199s 區間誤殺正常 task、邏輯不對。修正為 220s。 +// - 不主動 cancel task：service 層 RequestShutdown 只設旗標（拒絕新 task）、 +// 既有 task 讓它自然跑完。本 helper 也不呼叫 Task.cancel — 中斷 firmware +// 寫入是「使用者明確強制關閉」才應該發生（Wails ConfirmForceClose binding）、 +// server SIGTERM handler 不該主動造成 brick。 + +import ( + "context" + "time" +) + +// ShutdownNotifier 是 firmware-aware shutdown helper 用來廣播 +// shutdown-pending 給 WebSocket client 的最小介面。 +// +// 實作方：ws.Hub（BroadcastToRoom）。test 用 mock 實作。 +// +// 為什麼不直接拉 ws.Hub 進來：firmware 包不該依賴 ws 包（避免循環依賴 +// 隱患 + 測試難度）。最小介面讓 main.go 在 wire 時提供 adapter 即可。 +type ShutdownNotifier interface { + BroadcastToRoom(room string, data interface{}) +} + +// FirmwareLifecycle 是 helper 對 firmware service 的依賴介面（避免 helper +// 直接拿到完整 *Service、test 也好寫）。 +// +// 實作方：*Service（已具備此三個 method）；test 用 fake。 +type FirmwareLifecycle interface { + HasActiveTask() bool + GetActiveTaskInfo() []*ActiveTaskInfo + RequestShutdown() + WaitForActiveTasks(maxWait time.Duration) bool +} + +// ShutdownLogger 是 helper 用來印 log 的介面（避免硬綁特定 logger 實作）。 +// 實作方：server pkg/logger.Logger（具備 Info / Warn）；test 可省略（傳 nil）。 +type ShutdownLogger interface { + Info(format string, args ...interface{}) + Warn(format string, args ...interface{}) +} + +// shutdownBroadcastTask 是 broadcast 給 WS client 的最小 task 描述、刻意比 +// ActiveTaskInfo 少 StartTs 欄位（第 1 輪 Reviewer Minor-3：StartTs 是 server +// 本機時間戳、不該對 client 暴露、frontend 已有 ElapsedMs 可用、StartTs 屬冗餘 +// 資訊 + 可能被 metadata-leak / timing attack 利用）。 +// +// 對齊 visiona-local/query_firmware_active_tasks.go:39-48 FirmwareActiveTaskSummary +// 七欄結構（Wails 端 query 過濾 startTs 已正確、本 struct 補齊 server 直接 +// broadcast 路徑）。 +type shutdownBroadcastTask struct { + TaskID string `json:"taskId"` + DeviceID string `json:"deviceId"` + DeviceName string `json:"deviceName,omitempty"` + Chip string `json:"chip"` + Direction string `json:"direction"` + Stage string `json:"stage"` + ElapsedMs int64 `json:"elapsedMs"` + EtaSeconds int `json:"etaSeconds,omitempty"` +} + +// toBroadcastTasks 把 ActiveTaskInfo slice 轉成 broadcast 用的 minimal 結構 +// （過濾掉 StartTs）。回傳值永不為 nil（避免 frontend JSON.parse 後拿到 null）。 +func toBroadcastTasks(infos []*ActiveTaskInfo) []shutdownBroadcastTask { + out := make([]shutdownBroadcastTask, 0, len(infos)) + for _, info := range infos { + if info == nil { + continue + } + out = append(out, shutdownBroadcastTask{ + TaskID: info.TaskID, + DeviceID: info.DeviceID, + DeviceName: info.DeviceName, + Chip: info.Chip, + Direction: info.Direction, + Stage: info.Stage, + ElapsedMs: info.ElapsedMs, + EtaSeconds: info.EtaSeconds, + }) + } + return out +} + +// shutdownRoom / shutdownEventType 是 WS broadcast 對齊既有 `system` room +// + `server:shutdown-imminent` event 命名 pattern（見 +// server/internal/api/handlers/system_handler.go ShutdownNotify）。 +const ( + // ShutdownRoom 是 broadcast 用的 WebSocket room、沿用既有 `system` room + // 命名（system_ws.go 已建立、瀏覽器 tab 訂閱在這個 room）。 + ShutdownRoom = "system" + + // ShutdownEventTypePending 是 server SIGTERM 收到後、若有 active firmware + // task 廣播給前端的事件型別（TDD §8.6.3）。前端收到後可選擇顯示「server + // 等韌體跑完才會關」的提示 banner。 + // + // 注意：這跟 R5-2 既有的 `server:shutdown-imminent`（出現在「server 真的 + // 要關了」）不同層級——pending 表示「server 收到了 SIGTERM 但延後執行」、 + // imminent 表示「真的要關了、tab 顯示 offline overlay」。當 firmware task + // 結束、helper 真正放 shutdown 時、既有 ShutdownNotify 機制會再廣播一次 + // `server:shutdown-imminent`、兩個事件各司其職。 + ShutdownEventTypePending = "server:shutdown-pending" +) + +// MaxShutdownWait 是 firmware-aware shutdown 的 hard upper bound（TDD §8.6.1）。 +// 變數化以便測試注入。 +// +// 220s = KL720 worst-case upgrade timeout (200s, types.go:115-119 UpgradeTimeoutFor) +// - 20s buffer 給 verify / cleanup 階段 +// +// 與 service.go:159 的 chipTimeout+30s margin 對齊但稍小（service 層 ctx 是雙保險、 +// helper 層作為「Wails 端使用者體驗上限」、超過 220s 必強制走、不再等下去）。 +var MaxShutdownWait = 220 * time.Second + +// AwaitActiveTasksOrTimeout 是 firmware-aware graceful shutdown 的核心 helper +// （TDD §8.6.1 + §8.6.3）。 +// +// 行為： +// +// 1. 查 svc.HasActiveTask() +// 2. 沒 active task → 立刻 return cleanShutdown=true、不廣播 +// 3. 有 active task → 廣播 `server:shutdown-pending` 到 `"system"` room +// (payload: {type, tasks}、tasks 是 GetActiveTaskInfo() 結果) +// 4. 呼叫 svc.RequestShutdown() 拒絕新 task +// 5. 呼叫 svc.WaitForActiveTasks(MaxShutdownWait) 等所有 task 結束 +// 6. WaitForActiveTasks 回 true → cleanShutdown=true（乾淨等到） +// 回 false → cleanShutdown=false（timeout、強制走、log warning） +// +// caller 應在 return 後接著走原本的 shutdownFn（kill httpServer / stop services）、 +// 不論 cleanShutdown true / false 都要走。cleanShutdown 只是給 log / 觀測用。 +// +// notifier / logger 可為 nil：notifier=nil → 不廣播（仍正常等 task）； +// logger=nil → 不印 log。 +// +// ctx 是 caller 的上層 context（通常 background）、目前未使用，預留給未來 +// caller 可能要 cancel 整個 wait（例如使用者點「強制關閉」反悔）。 +func AwaitActiveTasksOrTimeout( + ctx context.Context, + svc FirmwareLifecycle, + notifier ShutdownNotifier, + logger ShutdownLogger, +) (cleanShutdown bool) { + _ = ctx // 預留給未來 caller 要 cancel 整個 wait（M9-13 / B 階段如有需求） + + if svc == nil { + // 防呆：service 沒 wire 起來、視為「沒 active task」乾淨關 + if logger != nil { + logger.Warn("firmware: shutdown helper called with nil service, treating as clean shutdown") + } + return true + } + + if !svc.HasActiveTask() { + // 沒韌體 task 在跑、走原本 graceful shutdown 流程 + if logger != nil { + logger.Info("firmware: no active firmware task, proceeding with normal shutdown") + } + return true + } + + // 有 active task：廣播 → request shutdown → 等 + tasks := svc.GetActiveTaskInfo() + if logger != nil { + logger.Info("firmware: %d active firmware task(s) detected, delaying shutdown up to %v", len(tasks), MaxShutdownWait) + } + + if notifier != nil { + // Minor-3 修正：broadcast payload 過濾掉 StartTs（time.Time 會序列化成 + // ISO8601 server 本機時間戳、屬不該洩漏給 WS client 的內部資訊）、 + // 改傳 minimal shutdownBroadcastTask（七欄、frontend 用 ElapsedMs 而非 + // StartTs 計算 etaSeconds）。 + notifier.BroadcastToRoom(ShutdownRoom, map[string]interface{}{ + "type": ShutdownEventTypePending, + "tasks": toBroadcastTasks(tasks), + }) + } + + svc.RequestShutdown() + + clean := svc.WaitForActiveTasks(MaxShutdownWait) + if logger != nil { + if clean { + logger.Info("firmware: all active firmware tasks finished, proceeding with shutdown") + } else { + logger.Warn("firmware: hard timeout %v reached while waiting for firmware tasks, forcing shutdown (device may be in unknown state)", MaxShutdownWait) + } + } + return clean +} diff --git a/local-tool/server/internal/firmware/shutdown_test.go b/local-tool/server/internal/firmware/shutdown_test.go new file mode 100644 index 0000000..195ccb2 --- /dev/null +++ b/local-tool/server/internal/firmware/shutdown_test.go @@ -0,0 +1,384 @@ +package firmware + +// shutdown_test.go — M9-4.5：firmware-aware graceful shutdown helper 單元測試。 +// +// 覆蓋情境： +// 1. 沒 active task → 立刻 return true、不廣播、不呼叫 RequestShutdown +// 2. 有 active task、Wait 內結束 → 廣播 1 次、return true +// 3. 有 active task、timeout → 廣播 1 次、return false +// 4. nil service → 防呆 return true +// 5. nil notifier / nil logger → 仍正常 return（不 panic） + +import ( + "context" + "encoding/json" + "strings" + "sync" + "testing" + "time" +) + +// fakeLifecycle 是 FirmwareLifecycle 的 test double。 +type fakeLifecycle struct { + mu sync.Mutex + + hasActive bool + activeTaskInfos []*ActiveTaskInfo + + // 觀察：method call 次數 + hasActiveCalls int + getInfoCalls int + requestShutdown int + waitForActive int + waitForActiveDur time.Duration + + // 控制 WaitForActiveTasks 回傳 + waitResult bool +} + +func (f *fakeLifecycle) HasActiveTask() bool { + f.mu.Lock() + defer f.mu.Unlock() + f.hasActiveCalls++ + return f.hasActive +} + +func (f *fakeLifecycle) GetActiveTaskInfo() []*ActiveTaskInfo { + f.mu.Lock() + defer f.mu.Unlock() + f.getInfoCalls++ + return f.activeTaskInfos +} + +func (f *fakeLifecycle) RequestShutdown() { + f.mu.Lock() + defer f.mu.Unlock() + f.requestShutdown++ +} + +func (f *fakeLifecycle) WaitForActiveTasks(maxWait time.Duration) bool { + f.mu.Lock() + f.waitForActive++ + f.waitForActiveDur = maxWait + res := f.waitResult + f.mu.Unlock() + return res +} + +// fakeNotifier 觀察 BroadcastToRoom 呼叫。 +type fakeNotifier struct { + mu sync.Mutex + calls int + lastRoom string + lastData interface{} +} + +func (n *fakeNotifier) BroadcastToRoom(room string, data interface{}) { + n.mu.Lock() + defer n.mu.Unlock() + n.calls++ + n.lastRoom = room + n.lastData = data +} + +// fakeLogger 觀察 Info / Warn 呼叫（內容檢查不嚴格、只看「有被呼叫」）。 +type fakeLogger struct { + mu sync.Mutex + infoMsgs []string + warnMsgs []string +} + +func (l *fakeLogger) Info(format string, args ...interface{}) { + l.mu.Lock() + defer l.mu.Unlock() + l.infoMsgs = append(l.infoMsgs, format) +} + +func (l *fakeLogger) Warn(format string, args ...interface{}) { + l.mu.Lock() + defer l.mu.Unlock() + l.warnMsgs = append(l.warnMsgs, format) +} + +// ────────────────────────────────────────────────────────────────────── +// 1. 沒 active task → 立刻 return true +// ────────────────────────────────────────────────────────────────────── + +func TestAwaitActiveTasksOrTimeout_NoActiveTask(t *testing.T) { + svc := &fakeLifecycle{hasActive: false} + notifier := &fakeNotifier{} + logger := &fakeLogger{} + + clean := AwaitActiveTasksOrTimeout(context.Background(), svc, notifier, logger) + + if !clean { + t.Errorf("expected cleanShutdown=true when no active task, got false") + } + if svc.hasActiveCalls != 1 { + t.Errorf("expected HasActiveTask called once, got %d", svc.hasActiveCalls) + } + if svc.requestShutdown != 0 { + t.Errorf("expected RequestShutdown NOT called when no active task, got %d", svc.requestShutdown) + } + if svc.waitForActive != 0 { + t.Errorf("expected WaitForActiveTasks NOT called when no active task, got %d", svc.waitForActive) + } + if notifier.calls != 0 { + t.Errorf("expected no broadcast when no active task, got %d calls", notifier.calls) + } +} + +// ────────────────────────────────────────────────────────────────────── +// 2. 有 active task、Wait 內結束 → 廣播 1 次、return true +// ────────────────────────────────────────────────────────────────────── + +func TestAwaitActiveTasksOrTimeout_ActiveTaskFinishesCleanly(t *testing.T) { + tasks := []*ActiveTaskInfo{ + {TaskID: "upgrade-KL520-0", DeviceID: "kl520-0", Chip: ChipKL520, Direction: DirectionUpgrade, Stage: StageFlashing}, + } + svc := &fakeLifecycle{ + hasActive: true, + activeTaskInfos: tasks, + waitResult: true, // task 在 timeout 前結束 + } + notifier := &fakeNotifier{} + logger := &fakeLogger{} + + clean := AwaitActiveTasksOrTimeout(context.Background(), svc, notifier, logger) + + if !clean { + t.Errorf("expected cleanShutdown=true when WaitForActiveTasks returns true") + } + if svc.requestShutdown != 1 { + t.Errorf("expected RequestShutdown called once, got %d", svc.requestShutdown) + } + if svc.waitForActive != 1 { + t.Errorf("expected WaitForActiveTasks called once, got %d", svc.waitForActive) + } + if svc.waitForActiveDur != MaxShutdownWait { + t.Errorf("expected wait dur=%v, got %v", MaxShutdownWait, svc.waitForActiveDur) + } + if notifier.calls != 1 { + t.Errorf("expected broadcast called once, got %d", notifier.calls) + } + if notifier.lastRoom != ShutdownRoom { + t.Errorf("expected broadcast room=%q, got %q", ShutdownRoom, notifier.lastRoom) + } + // 驗 broadcast payload schema + payload, ok := notifier.lastData.(map[string]interface{}) + if !ok { + t.Fatalf("broadcast payload not map[string]interface{}: %T", notifier.lastData) + } + if payload["type"] != ShutdownEventTypePending { + t.Errorf("expected payload.type=%q, got %v", ShutdownEventTypePending, payload["type"]) + } + if _, ok := payload["tasks"]; !ok { + t.Errorf("expected payload.tasks key present") + } + // Minor-3：tasks 應為 minimal struct slice、不是 *ActiveTaskInfo + broadcastTasks, ok := payload["tasks"].([]shutdownBroadcastTask) + if !ok { + t.Fatalf("expected payload.tasks to be []shutdownBroadcastTask (without StartTs), got %T", payload["tasks"]) + } + if len(broadcastTasks) != 1 { + t.Fatalf("expected 1 broadcast task, got %d", len(broadcastTasks)) + } + if broadcastTasks[0].TaskID != "upgrade-KL520-0" { + t.Errorf("expected broadcast task TaskID preserved, got %q", broadcastTasks[0].TaskID) + } +} + +// ────────────────────────────────────────────────────────────────────── +// 2a. Minor-3 verification：broadcast payload 不含 startTs 欄位（避免時間戳洩漏） +// ────────────────────────────────────────────────────────────────────── + +func TestAwaitActiveTasksOrTimeout_BroadcastFiltersStartTs(t *testing.T) { + // 帶非零 StartTs 的 task — 驗 broadcast 端確實過濾掉 + now := time.Now() + tasks := []*ActiveTaskInfo{ + { + TaskID: "upgrade-KL720-0", + DeviceID: "kl720-0", + DeviceName: "KL720 dev", + Chip: ChipKL720, + Direction: DirectionUpgrade, + Stage: StageFlashing, + StartTs: now, + ElapsedMs: 5000, + EtaSeconds: 30, + }, + } + svc := &fakeLifecycle{ + hasActive: true, + activeTaskInfos: tasks, + waitResult: true, + } + notifier := &fakeNotifier{} + + _ = AwaitActiveTasksOrTimeout(context.Background(), svc, notifier, nil) + + // 1. broadcast 一定要被呼叫 + if notifier.calls != 1 { + t.Fatalf("expected broadcast called once, got %d", notifier.calls) + } + + // 2. payload tasks 必須是 minimal struct slice + payload, ok := notifier.lastData.(map[string]interface{}) + if !ok { + t.Fatalf("payload not map[string]interface{}: %T", notifier.lastData) + } + broadcastTasks, ok := payload["tasks"].([]shutdownBroadcastTask) + if !ok { + t.Fatalf("expected []shutdownBroadcastTask (no StartTs), got %T — possible regression to *ActiveTaskInfo", payload["tasks"]) + } + if len(broadcastTasks) != 1 { + t.Fatalf("expected 1 task, got %d", len(broadcastTasks)) + } + + // 3. 序列化後驗 JSON 不含 "startTs" key（與直接 marshal *ActiveTaskInfo 對比） + jsonBytes, err := json.Marshal(broadcastTasks[0]) + if err != nil { + t.Fatalf("marshal broadcastTask failed: %v", err) + } + if strings.Contains(string(jsonBytes), "startTs") { + t.Errorf("broadcast payload should NOT contain 'startTs' field; got JSON: %s", string(jsonBytes)) + } + + // 4. 其他欄位確實保留（taskId / deviceId / deviceName / chip / direction / stage / elapsedMs / etaSeconds 七個） + bt := broadcastTasks[0] + if bt.TaskID != "upgrade-KL720-0" { + t.Errorf("TaskID lost: %q", bt.TaskID) + } + if bt.DeviceID != "kl720-0" { + t.Errorf("DeviceID lost: %q", bt.DeviceID) + } + if bt.DeviceName != "KL720 dev" { + t.Errorf("DeviceName lost: %q", bt.DeviceName) + } + if bt.Chip != ChipKL720 { + t.Errorf("Chip lost: %q", bt.Chip) + } + if bt.Direction != DirectionUpgrade { + t.Errorf("Direction lost: %q", bt.Direction) + } + if bt.Stage != StageFlashing { + t.Errorf("Stage lost: %q", bt.Stage) + } + if bt.ElapsedMs != 5000 { + t.Errorf("ElapsedMs lost: %d", bt.ElapsedMs) + } + if bt.EtaSeconds != 30 { + t.Errorf("EtaSeconds lost: %d", bt.EtaSeconds) + } +} + +// ────────────────────────────────────────────────────────────────────── +// 2b. Minor-3 helper：toBroadcastTasks 處理 nil 與空 slice +// ────────────────────────────────────────────────────────────────────── + +func TestToBroadcastTasks_NilAndEmpty(t *testing.T) { + // 空 slice → 回空 slice（非 nil、避免 frontend null） + out := toBroadcastTasks(nil) + if out == nil { + t.Errorf("toBroadcastTasks(nil) should return non-nil slice") + } + if len(out) != 0 { + t.Errorf("toBroadcastTasks(nil) should return empty slice, got %d", len(out)) + } + + // 含 nil pointer 的 slice → 跳過 nil + mixed := []*ActiveTaskInfo{ + nil, + {TaskID: "t1", Chip: ChipKL520}, + nil, + } + out = toBroadcastTasks(mixed) + if len(out) != 1 { + t.Errorf("expected 1 valid task (nil entries filtered), got %d", len(out)) + } + if out[0].TaskID != "t1" { + t.Errorf("expected TaskID=t1, got %q", out[0].TaskID) + } +} + +// ────────────────────────────────────────────────────────────────────── +// 3. 有 active task、timeout → 廣播 1 次、return false、走強制 shutdown +// ────────────────────────────────────────────────────────────────────── + +func TestAwaitActiveTasksOrTimeout_ActiveTaskTimeout(t *testing.T) { + // 縮短 MaxShutdownWait 讓測試快 + origMax := MaxShutdownWait + MaxShutdownWait = 10 * time.Millisecond + defer func() { MaxShutdownWait = origMax }() + + svc := &fakeLifecycle{ + hasActive: true, + activeTaskInfos: []*ActiveTaskInfo{{TaskID: "stuck", Chip: ChipKL720}}, + waitResult: false, // 等不到 task 結束 + } + notifier := &fakeNotifier{} + logger := &fakeLogger{} + + clean := AwaitActiveTasksOrTimeout(context.Background(), svc, notifier, logger) + + if clean { + t.Errorf("expected cleanShutdown=false when WaitForActiveTasks times out") + } + if notifier.calls != 1 { + t.Errorf("expected broadcast called once even on timeout, got %d", notifier.calls) + } + // timeout 應該觸發 Warn log（不是只有 Info） + if len(logger.warnMsgs) == 0 { + t.Errorf("expected at least one Warn log on timeout, got none") + } +} + +// ────────────────────────────────────────────────────────────────────── +// 4. nil service → 防呆 return true（視為「乾淨」、main 繼續走 shutdown） +// ────────────────────────────────────────────────────────────────────── + +func TestAwaitActiveTasksOrTimeout_NilService(t *testing.T) { + clean := AwaitActiveTasksOrTimeout(context.Background(), nil, nil, &fakeLogger{}) + if !clean { + t.Errorf("expected cleanShutdown=true with nil service (defensive default)") + } +} + +// ────────────────────────────────────────────────────────────────────── +// 5. nil notifier / nil logger → 仍正常 return（不 panic） +// ────────────────────────────────────────────────────────────────────── + +func TestAwaitActiveTasksOrTimeout_NilNotifierAndLogger(t *testing.T) { + svc := &fakeLifecycle{hasActive: true, activeTaskInfos: nil, waitResult: true} + + defer func() { + if r := recover(); r != nil { + t.Errorf("panicked with nil notifier/logger: %v", r) + } + }() + + clean := AwaitActiveTasksOrTimeout(context.Background(), svc, nil, nil) + if !clean { + t.Errorf("expected cleanShutdown=true (waitResult=true)") + } + if svc.requestShutdown != 1 { + t.Errorf("expected RequestShutdown still called with nil notifier, got %d", svc.requestShutdown) + } +} + +// ────────────────────────────────────────────────────────────────────── +// 6. 真接整合：用 real *Service 驗 helper 整條走通（不用 fake lifecycle） +// ────────────────────────────────────────────────────────────────────── + +func TestAwaitActiveTasksOrTimeout_RealServiceNoActive(t *testing.T) { + svc := NewService(&fakeLookup{drivers: map[string]UpgradeDriver{}}, FirmwareDir{Root: "/tmp"}) + notifier := &fakeNotifier{} + + clean := AwaitActiveTasksOrTimeout(context.Background(), svc, notifier, nil) + if !clean { + t.Errorf("expected cleanShutdown=true on fresh service with no tasks") + } + if notifier.calls != 0 { + t.Errorf("expected no broadcast on fresh service, got %d", notifier.calls) + } +} diff --git a/local-tool/server/main.go b/local-tool/server/main.go index 7d52694..1886f8f 100644 --- a/local-tool/server/main.go +++ b/local-tool/server/main.go @@ -315,12 +315,35 @@ func main() { Handler: r, } - // Handle OS signals for graceful shutdown + // Handle OS signals for graceful shutdown. + // + // M9-4.5（TDD §8.6.1 + §8.6.3）：firmware-aware shutdown + // 1. 若有 active firmware task（升降版進行中、寫 flash 中）： + // - broadcast `server:shutdown-pending` 給 WebSocket client + // - 呼叫 firmwareSvc.RequestShutdown() 拒絕新 task + // - WaitForActiveTasks(180s) 等到既有 task 結束或 timeout + // - 不論清不清乾淨、最後都走 shutdownFn（避免無限等下去把 process 卡死） + // 2. 若沒 active task：立刻走原本 shutdownFn 流程 + // + // 邏輯本身在 firmware.AwaitActiveTasksOrTimeout 提供（測試友善）、本檔只 + // 負責 wiring。timeout 期間 server 仍在跑（不 accept 新 task 但其他 HTTP + // 照常）、確保 Wails 視窗端能查到 firmware status 才好顯示 modal。 quit := make(chan os.Signal, 1) signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM) go func() { sig := <-quit - logger.Info("Received signal %v, shutting down gracefully...", sig) + logger.Info("Received signal %v, evaluating firmware lifecycle before shutdown...", sig) + + // 等到既有 firmware task 結束（或 hard timeout 180s 後強制走）。 + // 不論 clean 與否、最後都走 shutdownFn——helper 已 log warning。 + _ = firmware.AwaitActiveTasksOrTimeout( + context.Background(), + firmwareSvc, + wsHub, + logger, + ) + + logger.Info("Proceeding with graceful shutdown after firmware checks") shutdownFn() os.Exit(0) }() diff --git a/local-tool/visiona-local/app.go b/local-tool/visiona-local/app.go index e7688c7..3e7770c 100644 --- a/local-tool/visiona-local/app.go +++ b/local-tool/visiona-local/app.go @@ -128,6 +128,11 @@ type App struct { // 成 no-op 或 spy。生產環境 restartStartFn == nil → 走預設的 a.ctrl.Start()。 // test-only；正式環境不應設定。 restartStartFn func() error + + // M9-4.5：firmware-aware close guard（TDD §8.6.2）。 + // 韌體升降版進行中時擋住 OnBeforeClose、emit event 給前端攔截 modal。 + // 由 NewApp 初始化、shutdown 後保留（thread-safe via 自身 mu）。 + firmwareCloseGuard *FirmwareCloseGuard } // NewApp 建立 App 實例。 @@ -147,7 +152,8 @@ func NewApp() *App { } // R5-5a：沒插硬體就顯示空白狀態（由 UI 處理），一律真實硬體路徑 return &App{ - pythonMode: mode, + pythonMode: mode, + firmwareCloseGuard: NewFirmwareCloseGuard(), } } diff --git a/local-tool/visiona-local/firmware_close_guard.go b/local-tool/visiona-local/firmware_close_guard.go new file mode 100644 index 0000000..18f0d6f --- /dev/null +++ b/local-tool/visiona-local/firmware_close_guard.go @@ -0,0 +1,244 @@ +package main + +// firmware_close_guard.go — M9-4.5 (TDD §8.6.2)：Wails 控制台關閉攔截 +// +// 為什麼有這個檔： +// R5-2 設定「關 Wails 視窗 = 結束 server」、預設 OnBeforeClose return false +// 讓 Wails 進 OnShutdown 流程；但韌體升降版進行中（會寫 flash）中斷 = device +// brick。本檔提供 firmware-aware close guard、在 close 動作前查 server 是否 +// 有 active firmware task： +// +// 1. 有 active task → emit Wails event `app:firmware-in-progress` 給前端 +// modal、return true（prevent close）。前端 modal（Design §6a）顯示 +// 進度 / ETA / 「繼續等待」/「強制關閉」按鈕。 +// 2. 沒 active task → return false、走原本 R5-2 流程 +// +// 前端「強制關閉」按鈕（經 §6a.5 第二層 FORCE 確認後）會呼叫 App 的 +// `ConfirmForceClose` binding、binding 內 set forceCloseAccepted=true、再 +// 觸發 Wails Quit、下次 OnBeforeClose 直接 return false 放行。 +// +// 為什麼用 Wails event 而不是 native dialog： +// Design §6a 規格要求 modal 與前端設計系統一致（color tokens / focus +// management / i18n / 二層 FORCE 確認）、Wails native dialog 風格不一致也 +// 無法做二層確認字串輸入。改用前端 modal、Wails 端只負責 emit event + +// 接收 binding 回應。 + +import ( + "context" + "fmt" + "sync" + + wailsRuntime "github.com/wailsapp/wails/v2/pkg/runtime" +) + +// firmwareInProgressEvent 是 emit 給前端的事件型別（對齊 TDD §8.6.2）。 +// 前端訂閱此事件、收到後渲染 Design §6a 的攔截 modal。 +const firmwareInProgressEvent = "app:firmware-in-progress" + +// FirmwareCloseGuard 封裝關閉攔截邏輯（含 forceCloseAccepted 狀態）。 +// 嵌進 App 的 method 是 OnBeforeClose / ConfirmForceClose 兩個 wrapper。 +// +// 拆出獨立 struct 是為了：(1) 測試友善（可不靠 App 全域 state）；(2) +// thread-safe — Wails OnBeforeClose 由 main thread 呼叫、ConfirmForceClose +// 由 binding goroutine 呼叫、必須 mu 保護。 +type FirmwareCloseGuard struct { + mu sync.Mutex + + // forceCloseAccepted：使用者已通過 Design §6a 第二層 FORCE 確認、下次 + // OnBeforeClose 被叫到時直接放行。confirmForceClose() 設、放行後自動清。 + forceCloseAccepted bool +} + +// NewFirmwareCloseGuard 建空 guard。 +func NewFirmwareCloseGuard() *FirmwareCloseGuard { + return &FirmwareCloseGuard{} +} + +// ConfirmForceClose 由前端「強制關閉」按鈕（§6a.5 第二層 FORCE 確認後）呼叫。 +// 設旗標、下次 OnBeforeClose 直接 return false 放行。 +func (g *FirmwareCloseGuard) ConfirmForceClose() { + g.mu.Lock() + g.forceCloseAccepted = true + g.mu.Unlock() +} + +// consumeForceCloseAccepted 取出旗標（讀完即清）、給 OnBeforeClose 判斷用。 +// 抽出 method 方便測試直接觀察狀態。 +func (g *FirmwareCloseGuard) consumeForceCloseAccepted() bool { + g.mu.Lock() + defer g.mu.Unlock() + if g.forceCloseAccepted { + g.forceCloseAccepted = false + return true + } + return false +} + +// CloseGuardDeps 是 evaluateClose 對外部依賴的介面（測試友善）。 +// +// 實作方：production = App（透過 GetServerStatus 拿 port + emit event 用 ctx）； +// test = fake 提供確定的 port + 觀察 emit。 +type CloseGuardDeps interface { + // ServerPort 回傳 server 目前 listen 的 port、<=0 表示 server 沒起來。 + ServerPort() int + + // QueryFirmwareTasks 查詢 server 是否有 active firmware task。 + // production 直接打到 queryFirmwareActiveTasks(ctx, port)。 + QueryFirmwareTasks(ctx context.Context, port int) (FirmwareActiveTasksResponse, error) + + // EmitFirmwareInProgress 通知前端「韌體進行中、不能關」。 + // production = wailsRuntime.EventsEmit(ctx, firmwareInProgressEvent, payload)。 + EmitFirmwareInProgress(payload map[string]interface{}) + + // AppLog 寫 wails.log。production = App.appLog；test 用 no-op。 + AppLog(format string, args ...interface{}) +} + +// evaluateClose 是 OnBeforeClose 的核心邏輯（不直接接 Wails ctx、用 CloseGuardDeps +// 抽象出來方便測試）。 +// +// 回傳 prevent=true → Wails 應該擋住關閉（保留視窗）； +// 回傳 prevent=false → 走原本 R5-2 流程（OnShutdown → ctrl.Stop → 結束 server）。 +// +// 邏輯流程： +// +// 1. forceCloseAccepted=true → 重置 + return false 放行（使用者已通過二層確認） +// 2. ServerPort()<=0 → return false（server 沒起來、沒韌體任務可擋） +// 3. queryFirmwareTasks 回 hasActive=false 或錯誤 → return false（fail-open） +// 4. hasActive=true → emit `app:firmware-in-progress` event + return true +func (g *FirmwareCloseGuard) evaluateClose(ctx context.Context, deps CloseGuardDeps) bool { + if deps == nil { + // 防呆：deps 沒注入、不擋（避免使用者卡住） + return false + } + + // 1. 使用者已通過二層確認、放行 + if g.consumeForceCloseAccepted() { + deps.AppLog("close-guard: force-close confirmed by user, allowing close") + return false + } + + port := deps.ServerPort() + if port <= 0 { + // server 沒起來、沒韌體任務可擋 + deps.AppLog("close-guard: server not running (port=%d), allowing close", port) + return false + } + + // 2. 查 server firmware active task + res, err := deps.QueryFirmwareTasks(ctx, port) + if err != nil { + // fail-open：查不到狀態不擋（server 端 SIGTERM handler 還會擋一層） + deps.AppLog("close-guard: query firmware tasks failed (fail-open allow close): %v", err) + return false + } + + if !res.HasActive { + deps.AppLog("close-guard: no active firmware task, allowing close") + return false + } + + // 3. 有 active task：emit event 通知前端 modal、prevent close + tasksAny := make([]interface{}, 0, len(res.Tasks)) + for _, t := range res.Tasks { + tasksAny = append(tasksAny, map[string]interface{}{ + "taskId": t.TaskID, + "deviceId": t.DeviceID, + "deviceName": t.DeviceName, + "chip": t.Chip, + "direction": t.Direction, + "stage": t.Stage, + "elapsedMs": t.ElapsedMs, + "etaSeconds": t.EtaSeconds, + }) + } + payload := map[string]interface{}{ + "hasActive": true, + "tasks": tasksAny, + } + deps.EmitFirmwareInProgress(payload) + deps.AppLog("close-guard: %d active firmware task(s) detected, preventing close + emitted %s event", len(res.Tasks), firmwareInProgressEvent) + return true +} + +// ────────────────────────────────────────────────────────────────────── +// App 層 wiring（將 App 的 method 適配進 CloseGuardDeps） +// ────────────────────────────────────────────────────────────────────── + +// appCloseGuardDeps 將 *App 包裝成 CloseGuardDeps。production wire-up。 +type appCloseGuardDeps struct { + app *App +} + +func (a *appCloseGuardDeps) ServerPort() int { + if a.app == nil { + return 0 + } + st := a.app.GetServerStatus() + if !st.Running { + return 0 + } + return st.Port +} + +func (a *appCloseGuardDeps) QueryFirmwareTasks(ctx context.Context, port int) (FirmwareActiveTasksResponse, error) { + return queryFirmwareActiveTasks(ctx, port) +} + +func (a *appCloseGuardDeps) EmitFirmwareInProgress(payload map[string]interface{}) { + if a.app == nil || a.app.ctx == nil { + return + } + wailsRuntime.EventsEmit(a.app.ctx, firmwareInProgressEvent, payload) +} + +func (a *appCloseGuardDeps) AppLog(format string, args ...interface{}) { + if a.app == nil { + return + } + a.app.appLog(format, args...) +} + +// OnBeforeClose 是給 Wails options.App.OnBeforeClose 用的 callback。 +// M9-4.5 之前：永遠 return false。 +// M9-4.5 之後：firmware-aware（見 evaluateClose）。 +// +// 由 App 持有的 firmwareCloseGuard 提供 state；Wails 在 main thread 呼叫、 +// CloseGuard 的 mu 保護 ConfirmForceClose 與 OnBeforeClose 的併發。 +func (a *App) OnBeforeClose(ctx context.Context) bool { + if a.firmwareCloseGuard == nil { + // 防呆：guard 沒初始化（極早期啟動 / test 直接 new App）→ 走預設不擋 + return false + } + return a.firmwareCloseGuard.evaluateClose(ctx, &appCloseGuardDeps{app: a}) +} + +// ConfirmForceClose 是 Wails binding、給前端 Design §6a 第二層 FORCE 確認 +// modal 的「確認強制關閉」按鈕呼叫。 +// +// 行為： +// 1. 設 forceCloseAccepted=true +// 2. 立刻呼叫 wailsRuntime.Quit(a.ctx) 觸發關閉流程 +// 3. Wails 之後叫 OnBeforeClose → 看到旗標 → 直接放行 → 走 OnShutdown +// → ctrl.Stop() 既有 7+1s graceful shutdown +// +// 為什麼不直接 ForceKill：使用者「確認強制關閉」的意圖是「我知道風險、繼續 +// 走、但仍想優雅關閉 server 業務（會 broadcast offline overlay 給其他 tab）」、 +// graceful shutdown 流程比 SIGKILL 更友善。server 端 firmware-aware SIGTERM +// handler 收到 SIGTERM 後 ${firmware.MaxShutdownWait}s 內仍會等 firmware task、 +// 真的卡死才強制走 — 屬於雙層防護的合理取捨。 +// +// 回傳 error 給 binding caller 觀察（目前永遠 nil、預留未來擴展）。 +func (a *App) ConfirmForceClose() error { + if a.firmwareCloseGuard == nil { + return fmt.Errorf("firmware close guard not initialized") + } + a.firmwareCloseGuard.ConfirmForceClose() + a.appLog("ConfirmForceClose: user accepted force close, scheduling Wails Quit") + if a.ctx != nil { + // 用 goroutine 避免 binding caller 阻塞、且讓 Wails event loop 自由 + // 處理 close 流程（OnBeforeClose 會被再叫一次、看到旗標放行） + go wailsRuntime.Quit(a.ctx) + } + return nil +} diff --git a/local-tool/visiona-local/firmware_close_guard_test.go b/local-tool/visiona-local/firmware_close_guard_test.go new file mode 100644 index 0000000..b6d475e --- /dev/null +++ b/local-tool/visiona-local/firmware_close_guard_test.go @@ -0,0 +1,280 @@ +package main + +// firmware_close_guard_test.go — M9-4.5：Wails OnBeforeClose firmware-aware 攔截邏輯測試 +// +// 覆蓋情境： +// 1. forceCloseAccepted=true → 放行（return false）、旗標被清掉 +// 2. server port=0 → 放行 +// 3. queryFirmwareTasks 失敗 → fail-open 放行 +// 4. hasActive=false → 放行、不 emit event +// 5. hasActive=true → emit event + return true（prevent）、payload schema 正確 +// 6. ConfirmForceClose → 設旗標 + 下次 evaluateClose 放行 +// 7. nil deps → 防呆放行 +// 8. concurrent ConfirmForceClose 與 evaluateClose → race-free + +import ( + "context" + "errors" + "sync" + "sync/atomic" + "testing" +) + +// fakeCloseGuardDeps 是 CloseGuardDeps 的 test double。 +type fakeCloseGuardDeps struct { + mu sync.Mutex + + port int + queryResp FirmwareActiveTasksResponse + queryErr error + queryCalls int + emitCalls int + lastEmitPayload map[string]interface{} + logLines []string +} + +func (f *fakeCloseGuardDeps) ServerPort() int { + f.mu.Lock() + defer f.mu.Unlock() + return f.port +} + +func (f *fakeCloseGuardDeps) QueryFirmwareTasks(ctx context.Context, port int) (FirmwareActiveTasksResponse, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.queryCalls++ + return f.queryResp, f.queryErr +} + +func (f *fakeCloseGuardDeps) EmitFirmwareInProgress(payload map[string]interface{}) { + f.mu.Lock() + defer f.mu.Unlock() + f.emitCalls++ + f.lastEmitPayload = payload +} + +func (f *fakeCloseGuardDeps) AppLog(format string, args ...interface{}) { + f.mu.Lock() + defer f.mu.Unlock() + f.logLines = append(f.logLines, format) +} + +// ────────────────────────────────────────────────────────────────────── +// 1. forceCloseAccepted=true → 放行、旗標清空 +// ────────────────────────────────────────────────────────────────────── + +func TestEvaluateClose_ForceAccepted(t *testing.T) { + g := NewFirmwareCloseGuard() + g.ConfirmForceClose() + + deps := &fakeCloseGuardDeps{ + port: 8080, + queryResp: FirmwareActiveTasksResponse{HasActive: true}, // 即使有 active task 也該放行 + } + + prevent := g.evaluateClose(context.Background(), deps) + if prevent { + t.Errorf("expected prevent=false when force accepted, got true") + } + // queryFirmwareTasks 不該被叫（force-close 短路） + if deps.queryCalls != 0 { + t.Errorf("expected queryFirmwareTasks NOT called on force accept, got %d", deps.queryCalls) + } + if deps.emitCalls != 0 { + t.Errorf("expected no emit on force accept, got %d", deps.emitCalls) + } + + // 旗標應該已被清掉、下次呼叫如沒 active 才放行 + deps.queryResp = FirmwareActiveTasksResponse{HasActive: true, Tasks: []FirmwareActiveTaskSummary{{TaskID: "x"}}} + prevent2 := g.evaluateClose(context.Background(), deps) + if !prevent2 { + t.Errorf("expected prevent=true on 2nd call (flag cleared), got false") + } +} + +// ────────────────────────────────────────────────────────────────────── +// 2. server port<=0 → 放行 +// ────────────────────────────────────────────────────────────────────── + +func TestEvaluateClose_ServerNotRunning(t *testing.T) { + g := NewFirmwareCloseGuard() + deps := &fakeCloseGuardDeps{port: 0} + prevent := g.evaluateClose(context.Background(), deps) + if prevent { + t.Errorf("expected prevent=false when server not running") + } + if deps.queryCalls != 0 { + t.Errorf("expected queryFirmwareTasks NOT called when port=0, got %d", deps.queryCalls) + } +} + +// ────────────────────────────────────────────────────────────────────── +// 3. queryFirmwareTasks 失敗 → fail-open 放行 +// ────────────────────────────────────────────────────────────────────── + +func TestEvaluateClose_QueryError_FailOpen(t *testing.T) { + g := NewFirmwareCloseGuard() + deps := &fakeCloseGuardDeps{ + port: 3721, + queryErr: errors.New("connection refused"), + } + prevent := g.evaluateClose(context.Background(), deps) + if prevent { + t.Errorf("expected prevent=false (fail-open) on query error, got true") + } + if deps.queryCalls != 1 { + t.Errorf("expected queryFirmwareTasks called once, got %d", deps.queryCalls) + } + if deps.emitCalls != 0 { + t.Errorf("expected no emit on query error, got %d", deps.emitCalls) + } +} + +// ────────────────────────────────────────────────────────────────────── +// 4. hasActive=false → 放行、不 emit +// ────────────────────────────────────────────────────────────────────── + +func TestEvaluateClose_NoActiveTask(t *testing.T) { + g := NewFirmwareCloseGuard() + deps := &fakeCloseGuardDeps{ + port: 3721, + queryResp: FirmwareActiveTasksResponse{HasActive: false, Tasks: []FirmwareActiveTaskSummary{}}, + } + prevent := g.evaluateClose(context.Background(), deps) + if prevent { + t.Errorf("expected prevent=false when no active task") + } + if deps.emitCalls != 0 { + t.Errorf("expected no emit when no active task, got %d", deps.emitCalls) + } +} + +// ────────────────────────────────────────────────────────────────────── +// 5. hasActive=true → emit event + return true、payload schema 正確 +// ────────────────────────────────────────────────────────────────────── + +func TestEvaluateClose_HasActive_PreventAndEmit(t *testing.T) { + g := NewFirmwareCloseGuard() + deps := &fakeCloseGuardDeps{ + port: 3721, + queryResp: FirmwareActiveTasksResponse{ + HasActive: true, + Tasks: []FirmwareActiveTaskSummary{ + { + TaskID: "upgrade-KL520-0", + DeviceID: "kl520-0", + DeviceName: "KL520 #1", + Chip: "KL520", + Direction: "upgrade", + Stage: "flashing", + ElapsedMs: 12000, + EtaSeconds: 45, + }, + }, + }, + } + prevent := g.evaluateClose(context.Background(), deps) + if !prevent { + t.Errorf("expected prevent=true on active task") + } + if deps.emitCalls != 1 { + t.Errorf("expected emit called once, got %d", deps.emitCalls) + } + // payload schema：hasActive + tasks(slice) 必須存在 + if deps.lastEmitPayload["hasActive"] != true { + t.Errorf("expected payload.hasActive=true, got %v", deps.lastEmitPayload["hasActive"]) + } + tasksAny, ok := deps.lastEmitPayload["tasks"].([]interface{}) + if !ok { + t.Fatalf("expected payload.tasks []interface{}, got %T", deps.lastEmitPayload["tasks"]) + } + if len(tasksAny) != 1 { + t.Fatalf("expected 1 task in payload, got %d", len(tasksAny)) + } + task, ok := tasksAny[0].(map[string]interface{}) + if !ok { + t.Fatalf("expected task is map, got %T", tasksAny[0]) + } + // 重要欄位 frontend modal 會用 + if task["taskId"] != "upgrade-KL520-0" { + t.Errorf("expected taskId, got %v", task["taskId"]) + } + if task["deviceName"] != "KL520 #1" { + t.Errorf("expected deviceName, got %v", task["deviceName"]) + } + if task["chip"] != "KL520" { + t.Errorf("expected chip, got %v", task["chip"]) + } + if task["stage"] != "flashing" { + t.Errorf("expected stage, got %v", task["stage"]) + } + if task["etaSeconds"] != 45 { + t.Errorf("expected etaSeconds=45, got %v", task["etaSeconds"]) + } +} + +// ────────────────────────────────────────────────────────────────────── +// 6. ConfirmForceClose 設旗標 +// ────────────────────────────────────────────────────────────────────── + +func TestConfirmForceClose_SetsAndConsumesFlag(t *testing.T) { + g := NewFirmwareCloseGuard() + if g.consumeForceCloseAccepted() { + t.Errorf("expected default flag=false") + } + g.ConfirmForceClose() + if !g.consumeForceCloseAccepted() { + t.Errorf("expected flag=true after ConfirmForceClose") + } + // consume 後旗標清掉 + if g.consumeForceCloseAccepted() { + t.Errorf("expected flag=false after consume") + } +} + +// ────────────────────────────────────────────────────────────────────── +// 7. nil deps → 防呆放行 +// ────────────────────────────────────────────────────────────────────── + +func TestEvaluateClose_NilDeps(t *testing.T) { + g := NewFirmwareCloseGuard() + prevent := g.evaluateClose(context.Background(), nil) + if prevent { + t.Errorf("expected prevent=false with nil deps (defensive)") + } +} + +// ────────────────────────────────────────────────────────────────────── +// 8. concurrent ConfirmForceClose 與 evaluateClose 不 race（-race 模式） +// ────────────────────────────────────────────────────────────────────── + +func TestConfirmForceClose_ConcurrentAccess(t *testing.T) { + g := NewFirmwareCloseGuard() + deps := &fakeCloseGuardDeps{port: 3721, queryResp: FirmwareActiveTasksResponse{HasActive: false}} + + const N = 100 + var wg sync.WaitGroup + var preventCount, allowCount int64 + + for i := 0; i < N; i++ { + wg.Add(2) + go func() { + defer wg.Done() + g.ConfirmForceClose() + }() + go func() { + defer wg.Done() + prevent := g.evaluateClose(context.Background(), deps) + if prevent { + atomic.AddInt64(&preventCount, 1) + } else { + atomic.AddInt64(&allowCount, 1) + } + }() + } + wg.Wait() + + // 不檢查具體 prevent/allow 數量（race condition between Confirm + Evaluate + // 順序不可預期）、只驗 -race 模式沒抓到 race + t.Logf("concurrent stress: prevent=%d allow=%d (race-free under -race)", preventCount, allowCount) +} diff --git a/local-tool/visiona-local/main.go b/local-tool/visiona-local/main.go index 74e4c66..fff64f8 100644 --- a/local-tool/visiona-local/main.go +++ b/local-tool/visiona-local/main.go @@ -1,7 +1,6 @@ package main import ( - "context" "embed" "github.com/wailsapp/wails/v2" @@ -26,12 +25,17 @@ func main() { }, OnStartup: app.startup, OnShutdown: app.shutdown, - // M8-4 / R5-2：關視窗 = 結束 server + 結束 app。 - // 不 hide-to-tray、不彈確認對話框。回 false 允許 Wails 繼續走 OnShutdown 流程。 - // OnShutdown 會呼叫 ctrl.Stop()（7 秒 grace + 1 秒 modal）。 - OnBeforeClose: func(ctx context.Context) (prevent bool) { - return false - }, + // M8-4 / R5-2：關視窗 = 結束 server + 結束 app（預設行為）。 + // 不 hide-to-tray、不彈系統 native 對話框。 + // + // M9-4.5（TDD §8.6.2）：firmware-aware close guard。 + // - 韌體升降版進行中 → app.OnBeforeClose 回 true、擋住關閉、emit + // `app:firmware-in-progress` event 給前端 Design §6a 攔截 modal + // - 使用者通過 §6a 第二層 FORCE 確認 → 前端呼叫 ConfirmForceClose + // binding → 設旗標 → 觸發 Wails Quit → OnBeforeClose 再被叫 → 放行 + // - 沒韌體任務 → 直接 return false、走 OnShutdown → ctrl.Stop() + // 既有 7+1s graceful shutdown + OnBeforeClose: app.OnBeforeClose, Bind: []interface{}{ app, }, diff --git a/local-tool/visiona-local/query_firmware_active_tasks.go b/local-tool/visiona-local/query_firmware_active_tasks.go new file mode 100644 index 0000000..9f4d923 --- /dev/null +++ b/local-tool/visiona-local/query_firmware_active_tasks.go @@ -0,0 +1,111 @@ +package main + +// query_firmware_active_tasks.go — M9-4.5：firmware-aware close handler helper +// +// Wails 控制台收到使用者關閉視窗請求時、必須先查 server 是否有韌體升降版進 +// 行中（會寫 flash 的破壞性操作、中斷會 brick）。本 helper 提供小型 HTTP 客戶 +// 端、打 `GET /api/firmware/active-tasks`（M9-3 端點）、解析 hasActive + +// tasks 給 OnBeforeClose 做判斷。 +// +// 設計原則（仿 shutdown_notify.go）： +// - 1 秒 timeout：server 沒起來 / 卡死 / network error 全視為「無 active task」 +// 不擋關閉、避免使用者卡在「關不掉的視窗」 +// - port <= 0 → server 還沒起來、直接視為「無 active task」乾淨關 +// - 任何錯誤、不擋關閉路徑（fail-open）：寧可錯放也不卡使用者 +// - 變數化 client / timeout 以便測試注入 +// +// 為什麼 fail-open：誤判「沒 active task」最壞情況是進到既有 OnShutdown +// 的 ctrl.Stop() → server SIGTERM handler、那邊 firmware.AwaitActiveTasksOrTimeout +// 還會再擋一層；誤判「有 active task」要使用者強制關閉 = 體驗很差。雙層 +// 防護下 Wails 這層 fail-open 是合理取捨。 + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "time" +) + +// 變數化以便測試注入。 +var ( + queryFirmwareTasksTimeout = 1 * time.Second + queryFirmwareTasksClient = &http.Client{Timeout: queryFirmwareTasksTimeout} +) + +// FirmwareActiveTaskSummary 是 OnBeforeClose 用來顯示 modal 所需的最小欄位。 +// 對齊 server 端 firmware.ActiveTaskInfo 的 JSON 欄位（snake → camel 由 server +// 端 jsonTag 統一）。 +type FirmwareActiveTaskSummary struct { + TaskID string `json:"taskId"` + DeviceID string `json:"deviceId"` + DeviceName string `json:"deviceName,omitempty"` + Chip string `json:"chip"` + Direction string `json:"direction"` + Stage string `json:"stage"` + ElapsedMs int64 `json:"elapsedMs"` + EtaSeconds int `json:"etaSeconds,omitempty"` +} + +// FirmwareActiveTasksResponse 對應 server 端 GET /api/firmware/active-tasks +// 回應的 `data` 欄位（server 包裝 {success, data:{hasActive, tasks}}、本 struct +// 只描述 data 內層）。 +type FirmwareActiveTasksResponse struct { + HasActive bool `json:"hasActive"` + Tasks []FirmwareActiveTaskSummary `json:"tasks"` +} + +// queryFirmwareActiveTasks 打 GET /api/firmware/active-tasks、回傳 hasActive +// + 任務清單。 +// +// 錯誤路徑（fail-open）：任何錯誤回傳 hasActive=false + 空 tasks + 該 error、 +// caller 可選擇 log（不強制）、但**不**應該據此擋關閉。 +// +// 參數： +// +// ctx — caller context（會被 timeout 包裹） +// port — server 正在聽的 port；<= 0 代表 server 沒起來、直接回 hasActive=false +// +// 回傳 error 主要供 caller 寫 log debug 用；正常 fail-open 邏輯不檢查 error。 +func queryFirmwareActiveTasks(ctx context.Context, port int) (FirmwareActiveTasksResponse, error) { + empty := FirmwareActiveTasksResponse{HasActive: false, Tasks: nil} + if port <= 0 { + return empty, fmt.Errorf("server port not available") + } + if ctx == nil { + ctx = context.Background() + } + reqCtx, cancel := context.WithTimeout(ctx, queryFirmwareTasksTimeout) + defer cancel() + + url := fmt.Sprintf("http://127.0.0.1:%d/api/firmware/active-tasks", port) + req, err := http.NewRequestWithContext(reqCtx, http.MethodGet, url, nil) + if err != nil { + return empty, fmt.Errorf("build request: %w", err) + } + resp, err := queryFirmwareTasksClient.Do(req) + if err != nil { + return empty, fmt.Errorf("http get: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return empty, fmt.Errorf("unexpected status %d", resp.StatusCode) + } + + // server 包了一層 {success, data:{...}}、本 helper 只取 data + var wrapper struct { + Success bool `json:"success"` + Data FirmwareActiveTasksResponse `json:"data"` + } + if err := json.NewDecoder(resp.Body).Decode(&wrapper); err != nil { + return empty, fmt.Errorf("decode response: %w", err) + } + if !wrapper.Success { + return empty, fmt.Errorf("server returned success=false") + } + if wrapper.Data.Tasks == nil { + wrapper.Data.Tasks = []FirmwareActiveTaskSummary{} + } + return wrapper.Data, nil +} diff --git a/local-tool/visiona-local/query_firmware_active_tasks_test.go b/local-tool/visiona-local/query_firmware_active_tasks_test.go new file mode 100644 index 0000000..6281fbf --- /dev/null +++ b/local-tool/visiona-local/query_firmware_active_tasks_test.go @@ -0,0 +1,250 @@ +package main + +// query_firmware_active_tasks_test.go — M9-4.5：firmware 查詢 helper 測試 +// +// 覆蓋情境： +// 1. port <= 0 → 立即 return hasActive=false（fail-open 預設值） +// 2. server 正常回 hasActive=true + tasks → 解析正確 +// 3. server 正常回 hasActive=false + 空 tasks → tasks 非 nil（避免 caller 處理 nil panic） +// 4. server 回 500 → fail-open（hasActive=false）+ error +// 5. server timeout → fail-open + error（不卡 Wails 關閉流程） +// 6. server 回非預期 JSON → fail-open + error + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "testing" + "time" +) + +func portFromTestServerURL(t *testing.T, raw string) int { + t.Helper() + u, err := url.Parse(raw) + if err != nil { + t.Fatalf("parse test server url: %v", err) + } + p, err := strconv.Atoi(u.Port()) + if err != nil { + t.Fatalf("port atoi: %v", err) + } + return p +} + +// ────────────────────────────────────────────────────────────────────── +// 1. port <= 0 → 立即 fail-open +// ────────────────────────────────────────────────────────────────────── + +func TestQueryFirmwareActiveTasks_PortZero(t *testing.T) { + res, err := queryFirmwareActiveTasks(context.Background(), 0) + if err == nil { + t.Errorf("expected error when port=0") + } + if res.HasActive { + t.Errorf("expected hasActive=false on port=0") + } +} + +func TestQueryFirmwareActiveTasks_PortNegative(t *testing.T) { + res, err := queryFirmwareActiveTasks(context.Background(), -1) + if err == nil { + t.Errorf("expected error when port<0") + } + if res.HasActive { + t.Errorf("expected hasActive=false on port<0") + } +} + +// ────────────────────────────────────────────────────────────────────── +// 2. server 正常回 hasActive=true + tasks +// ────────────────────────────────────────────────────────────────────── + +func TestQueryFirmwareActiveTasks_HasActiveWithTasks(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/firmware/active-tasks" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "success": true, + "data": map[string]interface{}{ + "hasActive": true, + "tasks": []map[string]interface{}{ + { + "taskId": "upgrade-KL520-0", + "deviceId": "kl520-0", + "deviceName": "KL520 #1", + "chip": "KL520", + "direction": "upgrade", + "stage": "flashing", + "elapsedMs": int64(12000), + "etaSeconds": 45, + }, + }, + }, + }) + })) + defer srv.Close() + + port := portFromTestServerURL(t, srv.URL) + res, err := queryFirmwareActiveTasks(context.Background(), port) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !res.HasActive { + t.Errorf("expected hasActive=true") + } + if len(res.Tasks) != 1 { + t.Fatalf("expected 1 task, got %d", len(res.Tasks)) + } + tk := res.Tasks[0] + if tk.TaskID != "upgrade-KL520-0" || tk.DeviceID != "kl520-0" || tk.Chip != "KL520" { + t.Errorf("task decoded wrong: %+v", tk) + } + if tk.Direction != "upgrade" || tk.Stage != "flashing" { + t.Errorf("task direction/stage decoded wrong: %+v", tk) + } + if tk.EtaSeconds != 45 { + t.Errorf("expected etaSeconds=45, got %d", tk.EtaSeconds) + } +} + +// ────────────────────────────────────────────────────────────────────── +// 3. server 回 hasActive=false + null tasks → tasks 非 nil +// ────────────────────────────────────────────────────────────────────── + +func TestQueryFirmwareActiveTasks_HasActiveFalseNullTasks(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "success": true, + "data": map[string]interface{}{ + "hasActive": false, + "tasks": nil, + }, + }) + })) + defer srv.Close() + + port := portFromTestServerURL(t, srv.URL) + res, err := queryFirmwareActiveTasks(context.Background(), port) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res.HasActive { + t.Errorf("expected hasActive=false") + } + if res.Tasks == nil { + t.Errorf("expected Tasks to be non-nil (empty slice), got nil") + } +} + +// ────────────────────────────────────────────────────────────────────── +// 4. server 回 500 → fail-open +// ────────────────────────────────────────────────────────────────────── + +func TestQueryFirmwareActiveTasks_ServerError500(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = fmt.Fprintln(w, "internal error") + })) + defer srv.Close() + + port := portFromTestServerURL(t, srv.URL) + res, err := queryFirmwareActiveTasks(context.Background(), port) + if err == nil { + t.Errorf("expected error on 500") + } + if res.HasActive { + t.Errorf("expected hasActive=false on 500 (fail-open)") + } +} + +// ────────────────────────────────────────────────────────────────────── +// 5. server timeout → fail-open + error +// ────────────────────────────────────────────────────────────────────── + +func TestQueryFirmwareActiveTasks_Timeout(t *testing.T) { + // 縮短 helper timeout + origTimeout := queryFirmwareTasksTimeout + origClient := queryFirmwareTasksClient + queryFirmwareTasksTimeout = 100 * time.Millisecond + queryFirmwareTasksClient = &http.Client{Timeout: queryFirmwareTasksTimeout} + defer func() { + queryFirmwareTasksTimeout = origTimeout + queryFirmwareTasksClient = origClient + }() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(500 * time.Millisecond) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + port := portFromTestServerURL(t, srv.URL) + done := make(chan struct{}) + go func() { + res, err := queryFirmwareActiveTasks(context.Background(), port) + if err == nil { + t.Errorf("expected timeout error") + } + if res.HasActive { + t.Errorf("expected hasActive=false on timeout (fail-open)") + } + close(done) + }() + + select { + case <-done: + // ok + case <-time.After(2 * time.Second): + t.Fatalf("queryFirmwareActiveTasks blocked > 2s, timeout not respected") + } +} + +// ────────────────────────────────────────────────────────────────────── +// 6. server 回非預期 JSON → fail-open + error +// ────────────────────────────────────────────────────────────────────── + +func TestQueryFirmwareActiveTasks_MalformedJSON(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte("{this is not json")) + })) + defer srv.Close() + + port := portFromTestServerURL(t, srv.URL) + res, err := queryFirmwareActiveTasks(context.Background(), port) + if err == nil { + t.Errorf("expected decode error on malformed JSON") + } + if res.HasActive { + t.Errorf("expected hasActive=false on decode error (fail-open)") + } +} + +// ────────────────────────────────────────────────────────────────────── +// 7. success=false 即使 200 → 視為 error +// ────────────────────────────────────────────────────────────────────── + +func TestQueryFirmwareActiveTasks_SuccessFalse(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "success": false, + "data": map[string]interface{}{"hasActive": false}, + }) + })) + defer srv.Close() + + port := portFromTestServerURL(t, srv.URL) + res, err := queryFirmwareActiveTasks(context.Background(), port) + if err == nil { + t.Errorf("expected error when success=false") + } + if res.HasActive { + t.Errorf("expected hasActive=false (fail-open)") + } +}