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) <noreply@anthropic.com>
25 KiB
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-rejectedevent broadcast 給所有訂閱者」、限定firmware:room - TDD §8.6.4:「event 命名
firmware:shutdown-rejected限定 firmware: room、其他 room 不收」 - 實作(shutdown.go:66-77):用
systemroom、event typeserver:shutdown-pending
實作的選擇其實更合理(system room 已存在 server:shutdown-imminent 配對、前端有 useShutdownWatcher hook 訂閱、不需要叫前端再去訂閱所有 firmware:<device> 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-shutdownIPC 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):
- WS event:把 TDD §8.6.1 / §8.6.4 改成
server:shutdown-pending在systemroom(對齊既有 R5-2server:shutdown-imminent配對)、補一行說明這跟firmware:<device>progress room 區隔 - 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:
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
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 內 ✅ - 跟
shutdownReqflag 互動: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.broadcastchannel 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 從systemroom 改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 正面回饋)
- interface 拆分乾淨:ShutdownNotifier / FirmwareLifecycle / ShutdownLogger 三 interface 都是 minimum surface、測試友善、production wire-up 簡單。雖然有 S-1 略過度抽象的疑慮、但拆得很好。
- 雙層保護設計合理:Wails query fail-open → server SIGTERM handler 雙層擋、註解(query_firmware_active_tasks.go:17-20)清楚說明取捨理由。這比硬擋 Wails 體驗好太多。
- 註解品質極高:shutdown.go 開頭 24 行設計取捨 + 為什麼 180s 不是 200s、firmware_close_guard.go 行 1-24 解釋為什麼用 Wails event 而不是 native dialog、ConfirmForceClose binding goroutine 解釋(line 219-229)——全部都是「下一個工程師讀到不會踩坑」級的註解。
- N=100 -race 並發測試:firmware_close_guard_test.go:251-280 用 atomic counter 跑 N=100、認真寫 race-free 驗證、不是隨便糊個 test 交差。難得。
- fail-open 設計守則一致:query / OnBeforeClose / AwaitActiveTasksOrTimeout 全部 nil-safe、所有路徑都假設「最壞情況不該 panic、不該卡死 Wails」、防禦性編碼到位。
- 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)