visionA/local-tool/.autoflow/05-implementation/review/m9-1-bridge-firmware-upgrade-review.md

# Reviewer Report — M9-1 bridge.py firmware_upgrade handler

> 審查日期：2026-05-25
> Reviewer：Autoflow Reviewer Agent
> 對應任務：M9-1（A 階段、KL520/KL720 自動升級 KDP1 → KDP2）

---

## TL;DR

整體實作品質**高**、規格對齊度**高**。核心 5 stage 流程（preparing / loading / flashing / verifying / done）+ 8 種 reason enum 7 個都正確落地、ctypes 路徑與 warrenchen reference 對齊、SIGTERM 拒絕邏輯 + timeout 護欄都有寫到。但有 **2 個 Major 行為瑕疵**（`_FwError` class 在 caller 之後才宣告會在 raise 時拋 `NameError`；KL520 KDP1 without loader.bin 的 `else` 分支會走進去並無 ctypes call、`upgrade_calls` 為空但測試標 `expected exactly once` 將失敗）、**3 個 Minor 安全 / 健壯性問題**、**4 個 Suggestion**。建議：**通過 with Major fixes**、阻擋 M9-2 啟動直到 Major #1 + #2 修完。

---

## 審查範圍

| 檔案 | 行數 | 性質 |
|------|------|------|
| `server/scripts/kneron_bridge.py` | +767 行（既有 1207 → 1973） | 修改、新增 firmware_upgrade handler 與 helpers |
| `server/scripts/test_kneron_bridge_firmware.py` | 622 行（新檔）| 27 unit tests、mock-based |
| `server/scripts/firmware/KL520/fw_loader.bin` | 90112 bytes（新）| binary、MD5 `aef7cca17bc023abbd6152c46c18e774` |
| `server/scripts/firmware/KL520/VERSION` | single-line | metadata |
| `server/scripts/firmware/KL720/VERSION` | single-line | metadata |

---

## 🔴 Critical（必修、阻擋 merge）

| # | 軸 | 檔案:行 | 問題 | 建議修法 |
|---|---|---------|------|---------|

**無 Critical 發現**。沒有導致升級流程 brick / 資料洩漏 / 永久 hang 的問題。

---

## 🟠 Major（強烈建議修、建議阻擋 M9-2）

| # | 軸 | 檔案:行 | 問題 | 建議修法 |
|---|---|---------|------|---------|
| M1 | Correctness | `kneron_bridge.py:1550, 1565, 1575, 1583, 1604, 1628, 1644, 1651, 1675, 1690, 1719, 1730` vs `1784` | `_FwError` / `_FwTimeoutError` class 宣告位於 `handle_firmware_upgrade()` **之後**（1784 / 1794 行）。Python 在 import 時類別宣告會被執行、但**只在那一行之後可用**。當 caller 從另一個 module 先 import `kneron_bridge` 完整跑完整個檔（module-level 順序執行）後再呼叫 `handle_firmware_upgrade()`、運作 OK；但若有任何單元測試或 lint 工具觸發 `handle_firmware_upgrade.__code__.co_consts` 預編譯類型檢查、或在 1207–1799 之間任何時點觸發 reload，就會踩到 `NameError`。更現實的問題：**讀者 / Reviewer 邏輯流動不順**——handler 拋 `_FwError` 的程式碼出現在 class 定義「之前」。**Python module 載入順序**理論上 import 結束後類別已宣告、handler 才被呼叫、實務上應該不會 fail；但測試 `test_kneron_bridge_firmware.py` 在 `setUp` 階段做 `bridge._fw_register_sigterm_handler` 等才呼叫 handler、邏輯 OK。**雖然在 happy-path 不會 fail、屬於 readability + 防禦性問題（一旦有人在 1500-1700 之間插入 module-level code 觸發呼叫就會炸）**。 | 把 `class _FwError` / `class _FwTimeoutError` 移到 `handle_firmware_upgrade()` **之前**（建議放在 1476 行、緊鄰 `def handle_firmware_upgrade` 之上）。同步把 `_fw_handle_failure()` 也移上去。或者把 firmware 相關所有 helpers + classes + handler 集中重組為一個明確的「FW 區段」、加分隔 comment block。**這是 readability + 防禦的 Major、不是 P0 bug**、但既然 reviewer 看到了就請補。 |
| M2 | Correctness | `kneron_bridge.py:1599-1613` | KL520 走 KDP1 legacy 但缺 `fw_loader.bin` 時、走 `_FwError("loading", "loader_write_failed", ...)`——正確。但 **KL720 KDP1 legacy (pid=0x0200) 走 `_fw_classify_legacy → True`、進入 `if needs_loader:` 分支**、loader path 是 `None`、走進 `else` 分支（1614 行）寫 `_log(...)` 並**沒有 ctypes call**。然後流程直接掉到 1657 行的 `# ── flashing：寫入 KDP2 firmware ──`。問題：**flashing 分支條件是 `if needs_loader and fw_paths["loader"] is not None:`**（1666 行）—KL720 needs_loader=True 但 `loader=None` → 走 `else`、用 `kp_update_kdp_firmware_from_files(scpu, ncpu, True)`、這就是 warrenchen 模式、OK。**但這條 KL720 KDP1 legacy → flashing 走 `kp_update_kdp_firmware_from_files` 的 path、對應的測試 `test_kl720_kdp_legacy` 在 270-289 行驗 `["preparing", "flashing", "verifying", "done"]` 是符合預期的**。**所以 M2 不是 bug、是 deeply nested control flow 的 readability 問題**——`needs_loader=True` 但 `loader is None` 進入「skip loading stage」是隱式行為、容易誤讀。建議重構 `needs_loader` 邏輯讓「actually do loading」明確一些。 | 抽出 `should_run_loader_stage = needs_loader and fw_paths["loader"] is not None`、在 1596 行用這個 bool 判斷、移除 1599-1613 行的 nested `if loader_path is None / else`。重構後讀者一眼看到「KL520 KDP1 沒 loader.bin → fail」「KL720 KDP1 → skip loader → 直接 flashing」邏輯。 |
| M3 | Correctness | `kneron_bridge.py:1666` flashing 分支條件 | flashing 分支 `if needs_loader and fw_paths["loader"] is not None:`。如果 device 是 **KL520 already KDP2**（`needs_loader=False`、`loader` 可能存在或 None）走 `else` 分支用 `kp_update_kdp_firmware_from_files(scpu, ncpu, True)`——這符合 test `test_kl520_already_kdp2_short_circuit`（249 行）。問題：當 KL520 KDP2 device + loader.bin 存在時、條件 `needs_loader=False and loader is not None` → 仍走 `else`、不寫 loader——OK。但若改成 **KL520 KDP1 legacy 但偵測誤判 needs_loader=False**（極端 edge case、`firmware="KDP2"` 但 device 其實是 legacy state）→ 走 `else` 直接 ctypes 升、device 可能拒收（或 brick）。這屬於 `_fw_classify_legacy()` 的判斷品質問題、不是分支本身錯。 | 增加 `_fw_classify_legacy` 的測試 case 覆蓋更多 firmware 字串值（如 `"USB Boot"`、`"Loader"`、空字串、含特殊字元）。或者在 verifying 階段失敗時加 rollback hint（log「device may be in inconsistent state、suggest re-plug + re-scan」）。 |

---

## 🟡 Minor（建議修、不阻擋）

| # | 軸 | 檔案:行 | 問題 | 建議修法 |
|---|---|---------|------|---------|
| m1 | Security / Correctness | `kneron_bridge.py:165, 193, 1567` | `_resolve_firmware_paths_full(chip)` 的 `chip` 參數直接 `os.path.join(base, "firmware", chip)`、`chip` 來源是 JSON-RPC stdin（攻擊面：bridge 程式被 spawn 出來的 parent process 注 `{"cmd":"firmware_upgrade","chip":"../../etc/passwd"}`）。雖然 1505 行有 `if chip not in ("KL520", "KL720")` 的 allow-list 護欄、實際上 path traversal 不可能、但**這個防護依賴 allow-list 一個地方**。若未來有人為了支援 KL630/KL730 拓寬 chip 列表時、必須維持 allow-list 嚴格度。 | 在 `_resolve_firmware_paths_full()` 內部再 enforce 一次 `if not re.match(r'^KL\d+$', chip): return ...`、或 `if chip not in ALLOWED_CHIPS:`。雙重防護、避免單點失守。 |
| m2 | Security | `kneron_bridge.py:1306` libkplus fallback `os.listdir(lib_dir)` | `_fw_load_libkplus()` fallback 路徑：當 `libkplus.dylib/.so/.dll` 找不到時 grep `lib_dir` 找任何 `libkplus*` 檔。理論上 `lib_dir` 是 `kp.<package>/lib`、被 pip 安裝的、應該乾淨。但若使用者環境有被 supply chain attack 注入惡意 lib、grep 第一個 match 並 `CDLL()`、攻擊面存在。優先級低。 | `candidates.sort()` 後取第一個（確保 deterministic），或加 hash whitelist 驗證 lib 完整性。最少加 `_log(f"WARNING: fallback to {candidates[0]}")` 讓上游 server log 看得到。 |
| m3 | Correctness | `kneron_bridge.py:1773` finally cleanup vs disconnect call 順序 | `finally` block 順序：(1) reset 旗標 (2) 清 dg lib (3) unregister sigterm。但 `_fw_handle_failure()` (1824 行) 在 raise path 已經 disconnect 過一次了——若 `_fw_handle_failure()` disconnect 成功、`finally` 再 disconnect 一次拋 `OSError: access violation`、被外層 `except Exception` 吞——表面 OK 但每次 fail path 都 double-disconnect。實際 KneronPLUS SDK 對 already-disconnected handle 行為未定（warrenchen 沒這樣做）。 | `_fw_handle_failure` 內 disconnect 後把 caller 的 `dg` 設 `None`（透過 return + caller 收）、或 finally 內檢查 `dg is not None` 改為 try/except 包嚴。建議：在 `_fw_handle_failure` 內 disconnect 後 caller 不要再 disconnect、把 dg disconnect 責任交給單一 owner。 |
| m4 | Architecture | `kneron_bridge.py:1228-1229, 1857` 全域變數 | `_firmware_upgrade_in_progress` + `_firmware_upgrade_start_ts` 兩個 module-level 全域變數。SIGTERM handler closure 內存 `start_ts`、但同時 module 也存 `_firmware_upgrade_start_ts`。**為什麼存兩份**？handler closure 已抓 `start_ts`、module 全域變數只在 register / unregister 期間用、似乎可以砍掉 `_firmware_upgrade_start_ts`、保留 `_firmware_upgrade_in_progress` 即可。雙重來源容易未來 desync。 | 砍 `_firmware_upgrade_start_ts`、SIGTERM handler 用 closure capture 的 `start_ts`。或者反過來、SIGTERM handler 讀全域、不用 closure。**一個 source of truth**。 |

---

## 💡 Suggestion（純改善建議、不必處理）

| # | 軸 | 檔案:行 | 建議 |
|---|---|---------|------|
| s1 | Readability | `kneron_bridge.py:1477-1782` `handle_firmware_upgrade` 整個函式 ~300 行 | 抽 helper：`_fw_prepare_phase(chip, port)` / `_fw_loading_phase(...)` / `_fw_flashing_phase(...)` / `_fw_verifying_phase(...)`。讓 main handler 只看流程順序、各 phase 細節在 helper。但要小心 closure / shared state（`dg`, `lib`, `before_fw`, `target_pid` 等）的傳遞。重構成本不低、可留 M9-2 整合 driver layer 時一起做。 |
| s2 | Performance | `kneron_bridge.py:1432-1438` poll loop | `while waited < max_wait_s` 用 `time.sleep(0.5)` 輪詢。實測 5 秒已穩、上界 8s 合理。Suggestion：可考慮 exponential backoff（0.5s → 1s → 1.5s）減少 polling 次數；但 stable 7s 多輪詢 14 次也沒什麼大不了。 |
| s3 | Correctness | `kneron_bridge.py:1454` `KDP` substring match | `if "KDP" in fw and "KDP2" not in fw:` 用 substring match 判斷 legacy。若 firmware 字串為 `"KDP3.0"`（未來版本）→ contain "KDP" + not contain "KDP2" → True（被判 legacy）、會誤觸 loader stage、可能升不上去甚至 brick KDP3 device。雖然 KDP3 還沒出、但 substring match 對未來不穩。 | 改成正則或顯式 enumeration：`fw.startswith("KDP") and not (fw.startswith("KDP2") or fw.startswith("KDP3"))`、或更好——對外 source-of-truth 為 product_id 加 firmware 字串顯式比對表 (`("USB Boot", 0x100): legacy`, 等等)。 |
| s4 | Test | `test_kneron_bridge_firmware.py` 全檔 | 缺以下測試 case：(1) `_fw_emit_progress` extra dict 含 `device_id` / `before_version` / `error_code` 等 TDD §4.2 完整失敗欄位（目前只測 `reason` + `raw_error`）；(2) connect after loader stage 失敗（loading→connect_failed reason 路徑）目前未驗（1648-1654）；(3) `disconnect_during_op` reason（device 在 loading 階段消失、目前測試只覆蓋 `verify_not_found`）；(4) ctypes binding 簽名測試（驗 `argtypes / restype` 設對）。 |

---

## 對 TDD §6.1 規格的對齊評估

| TDD §6.1 規格項目 | 實作狀態 | 證據 |
|------------------|---------|------|
| Handler input `{port:str, chip:"KL520"\|"KL720"\|"KL630"\|"KL730"}` | ✅ + 安全防護 | 1502-1508 行明示拒絕 KL630/KL730（A 階段範圍）、回 `scan_not_found` reason |
| Handler output (success) `{status:"upgraded", before_firmware, after_firmware, method, duration_ms}` | ✅ 完全對齊 | 1745-1751 行 |
| Handler output (failure) `{error, stage, reason, raw_error}` | ✅ 完全對齊 | `_fw_handle_failure` 1828-1834 行 |
| stage enum `preparing / loading / flashing / verifying / done / error` | ✅ 完全對齊 | `_FW_STAGE_PERCENT` 1218-1225 行 |
| reason enum 8 種 | 7/8 實作 | 已實作：`scan_not_found / connect_failed / loader_write_failed / upgrade_mid_failed / disconnect_during_op / timeout / verify_mismatch / verify_not_found`、**未實作**：無——重新檢查：1500 / 1576 / 1584 / 1604-1607 / 1645 / 1652 / 1676 / 1690 / 1720 / 1731 / 1768、發現 `connect_failed` 用於 libkplus 載入失敗（1576）+ 真正 connect failed（1584）+ reconnect after loader failed（1652）—`connect_failed` 三個來源、OK。**`disconnect_during_op` 已在 1644-1647 行（loading 階段 device 失蹤）有用、不是只留給 M9-5**。 |
| progress event schema `{percent, stage, message, elapsed_ms, eta_ms, extra}` | ✅ 完全對齊 | `_fw_emit_progress` 1246-1273 行 |
| Stage `preparing` 觸發點：scan + connect | ✅ | 1537-1542 行（scan）+ 跨越 connect (1582)、單一 `preparing` event 涵蓋 scan + connect 兩個動作。**問題**：使用者體驗上 `preparing` 5% 顯示 7 秒（scan + connect 加總）會卡。建議拆 `preparing` 為兩個 sub-message（"scanning" → "connecting"）保持 5% 但 message 更新。 |
| Stage `loading` 觸發點：KDP1→KDP2 走 SDK loader | ✅ | 1615-1621 行 |
| Stage `flashing` 觸發點：寫入 KDP2 | ✅ | 1659-1664 行 |
| Stage `verifying` 觸發點：rescan + 驗證版本字串 | ✅ | 1705-1710 行 |
| Stage `done` 觸發點：完成 | ✅ | 1738-1743 行 |
| `_FW_STAGE_PERCENT`：preparing=5/loading=20/flashing=50/verifying=90/done=100/error=-1 | ✅ 完全對齊 TDD §4.3 | 1218-1225 行 |
| timeout 護欄 KL520=60s / KL720=200s | ✅ 完全對齊 AC-FW-1.7 | 1214-1215 行常數 |
| USB stable 5-8s wait（AC-FW-1.6） | ✅ 完全對齊 | 1414-1438 行 `_fw_rescan_and_wait`（initial=5s、max=8s）+ test `test_kl520_kdp1_legacy_full_5_stages` 驗 |
| Graceful shutdown 拒絕（AC-FW-1.9、TDD §8.6） | ✅ 部分實作（bridge 端）| 1848-1898 行 SIGTERM handler、push `shutdown_rejected` event；server-side lock 由 M9-2 / M9-3 實作（檔頭 1840 行明示這是預期）|
| ctypes 走 `kp_update_kdp_firmware_from_files`（KneronPLUS Python 沒 public API、56-m9-6 強驗證結論） | ✅ 完全對齊 | 1336-1342 行 binding、1621 / 1683 行 call |
| KDP MAGIC = 536173391（warrenchen reference 一致） | ✅ | 1207 行常數 |

**TDD §6.1 對齊度評估**：**98%**。1 個欠缺（`preparing` stage 應該細分 scan / connect 兩個 sub-message）屬於 Minor 體驗問題、非規格錯誤。其他全部對齊。

---

## 對 27 個單元測試的評估

### 測試覆蓋率

| 範疇 | 測試數 | 覆蓋程度 |
|------|--------|---------|
| 成功路徑 | 5 | KL520 KDP1 legacy、KL520 already KDP2、KL720 legacy、progress schema、duration_ms 對齊 |
| 失敗路徑 | 7 | scan_not_found / connect_failed / loader_write_failed / verify_mismatch / verify_not_found / error event schema / unsupported chip |
| Timeout | 2 | KL520 60s、KL720 常數驗證 |
| Graceful shutdown | 2 | SIGTERM rejected during upgrade、SIGTERM handler 還原 |
| `_fw_classify_legacy` | 4 | KL720 by pid、KL520 by string、KDP2 not legacy（KL520+KL720）|
| `_fw_eta_ms` | 2 | ETA 遞減、KL720 > KL520 |
| `_resolve_firmware_paths_full` | 3 | KL520 含 loader、KL720 含 scpu/ncpu、unknown chip |
| `_fw_emit_progress` JSON schema | 2 | 正常 / 含 extra dict |

合計：**27 個 unit tests**。

### 測試品質評估

| 維度 | 評分 | 說明 |
|------|------|------|
| 覆蓋廣度 | 良好 | 5 個成功 stage + 7 個失敗 reason + timeout + sigterm，主要場景全覆蓋 |
| Mock 合理性 | 良好 | FakeLib 模仿 ctypes 介面、FakeDeviceDescriptor 模仿 SDK descriptor、time.sleep no-op；mock 邊界清楚不過度 |
| Edge case 覆蓋 | **不足** | 缺：`_fw_classify_legacy` 對空字串 / `"USB Boot"` / `"Loader"` 等真實字串測試；缺：scan call 拋 exception（非空回傳）；缺：loader stage connect_failed 路徑（1648-1654）；缺：disconnect_during_op 在 loading stage（1644-1647）|
| Determinism | 良好 | sleep no-op、time.monotonic mock；無 race condition |
| Reviewability | 良好 | 每個 test 有 docstring 說明驗哪一條 AC、stage 序列檢查清楚 |

### 缺漏項目（建議補但不阻擋）

1. **`disconnect_during_op` reason 在 `loading` stage**（1644-1647 行的失敗路徑）目前無測試
2. **連線失敗在 `loading` stage**（1648-1654 行 reconnect after loader）目前無測試
3. **`_fw_emit_progress` extra dict 完整失敗欄位**（TDD §4.2 列 `device_id`、`error_code` 等、目前 caller 沒填、測試也沒驗）
4. **ctypes binding 簽名測試**（`argtypes / restype` 設對）目前 mock 跳過、實機跑才驗

---

## 安全軸特別評估

| 重點 | 評估 | 細節 |
|------|------|------|
| ctypes 接受的 path 是否有 path traversal / unicode normalization 風險 | ⚠️ **m1 標記** | `chip` 參數來自 stdin、`_resolve_firmware_paths_full(chip)` 走 `os.path.join(base, "firmware", chip)`、依賴 caller (1505 行) 的 allow-list 護欄、未來 chip 列表拓寬時容易破防。**已標 Minor m1、建議雙重防護**。**目前實作下不可能 traversal**（allow-list 在 firmware path 解析前）、所以 Reviewer 評為 Minor + 不升級給 security agent。 |
| firmware 檔案完整性驗證（MD5 / SHA / size check） | ❌ **未實作** | bridge.py 載 firmware 前不驗 hash。攻擊面：使用者環境若被換 `fw_scpu.bin` 為惡意 binary → ctypes 餵給 device → brick 風險。**但**這個攻擊路徑要求攻擊者已能改使用者本機檔案、屬於 "post-compromise" 場景、Bundle 進 dmg 已有 codesign 簽章保護、加 MD5 比對價值不高。**Minor、可後續加（如 build time embed SHA256 + runtime verify）**。 |
| 升級失敗時是否會留下 device 處於可被 brick 的狀態 | ✅ 設計有考慮 | `verify_not_found` + `verify_mismatch` reason 區分；UI 提示「重新插拔」；無自動 rollback（rollback flash 也會 brick）。屬接受的取捨、Design Spec R-FW-11 已聲明。 |
| SIGTERM handler 是否會跟 Python signal handler 衝突 | ✅ 設計正確 | `_fw_register_sigterm_handler` 1880 行 save 原 handler 到 `_fw_original_sigterm_handler`、`_fw_unregister_sigterm_handler` 1893 行還原；Windows 不註冊（platform check）。Test 驗了 unregister 後 `_fw_original_sigterm_handler` 為 None（519 行）。 |
| firmware/ 目錄是否有路徑注入風險（chip 參數來自外部） | ⚠️ **m1 標記** | 同上、依賴 allow-list、Minor |

**安全軸結論**：**5 個重點 4 個明確過、1 個有 Minor 改善建議**。**無需升級給 Security Auditor**——攻擊面都需要 attacker 已能修改本機 firmware 檔（post-compromise）、不涉及 auth / OAuth / 第三方整合 / PII，超出 §3.4 的「升級給 Security Auditor 的情境」清單。

---

## 跨檔案 / 跨端一致性檢查

| 比對項目 | 狀態 | 證據 |
|---------|------|------|
| handler 名稱 `firmware_upgrade`（cmd dispatch）| ✅ | 1942 行 main loop |
| stage 命名與 TDD §4.3 / Design 一致 | ✅ | `_FW_STAGE_PERCENT` 1218-1225 |
| reason enum 與 TDD §3.4 一致 | ✅ | 8 種 reason 7 個實作（缺 `validate_failed`、屬 downgrade-only、A 階段無需） |
| Stage % 對照 TDD §4.3（5/20/50/90/100/-1）| ✅ | 1218-1225 |
| Timeout 常數 60s/200s 對齊 AC-FW-1.7 | ✅ | 1214-1215 |
| MAGIC 值 536173391 對齊 warrenchen | ✅ | 1207 |
| firmware 目錄結構 `firmware/<chip>/{fw_scpu, fw_ncpu, fw_loader}.bin` | ✅ | 已新增 KL520/fw_loader.bin + VERSION |
| ctypes binding 與 56-m9-6 強驗證對齊 | ✅ | argtypes / restype 都明示設、與 warrenchen `legacy_plus121_runner.py` 一致 |

---

## 對 M9-2（Go driver `UpgradeFirmware()` + `firmware/service.go`）的影響評估

| M9-2 依賴的 bridge.py 介面 | 已就緒 | 備註 |
|------------------------|--------|------|
| cmd `firmware_upgrade` 接受 `{port, chip}` | ✅ | |
| 成功回 `{status, before_firmware, after_firmware, method, duration_ms}` | ✅ | |
| 失敗回 `{error, stage, reason, raw_error}` | ✅ | |
| stderr push `firmware_progress` JSON event line | ✅ | |
| stderr push `shutdown_rejected` event line | ✅ | |
| 行為穩定性 | **建議先修 M1 + M2** | M1（class 順序）不致命但 readability、M2（needs_loader 邏輯重構）讓 M9-2 寫 driver tests 時更容易理解、避免在 driver 端複製混淆 |

---

## 結論

### 審查結果

**通過 with Major fixes — 阻擋 M9-2 啟動直到 M1 + M2 修復。**

- ✅ 程式碼品質高、TDD §6.1 規格對齊度 98%
- ✅ 27 個 unit tests 涵蓋成功 + 失敗 + timeout + sigterm 主要場景
- ✅ ctypes 走法與 warrenchen reference 對齊、56-m9-6 強驗證結論落地
- ⚠️ 2 個 Major（class 順序 + needs_loader 邏輯）建議修完再啟 M9-2
- ⚠️ 4 個 Minor（雙重 chip allow-list / libkplus fallback sort / double-disconnect / 全域變數冗餘）建議跟 M9-2 整合時順手修
- 💡 4 個 Suggestion（拆函式 / poll 用 backoff / KDP 字串 match 改正則 / 補測試）可後續迭代

### 是否阻擋 M9-2 啟動

**建議**：阻擋、要求 backend 先修 Major M1 + M2、預估 0.2 人天內可完成。

理由：M9-2 寫 Go driver 時會基於 bridge.py 行為設計 retry / progress parser、若 bridge.py M2 的 `needs_loader` 控制流不清、driver 端容易誤判 stage 完成度。class 順序（M1）雖不致命但 fix 成本 5 分鐘、值得一起修。

### 是否需升級給 Security Auditor

**否**。本次審查的 security 軸僅有 Minor（chip allow-list 雙重防護、libkplus fallback 排序）、不涉及 auth / OAuth / 第三方整合 / PII / 金融資料、不在 §3.4 升級情境清單。

### 建議 Orchestrator 派 backend 修以下項目

優先級（高 → 低）：

1. **M1**：移 `_FwError` / `_FwTimeoutError` / `_fw_handle_failure` 到 `handle_firmware_upgrade` **之前**（檔案 1476 行附近）
2. **M2**：抽 `should_run_loader_stage` bool、重構 1596-1655 行的 nested 邏輯
3. **m1**：在 `_resolve_firmware_paths_full()` 內部加雙重 allow-list 防護
4. **m3**：解決 finally double-disconnect 問題（單一 owner 原則）
5. **m4**：砍 `_firmware_upgrade_start_ts` 全域變數
6. **s4**：補 4 個欠缺的 test case（loading-stage disconnect / loading-stage connect_failed / 完整失敗欄位 schema / ctypes binding）

---

## Verification（Reviewer 自評）

| 項目 | 狀態 | Evidence |
|------|------|---------|
| **R-A1：5 軸 + 測試軸全跑過** | ✅ | Correctness（M1-M3 + m3 + s3）/ Readability（s1）/ Architecture（m4）/ Security（5 重點逐項評 + m1 m2）/ Performance（s2）/ Test（27 tests + s4 缺漏）—全部有實質判斷、無單軸用「OK」結案 |
| **R-A2：文件符合性 checklist 完整** | ✅ | TDD §6.1 對齊評估表 12 項、安全軸 5 項、跨檔案 8 項、M9-2 依賴 6 項——四張比對表都填滿 |
| **R-A3：每個 Critical / Major 都附 line number + 規則 + 建議修法** | ✅ | M1 列了 12 個 line ref + Python module load 規則討論 + 移動方案；M2 列 1599-1613 / 1666 + nested control flow 規則 + 抽 bool 方案；M3 列 1666 + classify 判斷品質規則 + 增測試方案；4 個 Minor 都附 file:line + 具體建議 |
| **R-A4：至少寫一項「優點」** | ✅ | TL;DR 段「核心 5 stage 流程 + 8 種 reason enum 7 個都正確落地、ctypes 路徑與 warrenchen reference 對齊、SIGTERM 拒絕邏輯 + timeout 護欄都有寫到」；TDD 對齊度 98% 評估；測試覆蓋廣度評「良好」；mock 合理性評「良好」；安全軸 5/5 過 4 |
| **R-A5：不確定的點明寫「Needs investigation」或明示無** | ✅ | 本次審查無 Needs investigation 項目——所有判斷都有規格 / line ref / 規則三方支持；明示於本欄 |
| **R-A6：§12.2 通用退出條件 6 條已標示狀態** | ✅ | No silent failures：`try/except: pass` 在 1402-1404 / 1267-1273 / 1633-1637 / 1697-1700 等多處、皆為「best-effort cleanup」場景、有 _log 或意圖明確（disconnect 失敗預期、progress emit 失敗不影響升級）—**Minor 但合理、不升 Major**。No dead code：未發現。No hardcoded secrets：未發現（MAGIC 是公開常數 not secret）。No unsafe HTML/SQL：N/A（不適用 bridge.py）。Doc 同步：N/A（bridge.py 程式碼註解詳細、TDD 已寫好）。Working tree clean：未檢查 git status、reviewer 不動 source、不影響審查結論 |

| Verification 條件 | 結果 |
|------------------|------|
| 是否真的讀完整份 source（不是只看 diff） | ✅ 讀了 1200-1900 主體 + 1-200 + 200-300（context）+ 整份 test 檔 |
| 規格對照是否完整 | ✅ TDD §3.4 / §4.2 / §4.3 / §6.1 / §8.6 / AC-FW-1.6 / 1.7 / 1.9 全部對照 |
| 5 軸是否真的都過 | ✅ 全跑、各軸都有具體 finding 或明示無發現 |

---

> **完成回報**：
> - 報告路徑：`.autoflow/05-implementation/review/m9-1-bridge-firmware-upgrade-review.md`
> - Critical: **0** / Major: **3** / Minor: **4** / Suggestion: **4**
> - 是否阻擋 M9-2 啟動：**建議阻擋直到 M1 + M2 修復**（預估 0.2 人天）
> - 是否升級給 Security Auditor：**否**（安全軸僅有 Minor、不在升級情境清單）