visionA/local-tool/.autoflow/05-implementation/review/m9-4.5-shutdown-integration-review.md
jim800121chen ff9bbc81ed feat(local-tool): M9-4.5 — server SIGTERM + Wails OnBeforeClose firmware-aware shutdown
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>
2026-05-25 15:07:29 +08:00

25 KiB
Raw Permalink Blame History

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-1event 名 + 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 設計都乾淨。
  • Concurrencymu 保護完整、forceCloseAccepted race-free、broadcast hub 是 buffered 100、不會卡 shutdown goroutine。
  • SecurityHTTP target 寫死 127.0.0.1 ✓payload 含 startTstime.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 200stype.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-1spec 與實作對 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-77system room、event type server: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-shutdown IPC methodbypass 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-pendingsystem room對齊既有 R5-2 server:shutdown-imminent 配對)、補一行說明這跟 firmware:<device> progress room 區隔
  2. Force-close path:把 Design §6a.5 / §6a.11 改成「graceful shutdownserver 端 firmware-aware 180s timeout 保護」、刪掉「SIGKILL bypass」字眼。Code 註解firmware_close_guard.go:225-229已說明理由、寫得清楚、改 spec 採用實作的設計即可

或者反過來改 code 對齊 spec、但 reviewer 不建議——實作的設計更安全。


🟡 Minor

Minor-1MaxShutdownWait 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 應該設 240s200s timeout + 30s margin + 10s buffer

建議MaxShutdownWait 提高到 220s 或 240s、註解說明來源。

Minor-2Second 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-3WS 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 structtaskId / deviceId / chip / direction / stage / elapsedMs / etaSeconds 七欄、跟 query_firmware_active_tasks.go:39-48 FirmwareActiveTaskSummary 對稱)。

Wails 端的 appCloseGuardDeps.EmitFirmwareInProgressfirmware_close_guard.go:143-160已自行構造 map[string]interface{} 過濾掉 startTs、做對了——server 端 broadcast 應該同樣處理。


💡 Suggestion非阻擋、進 backlog

S-1shutdown.go 三個 interface 設計取捨

3 個 interfaceShutdownNotifier / FirmwareLifecycle / ShutdownLogger拆得乾淨、好測。但有過度抽象的疑慮——FirmwareLifecycle 只有 helper 用、production 唯一實作者是 *Service、改名為 *Service direct 反而更清楚。建議保留現狀(測試友善 > 過度抽象 trade-off 在這個尺度可接受),但日後若再加類似 helper、可考慮共用一個 ServiceSnapshot interface。

S-2firmware_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-3query_firmware_active_tasks.go 1s timeout 是否會 false negative

註解line 11-12「1 秒 timeoutserver 沒起來 / 卡死 / network error 全視為『無 active task』」。極端情境server 在升級 KL720 firmwarePython 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

Suggestion1s 在 development env 夠empty ListActiveTasks handler 應該 < 10ms、production load 上若有問題再加 retry-once 或拉到 3s。不阻擋 M9-5。

S-4fakeLifecycle.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 不對 specMajor-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 truetaskWg 計數歸零、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.go6 tests

Test 情境 品質
NoActiveTask 沒 task 立刻 return true 驗 hasActiveCalls / requestShutdown / waitForActive / notifier 都對
ActiveTaskFinishesCleanly 有 task、Wait return true 驗 broadcast room + payload schematype / tasks key
ActiveTaskTimeout 有 task、Wait timeout MaxShutdownWait reset / Warn 觸發
NilService nil service 防呆
NilNotifierAndLogger nil 不 panic recover guard
RealServiceNoActive 真接 Service 整合 驗 wire-up 不踩 nil

漏掉的情境(非阻擋):

  • 連續兩次呼叫 AwaitActiveTasksOrTimeoutsecond SIGTERM 場景)
  • ctx 被 cancel 的行為(目前 helper 收 ctx 但沒用、預留給未來)

query_firmware_active_tasks_test.go7 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.go8 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.1loopback 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 goroutinemain.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 = 100hub.go:56 不會卡 SIGTERM goroutine
  • hub.Run goroutine 在 server lifetime 一直跑、不會在 shutdown 開始前先死、broadcast 至少能進 channel

無 race / 無 deadlock 風險。


是否阻擋 M9-5

不阻擋

理由:

  • 核心 SIGTERM 流程正確、測試覆蓋好、有雙層保護兜底
  • 1 個 Majorspec 不對齊)屬文件層面、不是功能 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 / 密碼變更、無新對外 APIactive-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: roomConfirmForceClose 改成直接 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 6RealServiceNoActive整合驗 wire-up:不只用 fake、還跑一個真接 Service 驗 helper 整條走通——這種「最後一公里」的驗證很多 reviewer 會嫌麻煩跳過、backend 沒跳。

Needs Investigation

無。所有 5 軸 + 測試軸都有明確判斷文字、沒有 Reviewer 看不懂或不確定的地方。


Verification 自評

A 層(每個 review 必做)

  • R-A15 軸 + 測試軸全跑過、每軸 ≥ 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-A2TDD §8.6 對照表填滿 12 條、每條都有實作位置 + 對齊狀態
  • R-A3Major-1 附 spec line numberTDD §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-B28 檔全覆蓋、無漏審
  • 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