jim800121/visionA
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
visionA/local-tool/.autoflow/05-implementation/reviews/M8-7-review.md
jim800121chen 8cd5751ce3 feat(local-tool): M8 重構 — Wails 控制台 + 瀏覽器 Web UI（R5 決策） 
依 R5 五輪決策把 visionA-local 從「Wails 內嵌 Next.js」重構為「Wails
本機伺服器控制台 + 瀏覽器 Web UI」模式（類比 Docker Desktop / Ollama）。

程式碼變動
  - M8-1 砍 yt-dlp 全套（後端 resolver / URL handler / 前端 URL tab /
    Makefile vendor / installer / bootstrap / CI workflow，-555 行）
  - M8-2 砍 Mock 模式全套（driver/mock、mock_camera、Settings runtimeMode、
    VISIONA_MOCK 環境變數，-528 行）
  - M8-3 ffmpeg 從 GPL 切換到 LGPL 混合方案：Windows/Linux 用 BtbN 現成
    LGPL binary，macOS 自 build minimal decoder-only 進 git
    （vendor/ffmpeg/macos/ffmpeg 5.7MB + ffprobe 5.6MB，比 GPL 版省 85% 空間）
  - M8-4 Wails Server Controller：state machine、log ring buffer 2000 行、
    preferences.json atomic write、boot-id、Gin SkipPaths、shutdown 7+1 秒、
    notify_*.go 三平台 OS 通知、watchServer 改 Error state 不 os.Exit
  - M8-4b 啟動階段管線 R5-E：6 階段進度 event、20s soft / 60s hard timeout、
    stage 5/6 skip 規則、sentinel file、RestartStartupSequence 5 步驟
  - M8-5 Wails 控制台 vanilla HTML/JS/CSS（9 檔 ~2012 行）取代 M7-B splash：
    state 視覺、log panel、startup progress panel、Stage 6 manual CTA
    pulse、shutdown modal、Settings、Dark Mode、i18n 中英雙語
  - M8-6 上傳影片副檔名擴充（mp4/avi/mov/mpeg/mpg）
  - M8-7 Web UI Server Offline Overlay（role=alertdialog + focus trap +
    wsEverConnected 容錯 + Page Visibility）
  - M8-8 CORS middleware（127.0.0.1/localhost only + suffix attack 防護）+
    ws/origin.go 獨立 WebSocket CheckOrigin 避 package cycle
  - MAJ-4 server:shutdown-imminent WebSocket broadcast 機制
    （/ws/system endpoint + notifyShutdownImminent helper）
  - M8-9 Boot-ID + 瀏覽器 tab 自動重連（sessionStorage loop guard）

品質
  - ~105+ 新 unit test + race detector (-count=2) 全綠
  - 10 個 milestone 全部通過 Reviewer 審查
  - 三方 v2 + v2.1 文件（PRD / Design Spec / TDD）+ 交叉互審紀錄
    收錄在 .autoflow/

交付前待處理（M8-10）
  - 重跑 make payload-macos 把舊 GPL 77MB binary 換成新 LGPL
  - 三平台 end-to-end build 驗證

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-15 17:57:54 +08:00

189 lines
12 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Reviewer 審查 M8-7 Offline Overlay（2026-04-15）
## 摘要
- **總結論：✅ 通過**（無 Critical / Major 問題；2 個 Minor + 2 個 Suggestion）
- **不阻擋 M8-9 / M8-10**：store 已預留 `bootId` + `setBootId`，且刻意不做 boot-id 比對 reload（M8-9 職責），分工清楚。
- **親跑狀態**：`pnpm tsc --noEmit` PASS、`pnpm build` PASS（12 頁 static export）、新檔 lint 全乾淨（`pnpm lint` 的 11 errors 全部來自既有未觸及的檔案，與本次無關）。
---
## A. `system-store.ts`
| 檢查項 | 結果 | 備註 |
|-------|-----|------|
| 欄位 `serverOnline / forcedOffline / offlineReason / bootId / consecutiveFailures` | ✅ | L18-28 |
| `FAILURE_THRESHOLD = 2` exported | ✅ | L44 |
| Actions：`forceOffline / markOnline / recordFailure / setBootId` | ✅ | L30-37、L53-73 |
| `OfflineReason` type 覆蓋 quit / restart / healthcheck-failed / unknown | ✅ | L16 |
**設計簡化得當**：相較 TDD §4.2 的原稿（store 內部用 `setTimeout` 處理 restart 延遲），實作把延遲邏輯搬到 hook（`use-shutdown-watcher` L171-176），store 保持純資料、零副作用，易於測試。這是合理的偏離，不算問題。
**Minor 1（可選）**：`markOnline()` 沒有重置 `bootId`，這是刻意保留以供 M8-9 比對（見檔案註解 L13-14），但 TDD §4.2 原版 `setOnline` 會一起更新 bootId。此處改為由呼叫端自己 `setBootId` + `markOnline`（hook L83-84），職責分離沒問題。
---
## B. `use-shutdown-watcher.ts` hook 邏輯
| 檢查項 | 結果 | 引用 |
|-------|-----|------|
| Normal polling 10 s | ✅ | L28 `POLL_INTERVAL_NORMAL_MS = 10_000` |
| Active retry 3 s | ✅ | L29 `POLL_INTERVAL_ACTIVE_RETRY_MS = 3_000` |
| 連續 2 次失敗門檻 | ✅ | `FAILURE_THRESHOLD` 由 store 匯入，L124 比對 |
| `useSystemStore.subscribe` 自動切模式 | ✅ | L122-137 |
| 失敗達門檻自動補 `forceOffline('healthcheck-failed')` | ✅ | L127-133 |
| Health check URL = `/system/boot-id` | ✅ | L68（透過 `getApiBaseUrl()` 組出 `/api/system/boot-id`，與 TDD §3.2 一致）|
| AbortSignal.timeout 3 s | ✅ | L71、L271 |
| Page Visibility API：背景暫停、前景立即 probe | ✅ | L215-228、L231-235 |
| WebSocket 連線 `/ws/system` + token | ✅ | L147-151 |
| 訂閱 `server:shutdown-imminent` | ✅ | L161-185 |
| restart 延遲 10 s | ✅ | L168-177 |
| quit / app-closing / manual-stop 立即 | ✅ | L180-181，`mapReason` 把三者統一為 `quit` L49-61 |
| `wsEverConnected` flag 容錯 | ✅ | L98、L188、L192 |
| `retryServerHealth()` exported | ✅ | L264-289 |
| cleanup 清 interval / timer / WS listener | ✅ | L239-259 |
**Minor 2**：L188 `ws = null` 在 closure 內重設後，L252 的 `if (ws)` 不會進，但 `onerror` 是在 outer scope 持有的 `ws` 變數上設定監聽器，如果 close 觸發時 `ws` 已被改為 `null`，那 cleanup phase 的「ws.onclose = null」等就跳過——這其實 OK，因為 socket 本身已被 GC。唯一的副作用是：若 close 發生後 cleanup 才跑，`scheduleWsReconnect` 會啟動一個 5 s 後的 reconnect timer，但 cleanup 的 `cancelled = true` 加上 L206 的 check 會把 timer callback 內的 reconnect 攔掉（雖然 timer 本身仍被 `clearTimeout` 清掉 L244-247）。邏輯正確，但建議加一行註解說明。
**容錯驗證（重點）**：WebSocket close 時，程式碼**不**直接 `forceOffline`，而是 L194 呼叫 `void pollOnce()`——讓 polling 機制決定。這是聰明的做法：若 server 只是短暫 glitch（WebSocket 掉但 HTTP 還活），不會誤觸發 overlay。這比 TDD §4.3a L421-424 的原稿（直接 `forceOffline('manual-stop')`）更保守也更正確，避免 race condition。符合審查點 I 的要求。
**restart 延遲 10 s 的實作細節**（L171-176）：延遲 callback 裡檢查「`!serverOnline || consecutiveFailures >= FAILURE_THRESHOLD`」才 `forceOffline`。這代表若 10 s 內 polling 成功（server 回來）→ `markOnline()` 會把 `serverOnline` 設回 true、`consecutiveFailures` 歸零 → callback 進不了 `forceOffline` 分支 → overlay 不顯示。符合 Design §9.3「短暫 restart 使用者無感」的期望。
---
## C. `ServerOfflineOverlay` React 元件
| 檢查項 | 結果 | 引用 |
|-------|-----|------|
| `role="alertdialog"` + `aria-modal="true"` | ✅ | L96-97 |
| `aria-labelledby` / `aria-describedby` / `aria-live="assertive"` | ✅ | L98-100 |
| Focus trap | ✅ | L37-65（初始 focus 推到 retry 按鈕 + Tab 循環）|
| Dynamic subtitle by `offlineReason` | ✅ | L81-92（quit / restart / healthcheck-failed / default 四分支）|
| Retry 按鈕 → `retryServerHealth` | ✅ | L69-78 |
| 無 close button | ✅ | 僅 retry + help text，符合 TDD §2 L34 註釋與 Design §4「ESC/點背景不 dismiss」|
| Help text「如要離開本頁請直接關閉分頁」 | ✅ | L157（i18n `offline.helpText`）|
| Dark Mode | ✅ | L105 `dark:bg-black/70`、L116 `dark:bg-destructive/15` |
| i18n key 正確 | ✅ | 全部透過 `t()` 抽離，無硬編文字 |
| `data-testid` 供測試 | ✅ | L106 `server-offline-overlay`、L143 `server-offline-retry` |
**Suggestion 1**：Design Spec §2.2 有「了解更多 ↓」展開按鈕+展開式 help text（§5.2、§5.3），目前簡化為一行 `helpText`。這是刻意簡化（M8-7 issue 也沒列出展開式 help），可接受，但建議在 `待人工介入` 不算，因 Design 未被嚴格比對為必需。如未來要做，可在後續 M 級修改補上，不阻擋交付。
**Suggestion 2**：Design §6 規定的 fade-in / translateY(20→0) 入場動畫、重試失敗 shake 動畫、`prefers-reduced-motion` 降級——目前無任何動畫。屬於 UX polish，非功能缺失，列為 Suggestion。
**Focus trap 品質**：L46-62 的實作符合基本要求（Tab / Shift+Tab 循環），但只在卡片內 focusable 數量 ≥ 1 時才有效；目前只有一顆 retry 按鈕，Tab 實際上會停在原位，也無害。符合題目「有基本實作，不一定完美」的標準。
---
## D. 掛載位置
| 檢查項 | 結果 | 引用 |
|-------|-----|------|
| `layout.tsx` 掛 `<ShutdownWatcherMount />` + `<ServerOfflineOverlay />` | ✅ | `layout.tsx` L11-12 import、L58-59 JSX |
| SSR 相容：client-only 邏輯在 `'use client'` + `useEffect` | ✅ | watcher L1、L92 `typeof window === 'undefined' return`；overlay L1 `'use client'` |
| 整個 app lifecycle 只有一個 watcher | ✅ | root layout 只掛一次；mount wrapper 本身 `return null` |
`shutdown-watcher-mount.tsx` 是薄薄的 `'use client'` wrapper，符合 Next.js 16 App Router 的 server/client 分離要求。
---
## E. i18n
| 檢查項 | 結果 | 引用 |
|-------|-----|------|
| `types.ts` 有 `offline.*` type | ✅ | types.ts L417-427 |
| `zh-TW.ts` 完整 | ✅ | zh-TW.ts L419-429 |
| `en.ts` 完整 | ✅ | en.ts L419-429 |
| key 對齊 component 使用 | ✅ | `title / subtitle.{quit,restart,healthcheck} / retryButton / retrying / helpText` 全 match |
| 文案符合 Design Spec §5 | ⚠️ 部分 | 詳見下 |
**Minor 3**：Design §5.1 繁中副標應為「visionA-local 已結束或崩潰，請重新開啟應用程式」，實作為三段差異化文案（`quit/restart/healthcheck`）。這其實**比**原 Design 更精準（原 Design 未考慮 restart / healthcheck 的動態文案），不算 bug，但偏離了 Design 的字面規格。建議記錄為「實作優化 Design」，或反向更新 Design §5.1 補上三段文案。**不阻擋交付**。
---
## F. Build 結果
```
pnpm tsc --noEmit → PASS（無輸出）
pnpm build → PASS（12 頁 static export，Compiled successfully in 5.1s）
pnpm lint（新檔 8 檔） → 0 errors / 0 warnings
pnpm lint（全專案） → 11 errors 全部來自既有檔案（use-websocket / use-first-visit /
use-server-health / model-comparison-dialog / auto-connect），
與 M8-7 無關
```
---
## G. SSR 相容性
- `system-store.ts`：純 zustand，無 window/localStorage 存取，SSR safe。
- `use-shutdown-watcher.ts`：全部副作用包在 `useEffect` + L92 額外 `typeof window === 'undefined' return` 雙保險。
- `server-offline-overlay.tsx`：`'use client'`，且 `show=false` 時 L67 直接 `return null`，SSR 輸出空字串 → 後端無 window 存取。
- `shutdown-watcher-mount.tsx`：`'use client'` + `return null`。
- `layout.tsx`：未加 `'use client'`（root layout 仍是 server component），但掛的兩個元件都是 client component，Next.js 會自動切換 island 邊界。
`pnpm build` 通過 12 頁 static generation 證明沒有 `ReferenceError: window is not defined`。
---
## H. M8-9 預留
- ✅ `bootId` field（L26 store）
- ✅ `setBootId` action（L37、L73）
- ✅ **不做** boot-id 比對 + force reload：hook `pollOnce` L83-84 只 `setBootId + markOnline`，沒有 `if (prevBootId !== newBootId) window.location.reload()`。符合 M8-9 分工。
- 檔頭 L13-14 註解明確聲明「M8-7 不負責 boot-id 比對 + force reload」，給未來 reader 清楚信號。
---
## I. 容錯
| 情境 | 行為 | 是否符合預期 |
|-----|------|------------|
| `/ws/system` 從未連上 | `wsEverConnected=false`，onclose 不觸發 forcedOffline | ✅ L188-196 |
| `/ws/system` 連上後斷線 | `wsEverConnected=true`，僅呼叫 `pollOnce()` 由 polling 決定 | ✅ L192-195（比 TDD 更保守）|
| Network error / AbortError | `pollOnce` catch 統一 → `recordFailure` | ✅ L85-87 |
| fetch 非 200（5xx / 非 JSON） | `recordFailure` | ✅ L73-80 |
| 背景 tab recover double-probe | visibilitychange L215-228 先 `pollOnce` 再 `startPolling`，同一 tick 內只 probe 一次；setInterval 是新排程，不會 race | ✅ |
| 連上 server 時自動 reconnect | 5 s reconnect timer L204-212，有 `cancelled` + `wsReconnectTimer !== null` 雙重 guard 避免堆疊 | ✅ |
| Restart defer 後 server 回來 | L173 條件 `!serverOnline || failures >= 2` 未成立 → 不 forceOffline | ✅ |
**容錯整體評價**：比 TDD §4.3a 原稿謹慎，實務上更不易誤觸發。
---
## J. 問題清單
### Critical
（無）
### Major
（無）
### Minor
| # | 檔案 | 行數 | 問題 | 建議 |
|---|------|-----|------|------|
| 1 | `use-shutdown-watcher.ts` | 188, 194 | WebSocket onclose 後 `ws = null` 與 cleanup 的互動較隱晦（清理路徑 L252 不會進）| 加 1-2 行註解說明「已設 null，cleanup 靠 `cancelled` flag 攔後續 reconnect」 |
| 2 | `server-offline-overlay.tsx` | 81-92 | `subtitleKey` 對 `'unknown'` 會 fallback 到 `healthcheck` 文案，雖然合理，但英文環境可能讓使用者困惑 | 可選：為 `unknown` 另外加一個 i18n key，或在 `offlineReason === 'unknown'` 時顯示通用錯誤 |
| 3 | `zh-TW.ts` / `en.ts` | 419-429 | 副標文案比 Design §5.1 多了三段 reason 分支（Design 只列一段通用文案）| 反向更新 Design §5.1 補上三段，或保持現狀並在 PR 說明 |
### Suggestion
| # | 檔案 | 建議 |
|---|------|------|
| 1 | `server-offline-overlay.tsx` | Design §2.2 有「了解更多 ↓」展開式 help text（§5.2 詳細重啟步驟），目前簡化為單行。未來可補 |
| 2 | `server-offline-overlay.tsx` | Design §6 的 fade-in / translateY / shake / reduced-motion 動畫全缺，列為 polish |
---
## K. 結論
**✅ 通過**。M8-7 在功能正確性、SSR 相容、容錯設計、M8-9 預留、i18n 覆蓋上都達到交付標準。新增的四個檔案 lint/type-check/build 全部乾淨；修改的 `layout.tsx` + i18n 三檔也沒有破壞既有頁面（build 出 12 頁 static export 證明 /workspace, /devices, /models, /settings 等都 OK）。
**重點亮點：**
1. **容錯比規格更保守**：WebSocket close 不直接 forceOffline 而是交給 polling 驗證（L192-195），避免 race。
2. **M8-9 分工乾淨**：store 保留 `bootId` 但刻意不做 reload，檔頭註解明確聲明，未來 M8-9 接手零負擔。
3. **SSR 雙保險**：`'use client'` + `typeof window === 'undefined'` return，build 通過是最硬的證據。
4. **零新增 lint/type 錯誤**。
**不需修改即可進 M8-8 / M8-9 / M8-10**。Minor 1-3 + Suggestion 1-2 建議記錄為技術債，於後續迭代處理。
Reference in New Issue View Git Blame Copy Permalink