release: 將 AI 程式碼審查 action 發布到 master #2
+137
-169
@@ -7,52 +7,52 @@
|
|||||||
"suggestion": "只 resolve 可證明由本 bot 建立且格式符合預期的 comment,例如檢查作者、固定 marker、review body 簽章或 botFinding 解析結果;人類留言與未知格式留言不得自動關閉。",
|
"suggestion": "只 resolve 可證明由本 bot 建立且格式符合預期的 comment,例如檢查作者、固定 marker、review body 簽章或 botFinding 解析結果;人類留言與未知格式留言不得自動關閉。",
|
||||||
"is_new": false
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
|
||||||
"level": "critical",
|
|
||||||
"role": "Mage",
|
|
||||||
"location": "src/findings.js:471",
|
|
||||||
"problem": "當排除條目有 location 或 role 時,這裡直接把文字比對結果短路成 true。最小重現:exclusions.json 只有 `{ \"location\": \"app/a.js:10\", \"original_finding\": \"誤報 A\" }`,新的 finding 是 `app/a.js:99` 且 suggestion 完全不同,仍會因同檔案而被排除,導致真問題被靜默丟掉。",
|
|
||||||
"suggestion": "不要用 `exPath || ex.role ? true : textMatches` 跳過文字比對;應至少要求位置精確匹配到同一行,或在同檔/同角色時仍必須通過 `textMatches`,例如 `return locationMatches && roleMatches && textMatches`,並明確定義 suggestion 空白時才是萬用規則。",
|
|
||||||
"is_new": false
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "critical",
|
|
||||||
"role": "Mage",
|
|
||||||
"location": "src/findings.js:382",
|
|
||||||
"problem": "這個排除條件只要命中 `location` 或 `role`,就直接放行,不再檢查文字內容;最小重現:同一個 `app/a.js` 裡有一筆「誤報」排除後,`app/a.js` 的其他不同 finding 也會一起被濾掉。結果是單一排除項目可以吞掉整個檔案的有效問題。",
|
|
||||||
"suggestion": "不要在有 `location` 或 `role` 時跳過文字比對;至少要同時驗證檔案、角色與正規化後的問題文字都匹配才排除,或改成以更穩定的 finding 簽章精準比對。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "critical",
|
|
||||||
"role": "Mage",
|
|
||||||
"location": "src/gitea.js:86",
|
|
||||||
"problem": "這裡只要 `commit` 或 `branch` 訊息包含 `[ai-review-bot]` 就回傳 true,但沒有區分 `[success]` 與 `[failure]`;最小重現:上一輪 bot commit 是 `[ai-review-bot][failure]`,而 `getCommitMessageBySha` 讀取失敗時,流程會被當成「可跳過」直接結束,原本應該讓 workflow 失敗的訊號被吃掉。",
|
|
||||||
"suggestion": "讓這個函式回傳結構化結果,例如 `success / failure / unknown`,或至少在偵測到 `[failure]` 時明確回傳失敗狀態,交由 `main()` 先處理失敗再決定是否跳過。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
"level": "critical",
|
"level": "critical",
|
||||||
"role": "Maya",
|
"role": "Maya",
|
||||||
"location": "src/main.js:44",
|
"location": "src/main.js:44",
|
||||||
"problem": "這個 `main()` 是整個 action 的流程總管,但目前沒有任何整合測試或端到端測試去驗證 Step3~Step11 的分支切換與 `process.exit()` 行為。像是前置驗證失敗、偵測到 bot 自動提交、diff 為空、所有角色分析都失敗、JSON 驗證失敗、出現 critical finding、以及 commit/push 降級路徑,現在都只靠人工推演,實際接線後一旦流程順序或退出條件出錯,現有單元測試抓不到。",
|
"problem": "這個 `main()` 是整個 action 的流程總管,但目前沒有任何整合測試或端到端測試去驗證 Step3~Step11 的分支切換與 `process.exit()` 行為。像是前置驗證失敗、偵測到 bot 自動提交、diff 為空、所有角色分析都失敗、JSON 驗證失敗、出現 critical finding、以及 commit/push 降級路徑,現在都只靠人工推演,實際接線後一旦流程順序或退出條件出錯,現有單元測試抓不到。",
|
||||||
"suggestion": "補一組 `main.test.js`,把各模組依賴都 mock 掉,分別覆蓋 `runPreflight=false`、`shouldSkipBotCommit=true`、`getPRDiff=''`、分析全失敗、JSON 驗證拋錯、filtered 含 critical、push 失敗但流程不中斷等分支,並斷言對應的 exit code、呼叫順序與關鍵 log。",
|
"suggestion": "補一組 `main.test.js`,把各模組依賴都 mock 掉,分別覆蓋 `runPreflight=false`、`shouldSkipBotCommit=true`、`getPRDiff=''`、分析全失敗、JSON 驗證拋錯、filtered 含 critical、push 失敗但流程不中斷等分支,並斷言對應的 exit code、呼叫順序與關鍵 log。",
|
||||||
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "critical",
|
||||||
|
"role": "Assassin",
|
||||||
|
"location": "src/gitea.js:66",
|
||||||
|
"problem": "這裡直接從 PR head 讀取 `.reviewignore`,再拿它當成排除規則。攻擊者可以在自己的分支塞入排除條目,讓 bot 故意跳過包含惡意變更的檔案或整個目錄,等於自己決定哪些地方不被審查。",
|
||||||
|
"suggestion": "不要信任 PR head 裡的 `.reviewignore` 來決定安全掃描範圍;改從受保護的 base branch 或 maintainer 管控的位置讀取,且要與固定的預設排除清單合併,而不是讓它覆蓋預設規則。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "critical",
|
||||||
|
"role": "Assassin",
|
||||||
|
"location": "src/findings.js:352",
|
||||||
|
"problem": "這裡會直接讀取 PR 工作樹中的 `.gitea/ai-review/exclusions.json` 當成可信排除來源。攻擊者可以先在分支裡放一份藏在 `.gitea/` 下的 exclusions 檔,利用被忽略的路徑把自己的問題先排除掉,讓後續的 findings 被靜默吃掉。",
|
||||||
|
"suggestion": "把 exclusions 視為 bot 自己管理的狀態,不要從 PR head 的工作樹直接信任既有內容;應該改成只讀受保護來源,或在載入前驗證檔案確實由 bot 生成且未被 PR 作者預先植入。",
|
||||||
"is_new": true
|
"is_new": true
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
"role": "Assassin",
|
"role": "Assassin",
|
||||||
"location": "src/findings.js:14",
|
"location": "src/llm.js:21",
|
||||||
"problem": "這裡把未信任的 Git diff 直接送進 LLM。攻擊者可以在新增程式碼或註解中塞入提示詞注入內容,例如要求模型忽略安全問題、回傳空陣列或偽造低風險 findings,藉此讓自動安全審查失明。",
|
"problem": "這裡把未清洗的 `userContent` 直接塞進模型提示詞,等於讓 PR 內容、留言內容或其他外部文字能反過來操控 LLM。攻擊者可以在 diff 裡埋入『忽略前述規則、回傳空陣列』這類指令,讓審查模型漏報真正的風險或把嚴重問題降級成誤報。",
|
||||||
"suggestion": "在分析 prompt 中明確標示 diff 是不可信資料,要求模型忽略 diff 內任何指令;同時加入結構化封裝、輸出 schema 驗證與必要的規則式安全檢查,避免完全依賴可被 prompt injection 操控的 LLM 判斷。",
|
"suggestion": "不要把不可信內容當成可執行指令使用。至少要把 diff/留言做更強的結構化封裝與逸出處理,並在輸出端加上嚴格的 JSON schema 驗證與 deterministic guardrail,避免 LLM 直接決定安全性結論。",
|
||||||
"is_new": false
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
"role": "Leo",
|
"role": "Mage",
|
||||||
"location": "src/main.js:36",
|
"location": "src/main.js:69",
|
||||||
"problem": "`main()` 把前置驗證、bot commit 判斷、對話收斂、角色分析、合併去重、排除、發布、JSON 驗證、commit/push 與 gate 全部塞在同一個 190 行左右的函式裡,且中間散落多個 `process.exit()`。六個月後要改其中任一步驟時,很難隔離副作用,也不容易針對單一階段寫單元測試。",
|
"problem": "這裡先檢查 head SHA 對應的訊息是否為 failure,但如果 SHA 查詢失敗或是空值,後面的 `shouldSkipBotCommit()` 仍可能只看到分支 head 上的 `[ai-review-bot]` 標記就直接跳過。最小重現:`getCommitMessageBySha()` 因 Gitea API 暫時失敗回空字串,而分支 head 正好是 `[ai-review-bot][failure]`,流程就會 exit 0,等於把本來應該失敗的 bot commit 當成可跳過的自動提交。",
|
||||||
"suggestion": "將每個 Step 拆成可注入相依、回傳明確結果的函式,例如 `runAnalysisStep()`、`runFilteringStep()`、`runPublishStep()`;最外層再統一把結果轉成 exit code,讓流程控制與業務邏輯分離。",
|
"suggestion": "把「是否跳過」和「是否 failure」分開判斷,或讓 helper 回傳解析出的 outcome;只允許 success 標記走 skip,failure 標記不論 SHA/branch 來源都應優先讓流程失敗。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Rogue",
|
||||||
|
"location": "src/main.js:132",
|
||||||
|
"problem": "這裡把每個角色的 LLM 分析逐一 await,6 個角色就把總耗時堆成約 6 倍單次模型延遲;這些分析彼此獨立,CPU 沒偷到時間,反而把整條 pipeline 卡在序列網路/CLI 呼叫上。",
|
||||||
|
"suggestion": "改用 Promise.allSettled 平行執行 roles.map(role => analyzeWithRole(role, diff)),再彙整 fulfilled 結果與 warning;保留 fulfilledAnalyses 的判斷即可。",
|
||||||
"is_new": false
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
@@ -71,22 +71,6 @@
|
|||||||
"suggestion": "排除條目與 finding 應使用同一套正規化函式比對;例如改存並使用 `normalizeText(ex.text || ex.suggestion || ex.title || '')`,或讓 finding 也轉成同樣的 compact/lowercase key。",
|
"suggestion": "排除條目與 finding 應使用同一套正規化函式比對;例如改存並使用 `normalizeText(ex.text || ex.suggestion || ex.title || '')`,或讓 finding 也轉成同樣的 compact/lowercase key。",
|
||||||
"is_new": false
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Maya",
|
|
||||||
"location": "src/main.js:118",
|
|
||||||
"problem": "Step5 的角色分析與流程分支是整個 action 的核心,但目前測試沒有覆蓋 main orchestrator:例如所有角色分析都失敗時應 exit 1、部分角色失敗時仍繼續、diff 為空時 exit 0、critical finding 最後應讓 workflow 失敗。這些行為沒有被驗證,等於 pipeline 成敗判斷還沒通過試煉。",
|
|
||||||
"suggestion": "補上 main 流程層級測試,透過 mock getPRDiff、loadRoles、analyzeWithRole、postFindingsReview、process.exit 等相依,至少覆蓋:diff 空、全部分析失敗、部分分析失敗但繼續、產生 critical 後 exit 1、無 critical 後正常通過。",
|
|
||||||
"is_new": false
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Maya",
|
|
||||||
"location": "src/main.js:166",
|
|
||||||
"problem": "Step7 會把 reconcile.excludedFindings 追加到 exclusions,接著再載入並套用排除規則,但目前缺少整合測試驗證「誤報對話 → 寫入 exclusions → 後續 findings 被排除」這條關鍵路徑。若 append/load/apply 任一環節接錯 workspace 或 mirror,單元測試不一定會抓到。",
|
|
||||||
"suggestion": "補一個接近流程層級的測試,mock reconcileConversations 回傳 excludedFindings,準備一筆會被排除的新 finding,驗證 appendExclusions 寫入的檔案被 loadExclusions 讀到,且最後 save/post 的 filtered findings 不含該誤報。",
|
|
||||||
"is_new": false
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
"role": "Maya",
|
"role": "Maya",
|
||||||
@@ -106,17 +90,9 @@
|
|||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
"role": "Maya",
|
"role": "Maya",
|
||||||
"location": "src/llm.js:91",
|
"location": "src/llm.js:66",
|
||||||
"problem": "runAssistantCLI 有 timeout 與 maxBuffer 兩條重要失敗路徑,但目前測試只覆蓋 CLI 非零退出,沒有驗證逾時會 kill 子程序並拒絕、輸出超過限制會中止且不產生未處理的重複 reject。這些是 CI 上最常見的失敗情境。",
|
"problem": "`runAssistantCLI()` 目前只有成功與一般失敗的測試,沒有覆蓋 timeout、`maxBuffer` 超限、以及 `opencode` 分支建立的暫存 prompt 檔在例外發生時是否確實清理。這些都是外部 CLI 整合最常出問題的失敗路徑,沒有測到就很難確定不會留下殘檔或把流程卡死。",
|
||||||
"suggestion": "新增 llm 測試:用假的 CLI sleep 超過 AI_ASSISTANT_TIMEOUT_MS,斷言錯誤訊息包含逾時;再用大量 stdout/stderr 超過 AI_ASSISTANT_MAX_BUFFER,斷言錯誤訊息正確且測試過程沒有 unhandled rejection。",
|
"suggestion": "補 fake CLI 測試,讓子程序超時、輸出超過 `AI_ASSISTANT_MAX_BUFFER`、以及 `opencode` 在 `spawn`/`close` 前後失敗,分別斷言會回傳對應錯誤,且暫存目錄與 `prompt.md` 會被清掉。",
|
||||||
"is_new": false
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Rogue",
|
|
||||||
"location": "src/main.js:132",
|
|
||||||
"problem": "這裡把每個角色的 LLM 分析逐一 await,6 個角色就把總耗時堆成約 6 倍單次模型延遲;這些分析彼此獨立,CPU 沒偷到時間,反而把整條 pipeline 卡在序列網路/CLI 呼叫上。",
|
|
||||||
"suggestion": "改用 Promise.allSettled 平行執行 roles.map(role => analyzeWithRole(role, diff)),再彙整 fulfilled 結果與 warning;保留 fulfilledAnalyses 的判斷即可。",
|
|
||||||
"is_new": false
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
@@ -130,25 +106,9 @@
|
|||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
"role": "Rogue",
|
"role": "Rogue",
|
||||||
"location": "src/findings.js:430",
|
"location": "src/findings.js:442",
|
||||||
"problem": "applyExclusions 在 findings × exclusions 的巢狀比對裡,每遇到一條 exclusion 就重算同一個 finding 的 normalizeText;F 筆 finding、E 條 exclusion 會做最多 F×E 次正規化與正則替換,這是很明顯的 CPU 浪費。",
|
"problem": "這裡每一筆 finding 都要跟整包 exclusions 做一次 `.some()`,而且內層還反覆跑 `normalizeText` 和字串包含比對,資料一多就直接變成 O(F×E) 的熱點。像 300 筆 finding 配 500 筆 exclusions,會吃掉 15 萬次以上的比對與正規化,CPU 和字串配置都在浪費。",
|
||||||
"suggestion": "先把 findings 預處理成含 fPath、normalizedFindingText 的陣列,exclusions 也先補齊 normalizedExclusionText,再做比對;同一筆文字只正規化一次。",
|
"suggestion": "先把 exclusions 在載入時一次正規化並依 `filePath / role / textKey` 建索引,讓過濾改成近似 O(F);至少把 `normalizeText` 移到內層迴圈外,避免同一段字串被重算成百上千次。",
|
||||||
"is_new": false
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Assassin",
|
|
||||||
"location": "src/llm.js:21",
|
|
||||||
"problem": "這裡把未清洗的 `userContent` 直接塞進模型提示詞,等於讓 PR 內容、留言內容或其他外部文字能反過來操控 LLM。攻擊者可以在 diff 裡埋入『忽略前述規則、回傳空陣列』這類指令,讓審查模型漏報真正的風險或把嚴重問題降級成誤報。",
|
|
||||||
"suggestion": "不要把不可信內容當成可執行指令使用。至少要把 diff/留言做更強的結構化封裝與逸出處理,並在輸出端加上嚴格的 JSON schema 驗證與 deterministic guardrail,避免 LLM 直接決定安全性結論。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Leo",
|
|
||||||
"location": "src/findings.js:111",
|
|
||||||
"problem": "`normalizeText` 與 `toKeyText` 兩套正規化規則不一致,前者會轉小寫,後者不會,而且註解還直接寫了「不確定」。這會讓排除、去重、比對在不同路徑出現微妙分歧,半年後很難追出到底是哪個標準才是正確來源。",
|
|
||||||
"suggestion": "把文字正規化抽成單一共用 helper,讓大小寫是否敏感變成明確參數或不同命名的意圖函式,並補上覆蓋兩種路徑的測試,避免未來兩套規則繼續漂移。",
|
|
||||||
"is_new": true
|
"is_new": true
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
@@ -157,39 +117,7 @@
|
|||||||
"location": "src/resolve.js:183",
|
"location": "src/resolve.js:183",
|
||||||
"problem": "`reconcileConversations` 同時負責收 comment、分組、關閉、讀檔、AI 裁決、結果分類與降級處理,職責太多而且彼此耦合。任何一個小規則變動,都得先看完整條流程,單元測試也很難只鎖定某一段行為。",
|
"problem": "`reconcileConversations` 同時負責收 comment、分組、關閉、讀檔、AI 裁決、結果分類與降級處理,職責太多而且彼此耦合。任何一個小規則變動,都得先看完整條流程,單元測試也很難只鎖定某一段行為。",
|
||||||
"suggestion": "拆成幾個可測的純函式與薄編排層,例如 `groupConversations`、`closeOpenComments`、`buildJudgeItems`、`applyVerdicts` 分開處理,讓主流程只保留資料流轉與錯誤收斂。",
|
"suggestion": "拆成幾個可測的純函式與薄編排層,例如 `groupConversations`、`closeOpenComments`、`buildJudgeItems`、`applyVerdicts` 分開處理,讓主流程只保留資料流轉與錯誤收斂。",
|
||||||
"is_new": true
|
"is_new": false
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Leo",
|
|
||||||
"location": "src/git.js:222",
|
|
||||||
"problem": "`commitAndPush` 把 repo 對齊、檔案複製、stage、commit、push、失敗降級全部塞在一起,還保留了一個目前沒用到的 `_sourceRoot` 參數。這種 API 會越長越像腳本,之後要改 staging 規則或推送策略時,維護者很難快速定位應該改哪一段。",
|
|
||||||
"suggestion": "把它拆成 `syncRepo`、`stageReviewFiles`、`createCommit`、`pushCommit` 幾個步驟,再由一個很薄的 orchestrator 串起來;同時移除或真正使用 `_sourceRoot`,避免留下誤導性的簽章。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Leo",
|
|
||||||
"location": "src/main.js:54",
|
|
||||||
"problem": "`main()` 已經變成整條 pipeline 的超級入口,11 個 step、exit 判斷、資料收集、排序/過濾與發布全部擠在同一個函式裡。未來只要某一步的前置條件改了,維護者就得在這個巨型函式裡追完整條狀態流,認知負擔很高。",
|
|
||||||
"suggestion": "把每個 step 拆成獨立函式並回傳明確的 context,讓 `main()` 只負責流程編排與最終 exit 決策;這樣之後新增步驟或調整順序時,不會把整條 pipeline 綁死在同一個函式裡。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Mage",
|
|
||||||
"location": "src/comments.js:40",
|
|
||||||
"problem": "這裡把 `file:0` 也視為有效行號;最小重現:只要上游傳進 `app/foo.js:0`,`parseLocation()` 會回傳 line=0,後續 `postPullReviewComment` 會帶著 `new_position: 0` 發到 Gitea,通常會被拒絕或定位失敗。也就是說,0 行號沒有被當成缺值處理。",
|
|
||||||
"suggestion": "把行號門檻改成 `> 0`,`0` 與負數都應視為無效;同時讓需要行號的呼叫端把這種情況當作缺行號,重新定位或降級處理。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Mage",
|
|
||||||
"location": "src/json.js:88",
|
|
||||||
"problem": "這裡只檢查 `JSON.parse` 能不能過,沒有確認解析結果一定是陣列;最小重現:repairer 回傳 `{}` 或檔案本身就是 `{}` 時,函式仍會回報 valid 並寫回磁碟,但後續程式都把它當陣列讀取,最後會悄悄被當成空資料或造成形狀錯誤。",
|
|
||||||
"suggestion": "在驗證成功前先檢查 `Array.isArray(parsed)`,只有真正的陣列才算通過;修復後也要同樣做陣列檢查,否則就丟錯並保留原檔。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
@@ -197,6 +125,22 @@
|
|||||||
"location": "src/resolve.js:213",
|
"location": "src/resolve.js:213",
|
||||||
"problem": "`findingSig` 只用檔案路徑加上 `suggestion` 來識別問題,忽略了 `problem`、`role`,也沒有留下任何穩定的 thread 識別;最小重現:同一個 `a.js` 內有兩條都建議「加上 null 檢查」但其實是不同位置的 finding,先解掉其中一條後,另一條也會被當成同一筆而被 `dropResolvedFindings` / `addCarriedFindings` 誤合併或誤刪。",
|
"problem": "`findingSig` 只用檔案路徑加上 `suggestion` 來識別問題,忽略了 `problem`、`role`,也沒有留下任何穩定的 thread 識別;最小重現:同一個 `a.js` 內有兩條都建議「加上 null 檢查」但其實是不同位置的 finding,先解掉其中一條後,另一條也會被當成同一筆而被 `dropResolvedFindings` / `addCarriedFindings` 誤合併或誤刪。",
|
||||||
"suggestion": "把識別鍵改成更穩定的組合,例如檔案路徑 + 正規化後的 `problem` + `suggestion` + `role`,或直接使用可追蹤的 thread/issue id;不要只靠 `suggestion` 斷言是不是同一個問題。",
|
"suggestion": "把識別鍵改成更穩定的組合,例如檔案路徑 + 正規化後的 `problem` + `suggestion` + `role`,或直接使用可追蹤的 thread/issue id;不要只靠 `suggestion` 斷言是不是同一個問題。",
|
||||||
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Mage",
|
||||||
|
"location": "src/comments.js:40",
|
||||||
|
"problem": "這裡把 `file:0` 也視為有效行號;最小重現:只要上游傳進 `app/foo.js:0`,`parseLocation()` 會回傳 line=0,後續 `postPullReviewComment` 會帶著 `new_position: 0` 發到 Gitea,通常會被拒絕或定位失敗。也就是說,0 行號沒有被當成缺值處理。",
|
||||||
|
"suggestion": "把行號門檻改成 `> 0`,`0` 與負數都應視為無效;同時讓需要行號的呼叫端把這種情況當作缺行號,重新定位或降級處理。",
|
||||||
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Maya",
|
||||||
|
"location": "src/comments.js:233",
|
||||||
|
"problem": "`postFindingsReview` 的降級流程有兩層:先嘗試批次 review,再失敗時改成 summary-only,最後 summary-only 也失敗才退回一般 comment。現在的測試只驗到第一層失敗後、第二層成功的情境,沒有驗證 summary-only 也失敗時是否真的會呼叫 `postIssue(body)`,這是最脆弱的 fallback 路徑之一。",
|
||||||
|
"suggestion": "新增一個測試讓第一次 `postReview({comments})` 失敗、第二次 `postReview({comments: []})` 也失敗,然後斷言 `postIssue(body)` 有被呼叫,且 inline comments 仍會逐筆嘗試送出。",
|
||||||
"is_new": true
|
"is_new": true
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
@@ -205,71 +149,71 @@
|
|||||||
"location": "src/config.js:31",
|
"location": "src/config.js:31",
|
||||||
"problem": "這裡是整個 action 讀取 `INPUT_*`、`GITEA_*` 與事件 payload 的入口,但測試只覆蓋了 `getLLMConfig()`,沒有把 `GITEA_TOKEN`、`GITEA_COMMENT_TOKEN`、`PR_NUMBER`、`PR_HEAD_SHA` 這些環境與 payload 的優先序鎖住。特別是 comment token 退回主 token、以及 event 檔讀不到時回到空值的情境,都是 CI 最容易因環境差異壞掉的地方。",
|
"problem": "這裡是整個 action 讀取 `INPUT_*`、`GITEA_*` 與事件 payload 的入口,但測試只覆蓋了 `getLLMConfig()`,沒有把 `GITEA_TOKEN`、`GITEA_COMMENT_TOKEN`、`PR_NUMBER`、`PR_HEAD_SHA` 這些環境與 payload 的優先序鎖住。特別是 comment token 退回主 token、以及 event 檔讀不到時回到空值的情境,都是 CI 最容易因環境差異壞掉的地方。",
|
||||||
"suggestion": "新增 config 相關測試,分別用假 `process.env` 和暫存 event payload 檔驗證:`INPUT_*` 會蓋過 ambient env、`GITEA_COMMENT_TOKEN` 缺值時會 fallback 到主 token、`GITEA_EVENT_PATH` / `GITHUB_EVENT_PATH` 讀取失敗時不會拋錯且回傳預設值。",
|
"suggestion": "新增 config 相關測試,分別用假 `process.env` 和暫存 event payload 檔驗證:`INPUT_*` 會蓋過 ambient env、`GITEA_COMMENT_TOKEN` 缺值時會 fallback 到主 token、`GITEA_EVENT_PATH` / `GITHUB_EVENT_PATH` 讀取失敗時不會拋錯且回傳預設值。",
|
||||||
"is_new": true
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
"role": "Maya",
|
|
||||||
"location": "src/comments.js:211",
|
|
||||||
"problem": "`postFindingsReview()` 的主快樂路徑有測到,但它在批次 review 失敗後還有第二層降級邏輯:先重送 summary-only review,若 summary 也失敗才改走一般 comment,再逐筆補行內 comment。這條失敗鏈現在沒有被驗證,等於最重要的容錯行為只被程式碼描述,沒有被試煉。",
|
|
||||||
"suggestion": "補測 `postReview({ body, comments })` 先丟錯、再讓 `postReview({ body, comments: [] })` 也丟錯的情境,斷言最後會呼叫 `postIssue(body)`,且原本的 comments 仍會逐筆走 `postInline`。如果要更完整,也順便補 `postOldFindingsComment()` 與 `postNewNonCriticalComment()` 的篩選與空集合跳過案例。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Maya",
|
|
||||||
"location": "src/llm.js:66",
|
|
||||||
"problem": "`runAssistantCLI()` 目前只有成功與一般失敗的測試,沒有覆蓋 timeout、`maxBuffer` 超限、以及 `opencode` 分支建立的暫存 prompt 檔在例外發生時是否確實清理。這些都是外部 CLI 整合最常出問題的失敗路徑,沒有測到就很難確定不會留下殘檔或把流程卡死。",
|
|
||||||
"suggestion": "補 fake CLI 測試,讓子程序超時、輸出超過 `AI_ASSISTANT_MAX_BUFFER`、以及 `opencode` 在 `spawn`/`close` 前後失敗,分別斷言會回傳對應錯誤,且暫存目錄與 `prompt.md` 會被清掉。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Rogue",
|
|
||||||
"location": "src/main.js:98",
|
|
||||||
"problem": "這裡把每個角色的 LLM 分析用 `for...of + await` 串成一條龍,角色數一多就把總等待時間從「最慢那個角色」拉成「全部角色耗時相加」。每多一個角色,就白白多吃一輪模型呼叫延遲,熱路徑會被拖得很明顯。",
|
|
||||||
"suggestion": "改成平行發出各角色分析,例如先 `Promise.all` 收集結果,再依原順序合併與排序;如果擔心單一失敗中斷,搭配 `Promise.allSettled` 保留容錯。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Rogue",
|
|
||||||
"location": "src/preflight.js:152",
|
|
||||||
"problem": "前置驗證的 Gitea token、comment token、git remote、LLM 驗證彼此沒有相依,卻被拆成連續等待。每個步驟都可能卡網路與 30 秒級 timeout,最差會把啟動時間疊成多倍,白白浪費整段等待。",
|
|
||||||
"suggestion": "把互不相依的檢查改成並行執行,至少讓 token / remote / LLM 這幾項同時跑,只保留必要的 env 檢查先行。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Rogue",
|
|
||||||
"location": "src/findings.js:413",
|
|
||||||
"problem": "這裡對每一筆 finding 都用 `exclusions.some(...)` 線性掃完整份排除清單,還在內層反覆做文字正規化,複雜度直接變成 O(findings × exclusions)。排除規則一多,這段會把 CPU 週期浪費在重複比對上。",
|
|
||||||
"suggestion": "先把 exclusions 依 `filePath`、`role` 或正規化後的 `textKey` 建索引/分桶,再做比對;這樣可以把熱路徑從雙層掃描降到接近線性。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "info",
|
|
||||||
"role": "Bard",
|
"role": "Bard",
|
||||||
"location": "src/git.js:226",
|
"location": "src/config.js:27",
|
||||||
"problem": "_sourceRoot` 的參數文件寫著「不確定,待確認」,讓公開函式簽名帶著未完成的旁白,破壞 API 文件的一致與可信度。",
|
"problem": "這段註解已經跟著介面走音了。它宣稱使用端「只需傳 `with: token`」,但這次 action 其實已新增 `comment_token` 與 `model` 等輸入,註解仍停留在舊旋律,容易讓讀者誤判介面現況。",
|
||||||
"suggestion": "若參數已不使用,移除它;若為相容性保留,明確寫成 deprecated/compatibility note,不要留下模糊語句。",
|
"suggestion": "把這組說明改成與目前 inputs 一致,明確列出 `token`、`comment_token`、`model` 的優先序與用途;如果無法精簡,就直接移到 README 或設計文件,避免在程式中留下過時註記。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Mage",
|
||||||
|
"location": "src/findings.js:227",
|
||||||
|
"problem": "這個抽取器一旦命中目標檔案,就一路把後面的 diff 全部帶進去,沒有在下一個 `diff --git` 區塊時停下來。最小重現:diff 同時有 `a.js` 和 `b.js`,要補 `a.js` 的行號時,送給 LLM 的內容會混進 `b.js` 的 hunks,結果很容易定位到錯的行,或讓模型把別檔的內容誤認成目標檔上下文。",
|
||||||
|
"suggestion": "在開始捕捉後,遇到下一個 `diff --git ` 就應該停止,只回傳目前檔案那一段;找不到目標檔時再退回整份 diff。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Maya",
|
||||||
|
"location": "src/findings.js:277",
|
||||||
|
"problem": "`deduplicateWithAI` 是新的核心語意去重流程,但目前完全沒有直接測試它的成功與失敗分支。尤其是 LLM 回傳排序不同、夾雜幻覺項目、回傳空陣列或超量結果時,程式會改走保守 fallback,這些都是很容易壞掉但現在沒被驗證的邊界。",
|
||||||
|
"suggestion": "替 `deduplicateWithAI` 補測兩類情境:一是 stub `chatJSON` 回傳重排後的重複項與一筆幻覺項,確認只保留能對應回原始 findings 的資料;二是回傳空陣列或不合法結果時,確認會保守回傳原始 findings。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Rogue",
|
||||||
|
"location": "src/findings.js:293",
|
||||||
|
"problem": "每筆缺行號的 finding 都是單獨處理,還在每筆裡串行重試最多 3 次 LLM 查詢。N 筆問題就會膨脹成 N×3 次遠端呼叫與等待,幾十筆時延遲會直接被放大好幾倍。",
|
||||||
|
"suggestion": "把行號定位改成可平行的批次流程,例如同檔或同角色一起送出後用 `Promise.allSettled` 收結果;重試只保留在單筆失敗時,別讓每個 finding 都自己拖慢整條管線。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Rogue",
|
||||||
|
"location": "src/findings.js:442",
|
||||||
|
"problem": "這裡每一筆 finding 都要跟整包 exclusions 做一次 `.some()`,而且內層還反覆跑 `normalizeText` 和字串包含比對,資料一多就直接變成 O(F×E) 的熱點。像 300 筆 finding 配 500 筆 exclusions,會吃掉 15 萬次以上的比對與正規化,CPU 和字串配置都在浪費。",
|
||||||
|
"suggestion": "先把 exclusions 在載入時一次正規化並依 `filePath / role / textKey` 建索引,讓過濾改成近似 O(F);至少把 `normalizeText` 移到內層迴圈外,避免同一段字串被重算成百上千次。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Rogue",
|
||||||
|
"location": "src/gitea.js:239",
|
||||||
|
"problem": "這裡逐一 await 每個 review 的 comments,PR review 一多就變成 N 次遠端呼叫的線性延遲累加;例如 30 個 review 就是 30 個 round-trip 排隊等,時間都被網路空轉偷走。",
|
||||||
|
"suggestion": "把 reviews.map(review => getPullReviewComments(review.id).catch(...)) 丟進 Promise.all 或 Promise.allSettled 平行抓取,再 flat 結果;單筆失敗仍可記 warn 後略過。",
|
||||||
"is_new": false
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "info",
|
"level": "warning",
|
||||||
"role": "Leo",
|
"role": "Mage",
|
||||||
"location": "src/json.js:13",
|
"location": "src/comments.js:40",
|
||||||
"problem": "`stripCodeFence()` 與 `src/llm.js` 內的 `stripOuterFence()` 幾乎是同一個功能,未來如果要支援更多 fence 格式或修 bug,兩邊需要同步修改,容易產生行為漂移。",
|
"problem": "這裡把 `file:0` 也視為有效行號;最小重現:只要上游傳進 `app/foo.js:0`,`parseLocation()` 會回傳 line=0,後續 `postPullReviewComment` 會帶著 `new_position: 0` 發到 Gitea,通常會被拒絕或定位失敗。也就是說,0 行號沒有被當成缺值處理。",
|
||||||
"suggestion": "抽成共用的 JSON/text utility,例如 `src/text.js` 或 `src/json.js` 匯出單一 fence 清理函式,讓 LLM JSON 解析與 JSON repair 共用同一套邏輯。",
|
"suggestion": "把行號門檻改成 `> 0`,`0` 與負數都應視為無效;同時讓需要行號的呼叫端把這種情況當作缺行號,重新定位或降級處理。",
|
||||||
"is_new": false
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "info",
|
"level": "warning",
|
||||||
"role": "Leo",
|
"role": "Mage",
|
||||||
"location": "src/findings.js:104",
|
"location": "src/main.js:69",
|
||||||
"problem": "文字正規化邏輯分散在 `normalizeText()`、`toKeyText()`,而 `src/resolve.js` 也有另一套 `normalizeKey()`。這些函式對大小寫、標點與空白的處理不完全一致,長期會讓 finding 去重、排除與對話收斂出現難追的差異。",
|
"problem": "這裡先檢查 head SHA 對應的訊息是否為 failure,但如果 SHA 查詢失敗或是空值,後面的 `shouldSkipBotCommit()` 仍可能只看到分支 head 上的 `[ai-review-bot]` 標記就直接跳過。最小重現:`getCommitMessageBySha()` 因 Gitea API 暫時失敗回空字串,而分支 head 正好是 `[ai-review-bot][failure]`,流程就會 exit 0,等於把本來應該失敗的 bot commit 當成可跳過的自動提交。",
|
||||||
"suggestion": "建立單一 normalization 模組,明確定義 `normalizeForDisplayMatch`、`normalizeForSignature` 等用途,再讓 findings、resolve、exclusions 共用,並補上跨模組測試鎖定語意。",
|
"suggestion": "把「是否跳過」和「是否 failure」分開判斷,或讓 helper 回傳解析出的 outcome;只允許 success 標記走 skip,failure 標記不論 SHA/branch 來源都應優先讓流程失敗。",
|
||||||
"is_new": false
|
"is_new": true
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "info",
|
"level": "info",
|
||||||
@@ -282,9 +226,33 @@
|
|||||||
{
|
{
|
||||||
"level": "info",
|
"level": "info",
|
||||||
"role": "Rogue",
|
"role": "Rogue",
|
||||||
"location": "src/findings.js:382",
|
"location": "src/comments.js:102",
|
||||||
"problem": "loadExclusions 前面已經 normalizeExclusionEntry + dedupeExclusions,這裡又呼叫 buildExclusionContext(exclusions) 重新 normalize、dedupe、group 一輪,只為了 log groups 數;排除規則多時會多跑一趟 O(e log e) 的整理成本。",
|
"problem": "統計表與單行摘要各欄位都用 `filter(...).length` 重掃多次,同一批 findings 會被走 4 到 8 次。資料量一大,連 log 文字本身都開始吃不必要的掃描成本。",
|
||||||
"suggestion": "讓 buildExclusionContext 可接受已正規化/已去重的 exclusions,或直接在 loadExclusions 重用現有 exclusions 進行 group 統計,避免重複正規化與排序。",
|
"suggestion": "改成單次迴圈同時累加 critical / warning / info / 未分類計數,再把結果組成表格和摘要;一次走完就好,別讓統計自己變熱點。",
|
||||||
"is_new": false
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "info",
|
||||||
|
"role": "Maya",
|
||||||
|
"location": "src/comments.js:266",
|
||||||
|
"problem": "`postOldFindingsComment` 與緊接著的 `postNewNonCriticalComment` 都是這次新加的對外 comment 發布行為,但目前沒有專門測試它們的空陣列早退、標題文字與表格內容。這會讓 comment 分流邏輯只靠間接測試支撐,回歸時很容易漏掉。",
|
||||||
|
"suggestion": "補這兩個函式的單元測試:至少驗證空陣列時不會送 comment、非空時 body 內容包含正確標題與表格,且 `postOldFindingsComment` 只收舊問題、`postNewNonCriticalComment` 只收新非 critical 問題。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "info",
|
||||||
|
"role": "Rogue",
|
||||||
|
"location": "src/findings.js:393",
|
||||||
|
"problem": "前面已經把 exclusions 正規化、去重過一次了,這裡為了 log 又再丟進 `buildExclusionContext` 重做 normalize / dedupe / group。等於同一批資料在同一輪流程裡被重算兩次,白白多吃一輪 O(n) 到 O(n log n) 的 CPU。",
|
||||||
|
"suggestion": "把第一次處理的摘要一起回傳或快取下來,後面的 log 直接重用同一份結果,不要再對同一批 exclusions 重跑分組。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "info",
|
||||||
|
"role": "Rogue",
|
||||||
|
"location": "src/resolve.js:88",
|
||||||
|
"problem": "這個 `codeWindow` 每遇到一筆 open conversation 就對整份檔案內容再 `split('\\n')` 一次。若同一個檔案有多條 thread,O(L) 的切割和陣列配置會被重複吃掉,明明同一份內容卻一直重複解剖。",
|
||||||
|
"suggestion": "先把檔案內容預先切成行陣列並快取,或讓 `codeWindow` 直接吃已分割好的 lines;這樣同檔多條對話就不用重複掃描整份內容。",
|
||||||
|
"is_new": true
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
|||||||
Reference in New Issue
Block a user