reviewer

Phase 2 Worktree Integration — 架構分析報告

Agent: reviewer (reviewer)
Confidence: 65%
Cost: $0.7702
Duration: 2m 14s
Task ID: df024484-dde7-4986-a921-60298f4b3117

好的，我已完成所有程式碼的深入閱讀。以下是我的完整 Code Review 報告。

Phase 2 Worktree Integration — 架構分析報告

概述

Phase 2 的核心改動邏輯正確：shouldUseWorktree() 根據 capability 判斷是否建立 worktree，executeTask() 中有 graceful fallback，dispatch_task 正確繼承 parent 的 worktree。但有 1 個必須修的 bug 和 2 個中風險設計問題 需要關注。

分析詳情

1. 🔴 BUG — Worktree 永遠不會被清理（Leak）

嚴重度：5/5

worktree-manager.ts:225 中 cleanupOrphanWorktrees() 呼叫 removeTaskWorktree(wt.shortId)，但 removeTaskWorktree() 的參數名為 taskId，內部會再做一次 getShortId(taskId)（即 taskId.slice(0, 8)）。

當傳入的已經是 8 字元的 shortId（如 abc12345），getShortId() 會截取前 8 字元，恰好等於自身，所以路徑計算結果正確——這裡不是 bug。

但更嚴重的問題是：在正常 task 完成流程中，archiveTask() 完全沒有呼叫 removeTaskWorktree()。

  • worker-scheduler.ts:1291 archiveTask() 只做 JSONL append + queue 清除
  • executeTask() 完成後只做 report、stats、archive，沒有任何 worktree cleanup
  • 唯一的清理機制是 cleanupOrphanWorktrees()，但它從未被任何地方呼叫

結果：每個 code agent task 都會建立一個 worktree，但永遠不會被主動清理。MAX_WORKTREES=4 後就無法再建立，worktree 只能靠 TTL 2 小時後被標記為 orphaned——但 orphan cleanup 也沒有排程。

建議：

  1. 在 executeTask() 的 finally block 中加入 removeTaskWorktree(task.id)（如果 task.worktreePath 存在）
  2. 在 startWorkerScheduler() 或 heartbeat tick 中定期呼叫 cleanupOrphanWorktrees()

2. 🟡 architect 不建 worktree 但有 refactoring capability

嚴重度：3/5

shouldUseWorktree() 檢查 ['code', 'refactoring']。但 architect.json 的 capabilities 包含 "refactoring"。

根據變更摘要：「刻意排除 architecture 避免 reviewer 誤建」。但代碼中排除的不是 architecture capability，而是把觸發條件設為 code 和 refactoring。architect 有 refactoring 但沒有 code——所以 architect 會觸發 worktree 建立。

這可能是 intentional（architect 確實會寫 code），但與摘要描述「刻意排除 architecture」矛盾。需確認意圖。

受影響的 agent（會觸發 worktree）：

Agent Capability Match
programmer code
backoffice-op code
mcp-tools-op code
site-op code
architect refactoring

3. 🟡 bot-tools-server.ts 中 parent lookup 做了兩次

嚴重度：2/5

bot-tools-server.ts:573 和 bot-tools-server.ts:641 都對同一個 parentTaskId 做了 queue.tasks.find() + findTaskInHistory()。第一次是在 chain depth 計算中，第二次是在 worktree 繼承中。

1
2
3
4
5
6
7
// Line 573: chain depth check
const parentInQueue = queue.tasks.find(t => t['id'] === parentTaskId);
const parent = parentInQueue ?? await findTaskInHistory(parentTaskId);

// Line 641: worktree inheritance (same lookup again)
const parentInQueue = queue.tasks.find(t => t['id'] === parentTaskId);
const parentForWt = parentInQueue ?? await findTaskInHistory(parentTaskId);

findTaskInHistory() 會讀整個 history.jsonl 做反向掃描。在高頻 dispatch 場景下，同一個 parentTaskId 被讀了兩次。

建議：把 parent lookup 提到前面做一次，後面共用。

4. 🟢 正面設計評價

以下設計決策值得肯定：

  • Dynamic import worktree-manager.js（worker-scheduler.ts:477）：避免未啟用 worktree 時 import 全部依賴，且 import 失敗不影響執行
  • Graceful fallback（worker-scheduler.ts:483-489）：worktree 建立失敗就 fallback 到 LIGHTWEIGHT_CWD，不會阻斷任務
  • MCP 路徑不跟 worktree（worker-scheduler.ts:496）：MCP config 是靜態的，正確保持指向 LIGHTWEIGHT_CWD
  • shouldUseWorktree() 先檢查 task.worktreePath（worker-scheduler.ts:352）：已繼承 worktree 的就不重建，避免浪費
  • worktree 繼承邏輯放在 task push 之前（bot-tools-server.ts:639-647）：確保 queue 中的 task 已帶有 worktree 資訊

風險評估

  • 🔴 高風險：Worktree 從不清理 → 4 個 worktree 後 code agents 全部降級到 LIGHTWEIGHT_CWD（功能性退化）
  • 🟡 中風險：architect 意外觸發 worktree 建立（可能是 intentional，需確認）
  • 🟡 中風險：parent lookup 重複 I/O（效能）
  • 🟢 低風險：type annotation 使用 string | null 而非 string | undefined（與 AgentTask interface 不一致但無功能影響）

驗收結論

❌ 驗收不通過

必須修復 Issue #1（worktree 清理缺失）後才能 commit。這是一個 resource leak，在生產環境會快速耗盡 worktree quota。

需要修正的內容

必須修（阻塞 commit）

  1. worker-scheduler.ts — executeTask() 中加入 worktree cleanup

    • 位置：executeTask() 函數末尾（所有 try/catch 結束後）
    • 在 task 完成或失敗後，如果 task.worktreePath 存在且 task 不是從 parent 繼承的 worktree（即 task 是 worktree 的建立者），呼叫 removeTaskWorktree(task.id)
    • 注意：pipeline 中下游繼承的 worktree 不應由下游清理——只有建立者（最上游 code agent）負責清理

  2. worker-scheduler.ts 或 heartbeat — 定期呼叫 cleanupOrphanWorktrees()

    • 建議在 handleTick() 中每 N 次 tick 呼叫一次，作為安全網

建議改善（不阻塞 commit）

  1. bot-tools-server.ts — 合併重複的 parent lookup
  2. 確認 architect 是否應該建立 worktree——如果不應該，把 refactoring 從 CODE_MODIFY_CAPABILITIES 移除，或在 shouldUseWorktree() 中顯式排除 architecture capability

—HANDOFF—
TO: programmer
INTENT: feedback
ARTIFACT_TYPE: review
SUMMARY: Phase 2 worktree integration 驗收不通過 — worktree 建立後從未被清理（resource leak），需在 executeTask() 和 heartbeat 中加入 cleanup 邏輯