visionA/local-tool/.autoflow/05-implementation/reviews/M8-4b-review.md

Reviewer 審查 M8-4b 啟動階段管線（2026-04-15）

摘要

• 審查對象：M8-4b（R5-E 6 階段啟動 pipeline + watcher + sentinel file + RestartStartupSequence）
• 審查檔案：visiona-local/startup_pipeline.go（394 行）、startup_pipeline_test.go（457 行）、server/internal/api/ws/hub.go（sentinel 機制）、hub_sentinel_test.go（3 tests）、visiona-local/app.go（startup / shutdown / runStartupStage5）、visiona-local/server_control.go（stage hooks / probeDeviceListAndComplete / RestartStartupSequence / ForceKill）、server/main.go（wiring）
• 總結論：⚠️ 需小改 — 核心實作對齊 TDD v2/startup-pipeline.md，Build/Vet/Test/Race 全過，功能面沒有阻擋 M8-5 / M8-9 的問題；但發現 2 個 Major UX 問題（重複 OS 通知 + 重複開瀏覽器），建議在 M8-4b 收尾前順手修。
• 問題統計：Critical 0 / Major 2 / Minor 5 / Suggestion 3
• 阻擋後續 milestone 嗎：否。M8-5（前端控制台 UI）可直接使用現在的 event schema 與 RestartStartupSequence binding；M8-9（boot-id + 重連）不觸及 pipeline 內部，可平行推進。兩個 Major 屬 UX 層面，在 M8-10 end-to-end 驗收前修掉即可。

---

A. StartupPipeline struct

TDD §4 要求	實作位置	狀態
6 階段常量 / soft 20s / hard 60s / tick 1s	startup_pipeline.go L39-44	✅
Event schema（StartupProgressEvent / StageTimeoutEvent / ErrorEvent）	L54-75	✅ 欄位命名、json tag 與 §1 一致
Status 枚舉含 pending/running/completed/failed/skipped	L60, L82	✅ 和 §1.1 v2.1 新增 skipped 對齊
1-indexed stages 陣列 + current sentinel 值	L99-101	✅ 0/1-6/7/-1 語義註解清楚
CompleteStage 順序保護 + current != stage 忽略	L138-163	✅ 符合 §4「重複呼叫或順序錯誤，忽略」
SkipStage → 進下一階段	L168-192	✅ 與 CompleteStage 對稱
FailStage → emitError + stopWatcher	L200-213	✅
markReady → emit startup:ready + stopWatcher	L216-226	✅ 成功路徑走同步 emit（註解有解釋）
emitProgress 用 goroutine 非阻塞	L230-251	✅ 符合 §4 關鍵設計 3
emitError 同步 setState(Error) + 非阻塞 OS 通知	L255-277	✅

符合度 100%。

---

B. Watcher goroutine

行為	位置	狀態
每秒 tick + ctx.Done 退出	L291-297	✅
current 不在 1-6 範圍時 return	L300-303	✅
階段 6 每 tick 檢查 sentinel file	L313-318	✅
skipped status / 階段 6 + AutoOpenBrowser=false 跳過 timeout	L321-327	✅ 雙條件都檢查
Hard timeout 直接展開 mark failed + emitError(total-timeout)	L329-341	✅ cause 分流正確；不走 FailStage 以便 cause 設為 total-timeout
softTimeoutEmitted flag 每階段最多 emit 一次	L348-362	✅
skipTimeout continue 時不檢查 soft timeout	L343-345	✅

Minor B-1：watcher 讀取 p.app.prefs.AutoOpenBrowser（L325）未持 a.mu，但 SetPreferences（server_control.go L1062-1064）寫 a.prefs 時持 a.mu。因為 Preferences 是 struct value（含 bool + string），同時讀寫觸發 data race。實測環境罕見（使用者在 Starting state 改 pref），go test -race 沒觸發。建議 watcher 的 pref 讀取改走 getter 或先 snapshot。

---

C. Stage hooks 插入位置

Stage	位置	狀態
1 Wails 控制台	app.go L224 CompleteStage(1)（在 seedUserDataDir 之後、ctrl.Start 之前）	✅ 對齊 §3
2 Python runtime	server_control.go L437-448 FailStage(2) / CompleteStage(2) 夾 ensurePythonRuntime	✅
3 Server spawn	L554-566 FailStage(3) / CompleteStage(3) 夾 waitHealthy	✅
4 Device probe	L571-576 呼叫 probeDeviceListAndComplete(port) → HTTP GET /api/devices 5s timeout → 任何 response/err 都 CompleteStage(4)	✅ 行為與 §3 描述一致（「秒回算完成」），但 5s timeout 過於保守 — 見 §I-2
5 Open browser	app.go L243-267 runStartupStage5() helper：AutoOpenBrowser=false → SkipStage(5)，true → openBrowser(url) + CompleteStage(5)	⚠️ 見 Major C-1
6 WebSocket ready	由 watcher poll sentinel file 觸發	✅

Major C-1：瀏覽器會被開兩次 runStartupStage5（app.go L262）呼叫 openBrowser(url)，但 startInternal（server_control.go L188-194）的 R5-D3 邏輯在 Start 成功後也 openBrowser(url)。冷啟動路徑會執行兩次 openBrowser：

• macOS：open http://... 同 URL 兩次通常聚合到同一 tab，但取決於瀏覽器
• Linux：xdg-open 可能開兩個 tab
• Windows：start 行為不一致

建議的修法（二擇一）：

1. startInternal 在 a.startupPipeline != nil && a.startupPipeline.current >= 0 && a.startupPipeline.current <= startupTotalStages（冷啟動中）時跳過自己的 openBrowser，由 runStartupStage5 負責；RestartServer 情境下 pipeline 已 ready（current==7），R5-D3 仍執行。
2. 反過來：runStartupStage5 只負責 CompleteStage(5)，實際 open 由 startInternal 處理 — 但這樣做跟 TDD §3 階段 5 的語義（「Open browser」＝呼叫 OpenInBrowser 返回）不符。 推薦方案 1。

---

D. Sentinel file 機制

項目	位置	狀態
Hub.writeStartupSentinel 用 sync.Once	hub.go L79-98	✅
Register channel 第一次收到 Subscription 觸發	L112	✅ 在 h.mu.Unlock() 之後呼叫（避免重入）
檔案內容 bootId=... ts=...	L95	✅ 符合 §3
清檔時機：startup 早期（app.go L175）+ shutdown（L299）+ RestartStartupSequence Step 4（L844）		✅ 三處都清，符合 §3 的 best-effort 策略
Hub.SetStartupSentinel 從 server/main.go L133 注入 cfg.DataDir（L101）		✅
DataDir 空值時 disable	hub.go L85-87	✅ 有單測驗證（DisabledWhenDataDirEmpty）

Minor D-1：writeStartupSentinel 取 h.mu.RLock() 保護 sentinelDataDir，但 SetStartupSentinel（L68-72）用 Write lock — 這個 lock 保護與 rooms map 的 lock 共用。功能上正確但概念上有點混搭（同一把 lock 同時保護「訂閱狀態」與「初始化一次的 dataDir」），未來可用獨立 sync.Once 或 atomic.Pointer 分離。

---

E. RestartStartupSequence 5 步驟

Step	位置	狀態
1. cancel watcher goroutine	server_control.go L832-835 a.pipelineCancelFn() + 清 nil	✅
2. ForceKill server subprocess	L838 ctrl.ForceKill()，該 method 在 L273-291 已實作（cancel watcher → clear proc → proc.forceKill() → setState(Stopped)）	✅
3. Reset state machine to Stopped	L841 setState(Stopped, "")	✅ 雖與 Step 2 結尾重複，但 defensive 沒壞處
4. Clean sentinel file	L844 removeSentinelFile(a.dataDir)	✅ 符合 §3 明確要求「critical — 否則階段 6 會誤判瞬間完成」
5. 重建 StartupPipeline + stage 1 直接 complete + watcher + stage 2 running + ctrl.Start() + runStartupStage5()	L847-887	✅ 符合 §8.1
Stage 1 不重跑	L851-858 直接寫 stages[1].status = "completed" + emitProgress	✅
Binding 暴露	Wails binding 是整個 App（main.go L35-37），所有 exported method 自動暴露	✅ RestartStartupSequence() error 符合前端呼叫介面

Minor E-1：Step 5 手動操作 a.startupPipeline.mu 和 internal fields（stages[1].status = "completed" 等），繞過了 struct 的封裝 API。未來改 stageState 欄位或 status 語義時，這段極容易 drift。建議把這段封裝為 (*StartupPipeline).forceCompleteStage1And2Running(time.Time) 或類似方法，把操作內部狀態的邏輯收進 pipeline 自己的 file。

Minor E-2：Step 5 L865 設 a.startupPipeline.watcherCancel = cancel 同一個 cancel 也存在 a.pipelineCancelFn。FailStage → stopWatcher → watcherCancel() 會 cancel，但 a.pipelineCancelFn 不會被 nil-out → 下次 Retry 執行 Step 1 會呼叫已 cancelled 的 func（context.CancelFunc 文件保證 idempotent，OK），然後清 nil。功能正確，但語義上「cancel 執行後清 nil」一致性不足。可接受。

Minor E-3：RestartStartupSequence 在 a.ctx == nil 情境下（單測走這條）不啟動 watcher，正常執行環境下 a.ctx 一定有值，但如果未來 ctx 時序改變，可能踩到。建議在 a.ctx == nil 時 return error 而非 silent skip watcher。

---

F. Stage 5/6 skip 邏輯

規則	驗證
Linux + AutoOpenBrowser=false → Stage 5 skipped	✅ preferences.go L42 DefaultPreferences() 依 GOOS 把 Linux 設 false；runStartupStage5 L247-250 呼叫 SkipStage(5)
AutoOpenBrowser=false → Stage 6 不 trigger timeout	✅ startup_pipeline.go L325-327 skipTimeout 條件覆蓋 hard + soft
skipped 狀態 emit progress event	✅ SkipStage L178 呼叫 emitProgress(stage)，status 已是 "skipped"
Watcher 在 stage 6 + AutoOpenBrowser=false 時不誤判 hard timeout	✅ 有單測 TestStartupPipeline_Watcher_SkippedStageNoTimeout（set 70s sinceTotal 不觸發）

一個設計問題（非 bug）：AutoOpenBrowser=false + 使用者永遠不手動開瀏覽器時，pipeline 永遠停在 stage 6 running → Startup panel 永遠不淡出。TDD §7 驗收條件 case 7 只說「階段 5 立即 complete」，沒說 panel 該什麼時候淡出。M8-5 前端設計需要補一個「AutoOpenBrowser=false 時直接把 panel 淡出 + 顯示『伺服器已就緒，請點 Open in Browser』」的流程。這不是 M8-4b 的 bug，是 M8-5 要處理的 UX。 建議 M8-5 Agent 啟動前提醒。

---

G. 與 M8-4 補丁合併狀況

兩個並行改動：

• M8-4 補丁碰 Stop() / ForceKill() / handleWatchFailure() / logPump()（失敗與關閉路徑）
• M8-4b 碰 startServerV2 中間 hook 點 + 新增 probeDeviceListAndComplete + RestartStartupSequence（成功路徑）

合併狀況：

• ForceKill（L273-291）的 M8-4 修復（MAJ-1：cancelWatcher 先 cancel 避免 30s 後翻 Error）仍存在並被 RestartStartupSequence Step 2 正確利用。✅
• handleWatchFailure（L305-337）的 MAJ-2 修復（持 txMu + 檢查 state != Running 則 return）不被 M8-4b 直接依賴，但 RestartStartupSequence 的 ForceKill 路徑依賴「watcher 先被 cancel」的順序，兩個修復方向一致。✅
• startServerV2 沒被 M8-4 碰，M8-4b 只新增 pipeline hook，不破壞現有流程。✅

cd visiona-local && go build . → PASS cd visiona-local && go vet ./... → PASS cd visiona-local && go test ./... → PASS（7.263s） cd visiona-local && go test -race -count=2 ./... → PASS（15.122s） cd server && go build ./... → PASS cd server && go vet ./... → PASS cd server && go test ./... → PASS cd server && go test -race ./... → PASS

---

H. Build / Test / Race detector

全部 PASS（見 §G 尾）。

---

I. Agent 觀察評估

I-1 重複 OS notification — Major I-1，真的會發生

流程：

1. startServerV2 失敗（例如 stage 3 waitHealthy）→ L559 pipeline.FailStage(3, err)
2. FailStage → emitError(3, err, "stage-failure") → startup_pipeline.go L268-276：
   • c.app.ctrl.setState(ServerStateError, err.Error())
   • go sendCrashNotification("visionA Local — 啟動失敗", "第 3 階段失敗：...") ← 通知 1
3. startServerV2 return err → startInternal L166 收到 err
4. startInternal L168-180：
   • 再次 setState(Error, err.Error())
   • emit server:error event
   • go sendCrashNotification("visionA Local — Server 啟動失敗", "請打開 visionA Local 查看錯誤詳情或按 Restart 重試。") ← 通知 2

兩則通知 title 和 body 都不同，使用者會以為是兩個獨立錯誤；setState(Error) 被呼叫兩次（值相同、前端收到重複 server:state-change event，但 payload 一樣不致命）。

建議修法：在 startInternal L168-180 的 error 分支加守門：

// 若 pipeline 已 FailStage 過，不重複發通知與 setState（pipeline 已處理）
if c.app.startupPipeline == nil || c.app.startupPipeline.current != -1 {
    c.setState(ServerStateError, err.Error())
    if c.app.ctx != nil {
        wailsRuntime.EventsEmit(c.app.ctx, "server:error", ...)
    }
    go sendCrashNotification(...)
}

或 emitError 設一個 ctrl.suppressNextErrorNotification flag，startInternal 收到 err 時若 flag 為 true 則 clear 並 skip 通知。後者耦合小。阻擋 UX 可接受度，建議本次 M8-4b 一併修。

I-2 Stage 4 probe 5s timeout — Minor

TDD §3 原文「GET /api/devices 第一次收到 response（無論是否有硬體，秒回即算完成）」。實作用 http.Client{Timeout: 5 * time.Second} 並把 timeout err 也視為完成。

評估：

• 實測情境：/api/devices handler 讀內部 registry，ms 級就回，5s 幾乎不會用到
• 極端情境：deviceMgr Start 時非同步 USB 掃描若 block 主 goroutine（不太可能），handler 可能慢 >1s
• 5s 對 UX 來說：若硬體 driver 卡住而 health check 已 200，使用者會額外等 5s 看到 stage 4 才 complete。但因為 stage 4 timeout 也算完成，不 fail，只是卡視覺進度
• 對比 TDD「秒回」原意：5s 保守

建議：Timeout: 2 * time.Second。2s 足以涵蓋正常業務 latency 且符合 TDD 原意。Minor 優化，不阻擋。

---

J. 測試品質

14 個 pipeline test + 3 個 hub sentinel test 覆蓋：

項目	測試	狀態
CompleteStage 正常推進	CompleteStage_AdvancesToNext	✅
CompleteStage 順序錯誤忽略	OutOfOrder_Ignored	✅
CompleteStage 最後階段 → markReady	LastStageMarksReady	✅
SkipStage	SkipStage_AdvancesAndMarksSkipped	✅
FailStage	FailStage_StopsPipeline	✅
Soft timeout（mock 時間）	Watcher_SoftTimeout	✅ 用 startedAt = now.Add(-25s) 加速
Hard timeout（mock 時間）	Watcher_HardTimeout	✅ 不是真的跑 60s
AutoOpenBrowser=false + stage 6	Watcher_SkippedStageNoTimeout	✅
skipped status bypass	Watcher_SkippedStatusBypassesTimeout	✅
Sentinel file 偵測	CheckSentinelFile / Stage6CompletesOnSentinel	✅
removeSentinelFile	RemoveSentinelFile	✅ 驗證 idempotent + 空 dataDir
stopWatcher idempotent	StopWatcher_Idempotent	✅
Hub sentinel	3 test（first register / only once / disabled）	✅

Major J-1：TestStartupPipeline_RestartStartupSequence_StepsExecution（L386-439）沒有呼叫 RestartStartupSequence method 本身，而是把 Step 1-5 的邏輯手動複製一遍再驗證 side effect。這個測試保護性很弱：未來改 RestartStartupSequence 內部順序（例如把 Step 3 setState(Stopped) 拿掉 / 改順序），這個測試還是會過。建議重構為：把 Step 6 ctrl.Start() 的部分透過測試 double 攔截（例如改測 app.ctrl.Start 前的狀態 snapshot），直接呼叫 a.RestartStartupSequence() 驗證 Step 1-5 真的執行。

Minor J-1：沒有測試 race 情境「RestartStartupSequence 執行期間使用者再按 Retry」。雖然 pipelineCancelFn() 是 idempotent、NewStartupPipeline 是新 instance 不會 double free，但沒 test 保護。建議加一個 TestRestartStartupSequence_Concurrent 驗證連續呼叫兩次安全。

Minor J-2：沒有驗證 emitProgress 的 goroutine 路徑 — 因為測試環境 a.ctx == nil 讓 emit 走 short-circuit，只能間接驗證內部 state。可接受（Wails runtime 在單測中無法 mock）。

---

K. 問題清單

Critical

（無）

Major

#	檔案	行	問題	建議修法
M-1	visiona-local/startup_pipeline.go + server_control.go	emitError L272-276 + startInternal L168-179	啟動失敗時 OS 通知發兩次、標題內文不同，使用者誤以為是兩個獨立錯誤	startInternal error 分支檢查 startupPipeline.current == -1（代表 pipeline 已 FailStage）時 skip setState/emit/sendCrashNotification；或在 emitError 設 suppress flag 讓後續路徑 skip
M-2	visiona-local/app.go + server_control.go	runStartupStage5 L262 + startInternal L188-194	冷啟動時 openBrowser 被呼叫兩次（R5-D3 R5-E stage 5 hook 重複）	startInternal 的 R5-D3 分支檢查「pipeline 是否在冷啟動中」（current >= 1 && current <= 6），若是則 skip，由 runStartupStage5 統一負責
M-3	visiona-local/startup_pipeline_test.go	L386-439	TestStartupPipeline_RestartStartupSequence_StepsExecution 手動複製實作邏輯，未呼叫 method 本身，保護性弱	重構為直接呼叫 a.RestartStartupSequence()；把 ctrl.Start() 的 spawn 用環境旗標 skip（或 stub）使測試可執行

Minor

#	檔案	行	問題	建議
m-1	startup_pipeline.go	L325	watcher 讀 a.prefs.AutoOpenBrowser 未持 a.mu，與 SetPreferences 有潛在 race	增加 getter (a *App) preferencesSnapshot() Preferences（內部持 a.mu），watcher 改呼叫 getter
m-2	server_control.go	L847-887 RestartStartupSequence	手動操作 pipeline internal fields（stages[1].status = "completed" 等），繞過封裝	封裝為 (*StartupPipeline).RestartFromStage2(ctx, cancelStorer) method
m-3	server_control.go	L596-611 probeDeviceListAndComplete	HTTP timeout 5s 比 TDD「秒回」保守	改 Timeout: 2 * time.Second
m-4	server_control.go	L862-868 RestartStartupSequence	a.ctx == nil 時 silent skip watcher，實際執行環境不會觸發但語義不清	a.ctx == nil 時 return error "app context not ready"
m-5	startup_pipeline_test.go	—	沒有驗證 RestartStartupSequence 連續呼叫（race）	加 TestRestartStartupSequence_Concurrent：wg.Add(2); go restart(); go restart(); wg.Wait() 驗證不 panic + 單一 pipeline instance 存活

Suggestion

#	內容
s-1	hub.go sentinelDataDir 和 sentinelOnce 共用 Hub.mu，概念上混搭。可改用獨立 sync.Once 或 atomic.Value 分離初始化狀態與訂閱狀態
s-2	watcher 的 skipTimeout 判斷邏輯可抽成 helper (p *StartupPipeline) shouldSkipTimeout(stage int, status string) bool，提升可讀性
s-3	失敗時的 OS 通知可考慮改成 in-app modal + OS 通知擇一（OS 通知常被使用者忽略），搭配 Wails 原生 dialog 效果更好 — 但這超出 M8-4b 範圍，屬於 M8-5 / M8-10 UX 階段決定

---

L. 結論

判定：⚠️ 需小改

M8-4b 的核心工程實作（StartupPipeline struct / 6 階段 hook / watcher / sentinel file / RestartStartupSequence）完整對齊 TDD v2/startup-pipeline.md v2.1，Build/Vet/Test/Race 4 個平台全通過，合併 M8-4 補丁乾淨無衝突。

阻擋 M8-5 / M8-9 嗎：否。 目前的 event schema、binding 與 sentinel 機制已經足夠 M8-5（Wails 控制台 UI 訂閱事件）與 M8-9（boot-id 重連）直接開工。

建議的後續動作：

1. M8-4b Agent 再跑一輪，修 Major M-1（重複通知）+ M-2（重複開瀏覽器）+ M-3（Retry test 重構）。這三項都是工時小（< 30 分鐘）但影響使用者第一眼觀感的 UX 問題，在 M8-10 前必修。
2. Orchestrator 交棒 M8-5 時提醒 Frontend Agent：AutoOpenBrowser=false + stage 6 永遠 running 的情境下，startup panel 該如何淡出 / 顯示「請手動開瀏覽器」的 CTA。這個 UX 決定不屬於 M8-4b 範疇，但要在 M8-5 啟動前搞清楚。
3. Minor 5 項 + Suggestion 3 項可記為技術債，不強制 M8-4b 處理。

優點：

• TDD §4 結構完整重現，status 列舉 / 欄位 / emit 策略全部符合
• skipped 狀態在 watcher + CompleteStage/SkipStage 的處理一致，邊界條件清楚
• Test 用 mock 時間（startedAt.Add(-Xs)）加速 soft/hard timeout 驗證，沒有真跑 60 秒
• Sentinel file 三個清檔時機（startup 早期 / shutdown / Retry）覆蓋完整
• 與 M8-4 補丁（ForceKill / handleWatchFailure 的 MAJ-1/MAJ-2 修復）合併乾淨，RestartStartupSequence 正確利用了 ForceKill 的 cancel watcher 修復
• 測試覆蓋面積大（17 個 test），邊界條件完整