# Reviewer 審查 MAJ-4 shutdown-imminent broadcast（2026-04-15）

## 摘要
- **結論**：✅ 通過。實作完整、測試涵蓋、race 乾淨、文件契約對齊、兩條 flow（quit / restart）邏輯正確。
- **阻擋 M8-10？** 否。M8-4 遺留的 MAJ-4「shutdown-imminent 廣播」已補齊，可與 M8-5 patch、M8-7 / M8-8 / M8-9 一併收斂進 M8-10。
- 發現 1 個 Minor（payload reason 與 TDD §2.3 範例文字不完全一致），1 個 Info（Hub sentinel 行為變更），均不阻擋。

## A. ShutdownNotify handler
| 檢查 | 結果 | 依據 |
|------|------|------|
| `POST /api/system/shutdown-notify` 路徑 | ✅ | `router.go:64` |
| 接 query `reason` | ✅ | `system_handler.go:180` |
| 預設 reason（空） | ✅ 歸類 `unknown` | `system_handler.go:181-186`（空值走 `default` 分支）|
| 驗證 reason | ✅ | 只認 `quit` / `restart`，其餘→ `unknown` 仍 200 |
| 呼叫 BroadcastToRoom | ✅ | `system_handler.go:194` |
| sleep 100ms 後回 200 | ✅ | `system_handler.go:26, 196-198, 201` |
| 無 client 仍 200 | ✅ | Hub `BroadcastToRoom` 空 room no-op（`hub.go:129`），handler 不判斷 client 數 |
| wsHub nil 不 panic | ✅ | `system_handler.go:188` nil guard + `TestShutdownNotify_NoHub` 覆蓋 |

**注意**：預設（空 reason）不是直接對應到 `"quit"` 而是 `"unknown"`，與 prompt「預設 reason=quit」的文字描述不一致。但 **caller（Wails app）永遠明確帶 `quit` 或 `restart`**（`app.go:304`、`server_control.go:299`），預設值只在誤呼叫路徑生效，且 `"unknown"` 對前端而言透過 `mapReason` 仍會走 `'quit'` 分支立即顯示 overlay（`use-shutdown-watcher.ts:70-71` 會 fallback → overlay 仍顯示）。**功能等價且更安全**（避免亂送的 reason 誤映射到 quit）。可接受。

## B. shutdownNotifyBroadcaster interface
- `system_handler.go:18-20` 定義介面；`wsHub` 欄位以介面型別保存（`:36`, `:49-52`）。
- 單元測試用 `spyBroadcaster` 注入（`system_handler_test.go:29-57, 68-79`），完全脫離 real Hub goroutine。
- ✅ 解耦乾淨、便於測試。

## C. Hub.BroadcastToRoom
| 檢查 | 結果 | 依據 |
|------|------|------|
| 重用既有 API | ✅ | `hub.go:160-166`（既有方法，未新造）|
| Non-blocking | ✅ | `hub.go:131-136` select default → 滿 channel 直接 drop + `close` + `delete` |
| Room `system` 正確 | ✅ | `system_handler.go:194`、`system_ws.go:36` |
| 訊息格式 | ✅ | `system_handler.go:189-193`（`type`, `reason`, `ts` = `UnixMilli()`）|

`TestHub_BroadcastToRoom_FullChannelDoesNotBlock` 驗證慢 client 不卡 hub goroutine，第二次 broadcast 仍能送到 healthy client（`hub_broadcast_test.go:73-132`）。✅

## D. /ws/system WebSocket endpoint
| 檢查 | 結果 | 依據 |
|------|------|------|
| router 註冊 | ✅ | `router.go:102` |
| client 加入 `system` room | ✅ | `system_ws.go:36-37`（`RegisterSync` 保證返回時已 in-room）|
| 沿用 M8-8 Origin check | ✅ | 共用 `upgrader`，`device_events_ws.go:13-14` 的 `CheckOrigin: CheckOrigin` 自動繼承 |
| M8-4b sentinel 行為 | ⚠️ Info | 見下方說明 |

**M8-4b sentinel 互動（Info，非問題）**：Hub 的 `writeStartupSentinel` 由 `sync.Once` 保護，只要**任何** room 的第一個 client 連上就寫 `.first-ws-connected`（`hub.go:100-115`）。`/ws/system` 是 `BootIdWatcherMount` 在瀏覽器 tab 載入時自動連上（`use-shutdown-watcher.ts:298, connectWs`），實務上極可能**成為第一個**連上的 WS endpoint，讓 startup pipeline 階段 6 由 `/ws/system` 觸發完成。這**不違反** `startup-pipeline.md §3 階段 6` 的語意（定義是「Web UI 連上任何 WS」），且 sentinel 只在第一次寫入、後續 no-op，不會造成 race。✅

## E. notifyShutdownImminent helper
| 檢查 | 結果 | 依據 |
|------|------|------|
| 1 秒 timeout + 變數化 | ✅ | `shutdown_notify.go:26`（`notifyShutdownImminentTimeout`）|
| Best-effort（錯誤靜默）| ✅ | `:49, :53` 兩個 err 分支直接 return；不 log |
| `port <= 0` no-op | ✅ | `:38-40` |
| ctx nil 保護 | ✅ | `:41-43`（fallback 到 `context.Background`）|
| ctx timeout 包裹 | ✅ | `:44-45`（`context.WithTimeout`）|
| 釋放 resp.Body | ✅ | `:57`（`resp.Body.Close`）|

測試 6 個全部覆蓋（zero port / POST 正確 / reason=restart / timeout 不卡 / 5xx 不 panic / connection refused 靜默）。`TestNotifyShutdownImminent_TimeoutDoesNotBlock` 實測 elapsed < 400ms（`shutdown_notify_test.go:95-123`）。✅

## F. Wails app 端呼叫位置
| 檢查 | 結果 | 依據 |
|------|------|------|
| `shutdown()` 在 `ctrl.Stop()` 前呼叫 | ✅ | `app.go:302-309`（順序：notify → Stop）|
| `Restart()` 在 `Stop` 前呼叫 | ✅ | `server_control.go:291-305`（順序：notify → Stop → StartWithPort）|
| reason 正確 | ✅ | `app.go:304 = "quit"`；`server_control.go:299 = "restart"` |
| port 來源 | ✅ | shutdown 走 `snapshotStatus().Port`（`app.go:303`）；Restart 走 `c.proc.port` 鎖內複製成 `oldPort`（`server_control.go:284-289`）|

兩處都有 nil guard（`a.ctrl != nil` / `c.app != nil && oldPort > 0`），失敗路徑不阻塞主流程。✅

## G. 和 M8-7 前端對齊
讀 `use-shutdown-watcher.ts`（325 行）：

| 檢查 | 結果 | 依據 |
|------|------|------|
| 訂閱 `/ws/system` | ✅ | `:211, 215`（`getWsBaseUrl() + '/ws/system'`）|
| 解析 `server:shutdown-imminent` | ✅ | `:228` |
| `quit` 立即 `forceOffline` | ✅ | `:244-245`（`mapReason('quit') → 'quit' → forceOffline('quit')`）|
| `restart` 延遲 10 秒 | ✅ | `:232-242`（`RESTART_DEFER_MS = 10_000`）|
| `restart` 期間 polling 會先拿新 boot-id 觸發 reload | ✅ | `:186-201` polling 仍跑；restart defer 10s 內新 server 起來 → `pollOnce → handleBootIdCheck → mismatch → reload`（`:86-123`）|

`mapReason` 把 server 的 `quit` / `app-closing` / `manual-stop` 都 fold 到 `'quit'`（`:61-73`），所以後端改送 `"quit"` 而非 TDD 範例的 `"app-closing"`**對前端完全透明**（雙方都走立即 forceOffline 路徑）。✅

**與 TDD 範例的微差異**（Minor）：`server-lifecycle.md:141` 範例寫 `payload: { reason: "app-closing" }`，實作是 `reason: "quit"`。兩者在前端都觸發立即 overlay，但字面不一致。建議在 server-lifecycle.md §2.3 / §8 補註「實際 reason 由 caller 決定：Wails shutdown=`quit`、Restart=`restart`」即可，不需改 code。

## H. Integration test
- `system_ws_integration_test.go:22-83` 用 `httptest.NewServer` + gin router 掛 `SystemEventsHandler`，真實 gorilla `websocket.DefaultDialer.Dial` 連上 → `hub.BroadcastToRoom("system", ...)` → `conn.ReadMessage()` 解析 JSON → 驗證 `type`、`reason`。
- `:49-58` 等 Register 同步完成（poll `hub.rooms["system"]` 非空，best-effort 500ms）。同 package 可存取 `hub.mu` 私有欄位，無需匯出。
- `httptest.Server` 有 `defer srv.Close()`、`conn.Close()`、`SetReadDeadline` 2s 保護，不會 leak。✅

## I. Test 品質（15 個）
- **server/internal/api/handlers**（5）：Quit / Restart / Invalid（4 sub-case） / NoHub / DefaultSleepPositive — 全部親跑通過。
- **server/internal/api/ws**（4）：MultipleClients / EmptyRoom / FullChannelDoesNotBlock / SystemEventsHandler_ReceivesBroadcast — 全過。
- **visiona-local**（6）：ZeroPort / SendsPostWithReason / SendsReasonRestart / Timeout / ServerError / ConnectionRefused — 全過。
- 合計 **15 個新 test 全部 PASS**（含 sub-test）。
- 使用 `withNoSleep` helper 把 `shutdownNotifySleepDuration` 歸零加速，`t.Cleanup` 還原 — 寫法乾淨。
- `TestShutdownNotify_DefaultSleepIsPositive` 特別保護生產常數不被誤改為 0 — 有心。

## J. 親跑驗證
```
cd server
  go build ./...               OK
  go vet ./...                 OK
  go test ./...                OK（含 api/handlers/ws 全綠）
  go test -race -count=1 ./internal/api/...
                               api 2.057s / handlers 1.548s / ws 2.726s 全綠

cd visiona-local
  go build .                   OK
  go vet ./...                 OK
  go test -run NotifyShutdown -v ./...
                               6 tests PASS (含 timeout 0.50s)
  go test -race -count=1 ./... OK 8.930s（含 race detector）
```
全部通過，無 race warning、無 build error。

## K. 完整 flow 驗證（讀 code 推論）

**Quit flow**（使用者關 Wails 視窗）：
1. `OnBeforeClose` → `app.shutdown(ctx)`（`app.go:280`）
2. `a.pipelineCancelFn()` / `a.watchCancel()` / IPC close / sentinel 清理（`:282-294`）
3. `port := a.snapshotStatus().Port` → `notifyShutdownImminent(ctx, port, "quit")`（`:303-304`）
4. server `ShutdownNotify` handler：`BroadcastToRoom("system", {type, reason: "quit", ts})` → `time.Sleep(100ms)` → `200 OK`
5. `/ws/system` write pump 把 JSON 推到 TCP；瀏覽器 `onmessage` → `mapReason('quit')='quit'` → `forceOffline('quit')` 立即顯示 overlay
6. `a.ctrl.Stop()`（`:309`）→ 7s grace → server 退出
7. 瀏覽器 polling 後續都 ECONNREFUSED，但 overlay 已顯示，不重複觸發

✅ 正確。

**Restart flow**（使用者按 Restart）：
1. `ServerController.Restart()`（`server_control.go:282`）
2. 鎖內複製 `oldPort := c.proc.port`（`:284-289`）
3. `notifyShutdownImminent(ctx, oldPort, "restart")`（`:299`）
4. server 廣播 `{reason: "restart"}` → 100ms sleep → 200
5. 瀏覽器 `onmessage` → `mapReason('restart')='restart'` → `restartDeferTimer = setTimeout(forceOffline('restart'), 10_000)`（`use-shutdown-watcher.ts:232-241`）
6. `c.Stop()` → server 退出；`c.StartWithPort(oldPort)` → 新 server 起（新 boot-id）
7. 瀏覽器 polling（normal mode，10s interval）先打到新 server：`handleBootIdCheck` → `'mismatch'` → `window.location.reload()`（`:95-123`）
8. Reload 後新 page 初始化 → `pollOnce` → `'first'` → `markOnline` → normal 模式
9. 若 reload 發生在 10s defer 之前 → defer timer 被 unmount 時 `clearTimeout`（`:312-315`）→ overlay 沒機會顯示 ✅
10. 若 reload 發生在 10s 之後 → overlay 顯示後 reload 本身也會清除 → 正確復原

✅ 正確，兩條 flow 的時序都能正常收斂。

## L. 問題清單

### Major
（無）

### Minor
| # | 檔案:行 | 問題 | 建議 |
|---|---------|------|------|
| MIN-1 | server_handler.go:189-193 vs `server-lifecycle.md:141` | TDD 範例寫 `reason: "app-closing"`、實作送 `reason: "quit"`。前端 `mapReason` 雙向皆 fold 成 `'quit'`，功能等價，但文件字面不一致。 | 在 `server-lifecycle.md §2.3` 補一行：「實作中 reason 由 Wails caller 決定：shutdown `quit`、Restart `restart`」。不必改 code。 |
| MIN-2 | system_handler.go:181-186 | 空 reason 被 fold 成 `"unknown"`，prompt 審查項說「預設 reason=quit」。目前由 caller 永遠帶值，非實際風險。 | 如要嚴格對齊 prompt 可把 `default` 分支改成 `reason = "quit"`；或於 commit message 標註「預設 unknown 是刻意保守設計」。Reviewer 傾向維持現狀。 |

### Info（非問題）
- **Hub sentinel 與 /ws/system 交互**：`/ws/system` 因 mount 時機早，實務上會成為第一個寫 `.first-ws-connected` 的 endpoint。符合 `startup-pipeline.md §3 階段 6` 的語意（「Web UI 連上任何 WS」），無 race，無需調整。
- `TestSystemEventsHandler_ReceivesBroadcast` 直接存取 `hub.mu` / `hub.rooms`（私有）— 因為同 package 合法，但未來若搬到外部 integration package 會需要 exporter。目前 OK。

## M. 結論
MAJ-4 patch 品質達標：
1. 介面（`shutdownNotifyBroadcaster`）乾淨解耦，spy broadcaster 測試乾淨。
2. Hub.BroadcastToRoom 重用既有 API，未新造；non-blocking 行為有專屬 test 覆蓋。
3. `/ws/system` handler 沿用 M8-8 `upgrader` / `CheckOrigin`，無繞過安全機制。
4. notifyShutdownImminent helper best-effort 全面（port<=0 / ctx nil / timeout / connection refused / 5xx 五種路徑都有測試）。
5. Wails 兩處呼叫點（`shutdown` / `Restart`）順序正確，都在 Stop 之前、port 來源正確。
6. 與 M8-7 前端 `use-shutdown-watcher.ts` 的 reason 映射、restart 10 秒 defer、M8-9 boot-id reload guard 完整對齊。
7. 15 個新 test 全綠，`go build / vet / test / test -race` server + visiona-local 均乾淨。
8. 兩條完整 flow（Quit / Restart）讀 code 推論時序正確，overlay 不會 race、reload loop 有 guard。

**建議**：接受 patch，MAJ-4 結案。Minor 1 / 2 不阻擋、留作文檔與 M8-10 final pass 時順手修正即可。M8-4 遺留 5 個 Major 至此（MAJ-4）已補齊，可推進 M8-10。