Compare commits
7
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0be25e667c | ||
|
|
93142e2250 | ||
|
|
c521451b66 | ||
|
|
94d86809d5 | ||
|
|
1279cae575 | ||
|
|
47aa199e5e | ||
|
|
4b4b934cda |
+225
-113
@@ -5,87 +5,55 @@
|
|||||||
"location": "src/resolve.js:253",
|
"location": "src/resolve.js:253",
|
||||||
"problem": "這裡會把 PR 上所有未解決的 review comment ID 全部送去 resolve,而不是只處理 AI Review bot 自己建立的 thread。攻擊者只要開 PR 觸發這個 action,就可能讓 bot 關閉人類審查者留下的安全疑慮或阻擋性對話,繞過人工審查流程。",
|
"problem": "這裡會把 PR 上所有未解決的 review comment ID 全部送去 resolve,而不是只處理 AI Review bot 自己建立的 thread。攻擊者只要開 PR 觸發這個 action,就可能讓 bot 關閉人類審查者留下的安全疑慮或阻擋性對話,繞過人工審查流程。",
|
||||||
"suggestion": "只 resolve 可證明由本 bot 建立且格式符合預期的 comment,例如檢查作者、固定 marker、review body 簽章或 botFinding 解析結果;人類留言與未知格式留言不得自動關閉。",
|
"suggestion": "只 resolve 可證明由本 bot 建立且格式符合預期的 comment,例如檢查作者、固定 marker、review body 簽章或 botFinding 解析結果;人類留言與未知格式留言不得自動關閉。",
|
||||||
"is_new": true
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "critical",
|
"level": "critical",
|
||||||
"role": "Mage",
|
"role": "Maya",
|
||||||
"location": "src/findings.js:471",
|
"location": "src/main.js:44",
|
||||||
"problem": "當排除條目有 location 或 role 時,這裡直接把文字比對結果短路成 true。最小重現:exclusions.json 只有 `{ \"location\": \"app/a.js:10\", \"original_finding\": \"誤報 A\" }`,新的 finding 是 `app/a.js:99` 且 suggestion 完全不同,仍會因同檔案而被排除,導致真問題被靜默丟掉。",
|
"problem": "這個 `main()` 是整個 action 的流程總管,但目前沒有任何整合測試或端到端測試去驗證 Step3~Step11 的分支切換與 `process.exit()` 行為。像是前置驗證失敗、偵測到 bot 自動提交、diff 為空、所有角色分析都失敗、JSON 驗證失敗、出現 critical finding、以及 commit/push 降級路徑,現在都只靠人工推演,實際接線後一旦流程順序或退出條件出錯,現有單元測試抓不到。",
|
||||||
"suggestion": "不要用 `exPath || ex.role ? true : textMatches` 跳過文字比對;應至少要求位置精確匹配到同一行,或在同檔/同角色時仍必須通過 `textMatches`,例如 `return locationMatches && roleMatches && textMatches`,並明確定義 suggestion 空白時才是萬用規則。",
|
"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": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"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": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "critical",
|
||||||
|
"role": "Assassin",
|
||||||
|
"location": "src/main.js:158",
|
||||||
|
"problem": "這裡直接載入 PR 工作樹中的 `.gitea/ai-review/exclusions.json` 當成既有排除規則。攻擊者可以先在 PR 內預埋一份排除清單,因為 `.gitea/` 又被預設排除於 diff 之外,這些惡意排除不會被審查到,卻會被流程直接拿來吞掉真正的 findings,形成靜默的審查繞過。",
|
||||||
|
"suggestion": "不要從 PR head 讀取可由提交者任意修改的 exclusions;只接受由受信任 bot、受保護分支或外部持久化儲存產生的排除資料,並驗證來源身分與 commit marker,避免使用者自行預埋排除規則。",
|
||||||
"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": true
|
"is_new": false
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Leo",
|
|
||||||
"location": "src/main.js:36",
|
|
||||||
"problem": "`main()` 把前置驗證、bot commit 判斷、對話收斂、角色分析、合併去重、排除、發布、JSON 驗證、commit/push 與 gate 全部塞在同一個 190 行左右的函式裡,且中間散落多個 `process.exit()`。六個月後要改其中任一步驟時,很難隔離副作用,也不容易針對單一階段寫單元測試。",
|
|
||||||
"suggestion": "將每個 Step 拆成可注入相依、回傳明確結果的函式,例如 `runAnalysisStep()`、`runFilteringStep()`、`runPublishStep()`;最外層再統一把結果轉成 exit code,讓流程控制與業務邏輯分離。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Leo",
|
|
||||||
"location": "src/findings.js:381",
|
|
||||||
"problem": "`loadExclusions()` 同時負責讀檔、解析多種格式、正規化、去重、記錄 repo 狀態、改寫原檔、鏡像寫入與建立 AI prompt 摘要。這個函式的職責過多,之後只要調整 exclusions 格式或同步策略,就很容易牽動不相關行為。",
|
|
||||||
"suggestion": "拆成 `readExclusionsFile()`、`normalizeExclusionsData()`、`canonicalizeExclusionsFile()`、`logExclusionMetadata()` 等小函式,讓讀取、轉換、寫回與診斷各自可測。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
"role": "Mage",
|
"role": "Mage",
|
||||||
"location": "src/findings.js:466",
|
"location": "src/main.js:69",
|
||||||
"problem": "這裡優先使用 `ex.textKey`,但 `textKey` 是由 `toKeyText()` 產生的無分隔且未轉小寫文字,而 findingText 是 `normalizeText()` 產生的小寫、以空白分隔文字。最小重現:排除文字 `Update tests` 會變成 `Updatetests`,finding suggestion 會變成 `update tests`,兩邊互相 `includes` 都不成立,導致純文字排除規則失效。",
|
"problem": "這裡先檢查 head SHA 對應的訊息是否為 failure,但如果 SHA 查詢失敗或是空值,後面的 `shouldSkipBotCommit()` 仍可能只看到分支 head 上的 `[ai-review-bot]` 標記就直接跳過。最小重現:`getCommitMessageBySha()` 因 Gitea API 暫時失敗回空字串,而分支 head 正好是 `[ai-review-bot][failure]`,流程就會 exit 0,等於把本來應該失敗的 bot commit 當成可跳過的自動提交。",
|
||||||
"suggestion": "排除條目與 finding 應使用同一套正規化函式比對;例如改存並使用 `normalizeText(ex.text || ex.suggestion || ex.title || '')`,或讓 finding 也轉成同樣的 compact/lowercase key。",
|
"suggestion": "把「是否跳過」和「是否 failure」分開判斷,或讓 helper 回傳解析出的 outcome;只允許 success 標記走 skip,failure 標記不論 SHA/branch 來源都應優先讓流程失敗。",
|
||||||
"is_new": true
|
"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": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"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": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Maya",
|
|
||||||
"location": "src/findings.js:416",
|
|
||||||
"problem": "applyExclusions 的核心比對支援「只有文字、沒有路徑/角色」的排除規則,但現有測試多半靠相同檔案路徑命中,沒有驗證純文字排除、空文字排除、大小寫/標點差異等邊界。這條排除規則的最脆弱分支還沒被測到。",
|
|
||||||
"suggestion": "補上 applyExclusions 的邊界測試:只有 suggestion/title 文字沒有 location 的 exclusion 應如何比對;空文字 exclusion 不應意外排除全部;標點、空白、大小寫正規化後相同的文字應依預期排除。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Maya",
|
|
||||||
"location": "src/gitea.js:95",
|
|
||||||
"problem": "shouldSkipBotCommit 目前只看到命中 bot marker 的測試,缺少「commit API 失敗、分支查詢失敗、sha/branch 都沒有 marker」時應回 false 的失敗與保守路徑驗證。這是避免 workflow 誤跳過審查的關鍵判斷,不能只測快樂路徑。",
|
|
||||||
"suggestion": "新增測試讓 getCommitMessageBySha / getBranchHeadCommitMessage 對應的 axios 呼叫拋錯或回一般 commit message,斷言 shouldSkipBotCommit 回 false,且不會把查詢失敗誤判成 bot commit。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Maya",
|
|
||||||
"location": "src/llm.js:91",
|
|
||||||
"problem": "runAssistantCLI 有 timeout 與 maxBuffer 兩條重要失敗路徑,但目前測試只覆蓋 CLI 非零退出,沒有驗證逾時會 kill 子程序並拒絕、輸出超過限制會中止且不產生未處理的重複 reject。這些是 CI 上最常見的失敗情境。",
|
|
||||||
"suggestion": "新增 llm 測試:用假的 CLI sleep 超過 AI_ASSISTANT_TIMEOUT_MS,斷言錯誤訊息包含逾時;再用大量 stdout/stderr 超過 AI_ASSISTANT_MAX_BUFFER,斷言錯誤訊息正確且測試過程沒有 unhandled rejection。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
@@ -93,7 +61,47 @@
|
|||||||
"location": "src/main.js:132",
|
"location": "src/main.js:132",
|
||||||
"problem": "這裡把每個角色的 LLM 分析逐一 await,6 個角色就把總耗時堆成約 6 倍單次模型延遲;這些分析彼此獨立,CPU 沒偷到時間,反而把整條 pipeline 卡在序列網路/CLI 呼叫上。",
|
"problem": "這裡把每個角色的 LLM 分析逐一 await,6 個角色就把總耗時堆成約 6 倍單次模型延遲;這些分析彼此獨立,CPU 沒偷到時間,反而把整條 pipeline 卡在序列網路/CLI 呼叫上。",
|
||||||
"suggestion": "改用 Promise.allSettled 平行執行 roles.map(role => analyzeWithRole(role, diff)),再彙整 fulfilled 結果與 warning;保留 fulfilledAnalyses 的判斷即可。",
|
"suggestion": "改用 Promise.allSettled 平行執行 roles.map(role => analyzeWithRole(role, diff)),再彙整 fulfilled 結果與 warning;保留 fulfilledAnalyses 的判斷即可。",
|
||||||
"is_new": true
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Leo",
|
||||||
|
"location": "src/findings.js:381",
|
||||||
|
"problem": "`loadExclusions()` 同時負責讀檔、解析多種格式、正規化、去重、記錄 repo 狀態、改寫原檔、鏡像寫入與建立 AI prompt 摘要。這個函式的職責過多,之後只要調整 exclusions 格式或同步策略,就很容易牽動不相關行為。",
|
||||||
|
"suggestion": "拆成 `readExclusionsFile()`、`normalizeExclusionsData()`、`canonicalizeExclusionsFile()`、`logExclusionMetadata()` 等小函式,讓讀取、轉換、寫回與診斷各自可測。",
|
||||||
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Mage",
|
||||||
|
"location": "src/findings.js:466",
|
||||||
|
"problem": "這裡優先使用 `ex.textKey`,但 `textKey` 是由 `toKeyText()` 產生的無分隔且未轉小寫文字,而 findingText 是 `normalizeText()` 產生的小寫、以空白分隔文字。最小重現:排除文字 `Update tests` 會變成 `Updatetests`,finding suggestion 會變成 `update tests`,兩邊互相 `includes` 都不成立,導致純文字排除規則失效。",
|
||||||
|
"suggestion": "排除條目與 finding 應使用同一套正規化函式比對;例如改存並使用 `normalizeText(ex.text || ex.suggestion || ex.title || '')`,或讓 finding 也轉成同樣的 compact/lowercase key。",
|
||||||
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Maya",
|
||||||
|
"location": "src/findings.js:416",
|
||||||
|
"problem": "applyExclusions 的核心比對支援「只有文字、沒有路徑/角色」的排除規則,但現有測試多半靠相同檔案路徑命中,沒有驗證純文字排除、空文字排除、大小寫/標點差異等邊界。這條排除規則的最脆弱分支還沒被測到。",
|
||||||
|
"suggestion": "補上 applyExclusions 的邊界測試:只有 suggestion/title 文字沒有 location 的 exclusion 應如何比對;空文字 exclusion 不應意外排除全部;標點、空白、大小寫正規化後相同的文字應依預期排除。",
|
||||||
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Maya",
|
||||||
|
"location": "src/gitea.js:95",
|
||||||
|
"problem": "shouldSkipBotCommit 目前只看到命中 bot marker 的測試,缺少「commit API 失敗、分支查詢失敗、sha/branch 都沒有 marker」時應回 false 的失敗與保守路徑驗證。這是避免 workflow 誤跳過審查的關鍵判斷,不能只測快樂路徑。",
|
||||||
|
"suggestion": "新增測試讓 getCommitMessageBySha / getBranchHeadCommitMessage 對應的 axios 呼叫拋錯或回一般 commit message,斷言 shouldSkipBotCommit 回 false,且不會把查詢失敗誤判成 bot commit。",
|
||||||
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"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": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
@@ -101,62 +109,134 @@
|
|||||||
"location": "src/gitea.js:239",
|
"location": "src/gitea.js:239",
|
||||||
"problem": "這裡逐一 await 每個 review 的 comments,PR review 一多就變成 N 次遠端呼叫的線性延遲累加;例如 30 個 review 就是 30 個 round-trip 排隊等,時間都被網路空轉偷走。",
|
"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 後略過。",
|
"suggestion": "把 reviews.map(review => getPullReviewComments(review.id).catch(...)) 丟進 Promise.all 或 Promise.allSettled 平行抓取,再 flat 結果;單筆失敗仍可記 warn 後略過。",
|
||||||
"is_new": true
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"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": true
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "info",
|
"level": "warning",
|
||||||
|
"role": "Mage",
|
||||||
|
"location": "src/resolve.js:213",
|
||||||
|
"problem": "`findingSig` 只用檔案路徑加上 `suggestion` 來識別問題,忽略了 `problem`、`role`,也沒有留下任何穩定的 thread 識別;最小重現:同一個 `a.js` 內有兩條都建議「加上 null 檢查」但其實是不同位置的 finding,先解掉其中一條後,另一條也會被當成同一筆而被 `dropResolvedFindings` / `addCarriedFindings` 誤合併或誤刪。",
|
||||||
|
"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": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Maya",
|
||||||
|
"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 最容易因環境差異壞掉的地方。",
|
||||||
|
"suggestion": "新增 config 相關測試,分別用假 `process.env` 和暫存 event payload 檔驗證:`INPUT_*` 會蓋過 ambient env、`GITEA_COMMENT_TOKEN` 缺值時會 fallback 到主 token、`GITEA_EVENT_PATH` / `GITHUB_EVENT_PATH` 讀取失敗時不會拋錯且回傳預設值。",
|
||||||
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
"role": "Bard",
|
"role": "Bard",
|
||||||
"location": "src/findings.js:138",
|
"location": "src/config.js:27",
|
||||||
"problem": "註解中留下「不確定」這種未定案語氣,像樂譜上的猶豫記號;讀者無法判斷這是刻意設計、待辦事項,還是審查遺留。",
|
"problem": "這段註解已經跟著介面走音了。它宣稱使用端「只需傳 `with: token`」,但這次 action 其實已新增 `comment_token` 與 `model` 等輸入,註解仍停留在舊旋律,容易讓讀者誤判介面現況。",
|
||||||
"suggestion": "若是刻意差異,改寫成明確理由;若待確認,改成可追蹤的 TODO 並標明決策者或議題。",
|
"suggestion": "把這組說明改成與目前 inputs 一致,明確列出 `token`、`comment_token`、`model` 的優先序與用途;如果無法精簡,就直接移到 README 或設計文件,避免在程式中留下過時註記。",
|
||||||
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"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": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Maya",
|
||||||
|
"location": "src/findings.js:277",
|
||||||
|
"problem": "`deduplicateWithAI` 是新的核心語意去重流程,但目前完全沒有直接測試它的成功與失敗分支。尤其是 LLM 回傳排序不同、夾雜幻覺項目、回傳空陣列或超量結果時,程式會改走保守 fallback,這些都是很容易壞掉但現在沒被驗證的邊界。",
|
||||||
|
"suggestion": "替 `deduplicateWithAI` 補測兩類情境:一是 stub `chatJSON` 回傳重排後的重複項與一筆幻覺項,確認只保留能對應回原始 findings 的資料;二是回傳空陣列或不合法結果時,確認會保守回傳原始 findings。",
|
||||||
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Rogue",
|
||||||
|
"location": "src/findings.js:322",
|
||||||
|
"problem": "每一筆缺行號的 finding 都重新呼叫 `extractFileDiff(diff, file)` 掃完整份 diff,若同一檔案有 k 筆問題,就會重複做 k 次整份 diff 解析,浪費量是 O(k × diff長度)。",
|
||||||
|
"suggestion": "先把 diff 依檔名切成快取 Map,一次掃描建立好 `file -> fileDiff`,後續同檔 finding 直接共用已切好的片段。",
|
||||||
"is_new": true
|
"is_new": true
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "info",
|
"level": "warning",
|
||||||
"role": "Bard",
|
"role": "Bard",
|
||||||
"location": "src/git.js:226",
|
"location": "src/llm.js:56",
|
||||||
"problem": "_sourceRoot` 的參數文件寫著「不確定,待確認」,讓公開函式簽名帶著未完成的旁白,破壞 API 文件的一致與可信度。",
|
"problem": "`cliArgs` 把不同提供者的參數拼湊在同一個分支裡,還讓 `opencode` 走了另一套文字輸入路線,整個 helper 的節奏忽然一分為二。讀起來像兩個介面硬塞進同一支笛子。",
|
||||||
"suggestion": "若參數已不使用,移除它;若為相容性保留,明確寫成 deprecated/compatibility note,不要留下模糊語句。",
|
"suggestion": "拆成各提供者各自的 argv builder,或至少把 prompt 輸入方式抽成獨立 helper,讓每個分支只處理一種責任,結構會更俐落。",
|
||||||
"is_new": true
|
"is_new": true
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "info",
|
"level": "warning",
|
||||||
"role": "Bard",
|
|
||||||
"location": "src/config.js:4",
|
|
||||||
"problem": "註解說「需要內部服務相容時才使用 getInsecureHttpsAgent()」,下一行卻在模組載入時全域設定 TLS 環境變數,文件與程式碼唱了不同旋律。",
|
|
||||||
"suggestion": "讓註解忠實描述目前行為,或把全域設定移到明確命名的初始化函式;至少避免文件暗示這是選擇性使用。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "info",
|
|
||||||
"role": "Leo",
|
"role": "Leo",
|
||||||
"location": "src/json.js:13",
|
"location": "src/findings.js:1",
|
||||||
"problem": "`stripCodeFence()` 與 `src/llm.js` 內的 `stripOuterFence()` 幾乎是同一個功能,未來如果要支援更多 fence 格式或修 bug,兩邊需要同步修改,容易產生行為漂移。",
|
"problem": "這個模組同時處理舊 findings 載入、合併去重、缺行號補齊、排除規則正規化、誤報過濾、AI 去重、以及 exclusions 的讀寫,職責已經混成一包。更麻煩的是 `loadExclusions`、`appendExclusions`、`applyExclusions` 各自都有一套相近但不完全一致的比對邏輯,未來只要規則改一處,另一處沒同步就會開始出現不可預期的行為差異。",
|
||||||
"suggestion": "抽成共用的 JSON/text utility,例如 `src/text.js` 或 `src/json.js` 匯出單一 fence 清理函式,讓 LLM JSON 解析與 JSON repair 共用同一套邏輯。",
|
"suggestion": "把 exclusions 的正規化與比對規則抽成唯一來源,例如 `normalizeExclusionEntry` + `matchesExclusion` 之類的共用 helper,並把 AI 去重、行號補齊、檔案持久化拆到不同模組,減少這個檔案的責任面。",
|
||||||
"is_new": true
|
"is_new": true
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "info",
|
"level": "warning",
|
||||||
"role": "Leo",
|
"role": "Mage",
|
||||||
"location": "src/findings.js:104",
|
"location": "src/json.js:113",
|
||||||
"problem": "文字正規化邏輯分散在 `normalizeText()`、`toKeyText()`,而 `src/resolve.js` 也有另一套 `normalizeKey()`。這些函式對大小寫、標點與空白的處理不完全一致,長期會讓 finding 去重、排除與對話收斂出現難追的差異。",
|
"problem": "這裡只檢查 `JSON.parse(normalized)` 能不能成功,沒有確認修復後的內容真的是陣列。最小重現是 AI 把 `findings.json` 修成 `{ \"a\": 1 }`,函式會照樣寫回檔案並回報成功,但下一輪讀取時 `readJSONArray` 會把它當成非陣列而視為空值,等於把資料靜默吃掉。",
|
||||||
"suggestion": "建立單一 normalization 模組,明確定義 `normalizeForDisplayMatch`、`normalizeForSignature` 等用途,再讓 findings、resolve、exclusions 共用,並補上跨模組測試鎖定語意。",
|
"suggestion": "在寫檔前先 `const parsed = JSON.parse(normalized)`,再加上 `Array.isArray(parsed)` 檢查;不是陣列就直接丟錯,不要覆寫原檔。",
|
||||||
"is_new": true
|
"is_new": true
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "info",
|
"level": "warning",
|
||||||
"role": "Leo",
|
"role": "Mage",
|
||||||
"location": "src/roles.js:7",
|
"location": "src/resolve.js:74",
|
||||||
"problem": "`ROLES_DIR` 用 `fileURLToPath(import.meta.url)` 直接接 `..` 來推目錄,雖然目前可運作,但語意上把檔案路徑當目錄路徑處理,未來搬檔或重構時不直覺。",
|
"problem": "這裡用 `path + line` 當唯一群組鍵,且只保留第一筆 `botFinding`。最小重現是同一個檔案同一行同時被兩個角色指出不同問題,`groupConversations` 會把它們合成同一組,後來的那筆 finding 會被吞掉,導致後續關閉、回寫或保留時少掉一個問題。",
|
||||||
"suggestion": "先用 `path.dirname(fileURLToPath(import.meta.url))` 取得目前模組目錄,再組 `prompts/roles`,讓路徑意圖清楚且不依賴 `..` 抵銷檔名的技巧。",
|
"suggestion": "不要只用 `path + line` 折疊所有 comment;至少要保留同一組內的所有 botFinding,或改成以 comment id / finding 本身為單位處理,再在最後階段做去重。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Maya",
|
||||||
|
"location": "src/comments.js:214",
|
||||||
|
"problem": "這裡新增了 `postOldFindingsComment` 與 `postNewNonCriticalComment` 兩條公開的留言分流路徑,但現有測試只驗證了 `postNewCriticalComments` 與 `postFindingsReview`,完全沒有案例確認這兩個函式的篩選條件、空陣列時是否跳過、以及輸出的 Markdown 內容是否真的只包含對應的 findings。這種分流邏輯一旦算錯,就會發生該發的沒發、或不該公告的問題被貼出去。",
|
||||||
|
"suggestion": "補上這兩個函式的單元測試:空陣列時不呼叫 `postComment`;`postOldFindingsComment` 只送出 `is_new === false` 的項目;`postNewNonCriticalComment` 只送出 `is_new` 且 `level !== 'critical'` 的項目;再斷言 comment 標題與表格列數都符合預期。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Maya",
|
||||||
|
"location": "src/comments.js:171",
|
||||||
|
"problem": "`postFindingsReview` 的救援路徑只測到「批次 review 失敗後,改發逐筆 inline comment」這一段,卻沒有驗證第二次 `postReview({ comments: [] })` 也失敗時,會正確降級到 `postIssue(body)`。這條路徑是 Gitea review API 整個故障時保住摘要的最後保險絲,沒測到的話,真正出事時很容易靜默漏報。",
|
||||||
|
"suggestion": "新增一個雙重失敗測試:第一次 `postReview` 因 comments 拋錯、第二次 `postReview` 也拋錯,最後斷言有呼叫 `postIssue`,而且 inline comments 仍會依序嘗試發布。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Maya",
|
||||||
|
"location": "src/main.js:17",
|
||||||
|
"problem": "`main()` 整個流程目前沒有任何直接測試,只能靠零散的子函式單測推測結果;但這裡包含多個關鍵分支與 `process.exit` 行為,例如 preflight 失敗、bot 自動提交跳過、空 diff 提早結束、JSON 驗證失敗、以及偵測到 critical 後結束失敗。只要接線順序或退出碼改壞,現有測試不會第一時間抓到。",
|
||||||
|
"suggestion": "補一組整合測試,把 `runPreflight`、`getPRDiff`、`reconcileConversations`、`validateJSONArrayFile`、`commitAndPush` 以 stub 注入,逐一覆蓋 Step3/5/9/11 的 exit 0/1 分支,至少驗證 `process.exit` 與主要副作用被正確觸發。",
|
||||||
"is_new": true
|
"is_new": true
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
@@ -165,22 +245,54 @@
|
|||||||
"location": "src/comments.js:25",
|
"location": "src/comments.js:25",
|
||||||
"problem": "Markdown 表格列直接嵌入 role、location、suggestion,但測試沒有覆蓋 suggestion 含 `|`、換行或 Markdown 特殊字元時的輸出。這不是要求現在一定要改格式,而是目前缺少案例確認表格在真實 LLM 輸出下不會被破壞。",
|
"problem": "Markdown 表格列直接嵌入 role、location、suggestion,但測試沒有覆蓋 suggestion 含 `|`、換行或 Markdown 特殊字元時的輸出。這不是要求現在一定要改格式,而是目前缺少案例確認表格在真實 LLM 輸出下不會被破壞。",
|
||||||
"suggestion": "補一個 comment body 格式測試,輸入 suggestion 含 pipe、換行與粗體符號,斷言輸出的 Markdown 結構符合預期;若目前行為會破表,應先定義轉義或替換規則再測。",
|
"suggestion": "補一個 comment body 格式測試,輸入 suggestion 含 pipe、換行與粗體符號,斷言輸出的 Markdown 結構符合預期;若目前行為會破表,應先定義轉義或替換規則再測。",
|
||||||
"is_new": true
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "info",
|
"level": "info",
|
||||||
"role": "Rogue",
|
"role": "Rogue",
|
||||||
"location": "src/comments.js:126",
|
"location": "src/comments.js:102",
|
||||||
"problem": "countBy 用 filter(predicate).length 只為了計數卻配置中間陣列;formatFindingsStats/formatFindingsStatsLine 每列又重複掃多次,雖然 findings 通常不大,但這是在白白丟記憶體與掃描週期。",
|
"problem": "統計表與單行摘要各欄位都用 `filter(...).length` 重掃多次,同一批 findings 會被走 4 到 8 次。資料量一大,連 log 文字本身都開始吃不必要的掃描成本。",
|
||||||
"suggestion": "改成單趟 reduce 統計 new/old × level 的計數表,或讓 countBy 用 for-of 累加數字、不建立 filter 結果陣列。",
|
"suggestion": "改成單次迴圈同時累加 critical / warning / info / 未分類計數,再把結果組成表格和摘要;一次走完就好,別讓統計自己變熱點。",
|
||||||
"is_new": true
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "info",
|
||||||
|
"role": "Maya",
|
||||||
|
"location": "src/comments.js:266",
|
||||||
|
"problem": "`postOldFindingsComment` 與緊接著的 `postNewNonCriticalComment` 都是這次新加的對外 comment 發布行為,但目前沒有專門測試它們的空陣列早退、標題文字與表格內容。這會讓 comment 分流邏輯只靠間接測試支撐,回歸時很容易漏掉。",
|
||||||
|
"suggestion": "補這兩個函式的單元測試:至少驗證空陣列時不會送 comment、非空時 body 內容包含正確標題與表格,且 `postOldFindingsComment` 只收舊問題、`postNewNonCriticalComment` 只收新非 critical 問題。",
|
||||||
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "info",
|
"level": "info",
|
||||||
"role": "Rogue",
|
"role": "Rogue",
|
||||||
"location": "src/findings.js:382",
|
"location": "src/findings.js:393",
|
||||||
"problem": "loadExclusions 前面已經 normalizeExclusionEntry + dedupeExclusions,這裡又呼叫 buildExclusionContext(exclusions) 重新 normalize、dedupe、group 一輪,只為了 log groups 數;排除規則多時會多跑一趟 O(e log e) 的整理成本。",
|
"problem": "前面已經把 exclusions 正規化、去重過一次了,這裡為了 log 又再丟進 `buildExclusionContext` 重做 normalize / dedupe / group。等於同一批資料在同一輪流程裡被重算兩次,白白多吃一輪 O(n) 到 O(n log n) 的 CPU。",
|
||||||
"suggestion": "讓 buildExclusionContext 可接受已正規化/已去重的 exclusions,或直接在 loadExclusions 重用現有 exclusions 進行 group 統計,避免重複正規化與排序。",
|
"suggestion": "把第一次處理的摘要一起回傳或快取下來,後面的 log 直接重用同一份結果,不要再對同一批 exclusions 重跑分組。",
|
||||||
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "info",
|
||||||
|
"role": "Rogue",
|
||||||
|
"location": "src/resolve.js:88",
|
||||||
|
"problem": "這個 `codeWindow` 每遇到一筆 open conversation 就對整份檔案內容再 `split('\\n')` 一次。若同一個檔案有多條 thread,O(L) 的切割和陣列配置會被重複吃掉,明明同一份內容卻一直重複解剖。",
|
||||||
|
"suggestion": "先把檔案內容預先切成行陣列並快取,或讓 `codeWindow` 直接吃已分割好的 lines;這樣同檔多條對話就不用重複掃描整份內容。",
|
||||||
|
"is_new": false
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "info",
|
||||||
|
"role": "Leo",
|
||||||
|
"location": "src/resolve.js:201",
|
||||||
|
"problem": "`reconcileConversations()` 同時在做 comment 分組、關閉遠端 review、讀檔、抽 code window、AI 裁決、再把結果拆成 resolved / excluded / carried 三條路徑,流程很完整,但也很難局部理解或替換。未來任何一段判斷要調整,都得先吞下整個函式的心智負擔,維護門檻偏高。",
|
||||||
|
"suggestion": "把它拆成幾個可單獨測試的步驟,例如 `collectOpenConversations()`、`loadConversationCode()`、`judgeConversationVerdicts()`、`mapVerdictsToFindings()`,讓主流程只保留編排,不要把資料轉換與外部副作用全塞在一起。",
|
||||||
|
"is_new": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "info",
|
||||||
|
"role": "Mage",
|
||||||
|
"location": "src/usage.js:212",
|
||||||
|
"problem": "這個百分比計算只擋了 `limit <= 0`,沒有擋 `remaining < 0`。最小重現是 `resolveRemainingPercent({ available: true, used: 150, limit: 100 }, null)` 或 `remaining = -1`,會算出負百分比,讓使用量摘要出現不合理的 `-50%` 之類結果,和函式註解宣告的「負數視為無法計算」不一致。",
|
||||||
|
"suggestion": "把 `remaining < 0` 也納入無效值判斷,直接回 `null`;若你想保守顯示,可另外在輸出層把負值夾到 0,但不要讓計算層回傳負百分比。",
|
||||||
"is_new": true
|
"is_new": true
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -0,0 +1,13 @@
|
|||||||
|
# AI Code Review 忽略清單
|
||||||
|
# 符合下列前綴/路徑的檔案不會納入送給 LLM 的 git diff。
|
||||||
|
# 規則:每行一個路徑前綴(相對 repo 根),# 開頭為註解,空行略過。
|
||||||
|
# 註:任何深度的 node_modules/ 一律排除(程式內建保險),此處列出僅為明示。
|
||||||
|
|
||||||
|
.gitea/
|
||||||
|
.github/
|
||||||
|
README.md
|
||||||
|
TODO.md
|
||||||
|
package-lock.json
|
||||||
|
src/package-lock.json
|
||||||
|
dist/
|
||||||
|
node_modules/
|
||||||
+19
-17
@@ -1,6 +1,6 @@
|
|||||||
import fs from 'fs';
|
import fs from 'fs';
|
||||||
import path from 'path';
|
import path from 'path';
|
||||||
import { chatJSON } from './llm.js';
|
import { chatJSON, mapWithConcurrency, LLM_CONCURRENCY } from './llm.js';
|
||||||
import { buildAnalysisPrompt, loadRole, buildVerdictPrompt, buildLocateLinePrompt } from './roles.js';
|
import { buildAnalysisPrompt, loadRole, buildVerdictPrompt, buildLocateLinePrompt } from './roles.js';
|
||||||
import { FINDINGS_PATH, EXCLUSIONS_PATH } from './config.js';
|
import { FINDINGS_PATH, EXCLUSIONS_PATH } from './config.js';
|
||||||
import { line, ok, warn } from './log.js';
|
import { line, ok, warn } from './log.js';
|
||||||
@@ -368,14 +368,14 @@ function extractFileDiff(diff, file) {
|
|||||||
* 成功則把 location 補成 `檔案:行號`,否則保留原檔名。
|
* 成功則把 location 補成 `檔案:行號`,否則保留原檔名。
|
||||||
*/
|
*/
|
||||||
export async function resolveMissingLineNumbers(findings, diff, deps = {}) {
|
export async function resolveMissingLineNumbers(findings, diff, deps = {}) {
|
||||||
const { chatFn = chatJSON, getRole = loadRole, maxAttempts = MAX_LOCATE_ATTEMPTS } = deps;
|
const { chatFn = chatJSON, getRole = loadRole, maxAttempts = MAX_LOCATE_ATTEMPTS, concurrency = LLM_CONCURRENCY } = deps;
|
||||||
let resolved = 0;
|
// 只挑「缺行號且有檔名」的 finding;各自以獨立 LLM 子行程並行定位(併發上限見 concurrency)。
|
||||||
let pending = 0;
|
const pending = findings.filter(f => findingLine(f.location) == null
|
||||||
for (const f of findings) {
|
&& String(f.location || '').split(',')[0].split(':')[0].trim());
|
||||||
if (findingLine(f.location) != null) continue; // 已有行號
|
if (pending.length === 0) return findings;
|
||||||
|
|
||||||
|
const outcomes = await mapWithConcurrency(pending, concurrency, async (f) => {
|
||||||
const file = String(f.location || '').split(',')[0].split(':')[0].trim();
|
const file = String(f.location || '').split(',')[0].split(':')[0].trim();
|
||||||
if (!file) continue;
|
|
||||||
pending += 1;
|
|
||||||
const systemPrompt = buildLocateLinePrompt(getRole(f.role) || { name: f.role });
|
const systemPrompt = buildLocateLinePrompt(getRole(f.role) || { name: f.role });
|
||||||
const userContent = `${JSON.stringify({ file, problem: f.problem, suggestion: f.suggestion })}\n\n--- ${file} Git Diff ---\n${extractFileDiff(diff, file)}`;
|
const userContent = `${JSON.stringify({ file, problem: f.problem, suggestion: f.suggestion })}\n\n--- ${file} Git Diff ---\n${extractFileDiff(diff, file)}`;
|
||||||
let located = null;
|
let located = null;
|
||||||
@@ -390,12 +390,13 @@ export async function resolveMissingLineNumbers(findings, diff, deps = {}) {
|
|||||||
}
|
}
|
||||||
if (located != null) {
|
if (located != null) {
|
||||||
f.location = `${file}:${located}`;
|
f.location = `${file}:${located}`;
|
||||||
resolved += 1;
|
return true;
|
||||||
} else {
|
}
|
||||||
warn(`[${f.role}] ${maxAttempts} 次嘗試後仍無法定位行號,保留檔名: ${file}`);
|
warn(`[${f.role}] ${maxAttempts} 次嘗試後仍無法定位行號,保留檔名: ${file}`);
|
||||||
}
|
return false;
|
||||||
}
|
});
|
||||||
if (pending > 0) ok(`補行號: ${resolved}/${pending} 筆成功定位`);
|
|
||||||
|
ok(`補行號: ${outcomes.filter(Boolean).length}/${pending.length} 筆成功定位`);
|
||||||
return findings;
|
return findings;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -576,10 +577,11 @@ export async function filterFalsePositivesWithAI(findings, exclusions = [], chat
|
|||||||
? `${exclusionContext.prompt}\n規則:若此 finding 與上述任何一類的路徑、角色或描述高度相似,優先視為誤報或不適用。`
|
? `${exclusionContext.prompt}\n規則:若此 finding 與上述任何一類的路徑、角色或描述高度相似,優先視為誤報或不適用。`
|
||||||
: '';
|
: '';
|
||||||
|
|
||||||
// 每條 finding 各派一個防守方 sub-agent 裁決,多條時平行處理
|
// 每條 finding 各派一個防守方 sub-agent 裁決;併發上限與其他 LLM 任務共用 LLM_CONCURRENCY(預設不限制)。
|
||||||
const verdicts = await Promise.all(
|
const verdicts = await mapWithConcurrency(findings, LLM_CONCURRENCY, async (f) => ({
|
||||||
findings.map(f => judgeFindingIsFalsePositive(f, defender, exclusionHint, chatFn).then(isFP => ({ f, isFP }))),
|
f,
|
||||||
);
|
isFP: await judgeFindingIsFalsePositive(f, defender, exclusionHint, chatFn),
|
||||||
|
}));
|
||||||
const kept = verdicts.filter(v => !v.isFP).map(v => v.f);
|
const kept = verdicts.filter(v => !v.isFP).map(v => v.f);
|
||||||
ok(`AI 誤報過濾(防守方${findings.length > 1 ? '平行' : ''}裁決): ${findings.length} -> ${kept.length} 筆`);
|
ok(`AI 誤報過濾(防守方${findings.length > 1 ? '平行' : ''}裁決): ${findings.length} -> ${kept.length} 筆`);
|
||||||
return kept;
|
return kept;
|
||||||
|
|||||||
+42
-10
@@ -41,15 +41,9 @@ export function getBotReviewOutcome(message) {
|
|||||||
return match?.[1]?.toLowerCase() || 'unknown';
|
return match?.[1]?.toLowerCase() || 'unknown';
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
// 找不到 .reviewignore 時(例如其他 repo 未提供)採用的內建預設排除清單。
|
||||||
* 取得目前 PR 的完整 Git diff,並排除 CI/文件等不需審查的路徑(.gitea/、.github/、README.md、TODO.md)。
|
// 任何深度的 node_modules/ 另由 filterDiff 內建強制排除,不倚賴此清單。
|
||||||
* 透過 Gitea `GET /repos/{repo}/pulls/{index}.diff`(純文字 diff),授權使用 GITEA_TOKEN。
|
export const DEFAULT_REVIEW_IGNORE = [
|
||||||
* @returns {Promise<string>} 過濾後的 diff 文字。
|
|
||||||
* @throws {Error} 當 Gitea API 請求失敗(網路錯誤、逾時或非 2xx 狀態)時拋出 axios 例外。
|
|
||||||
*/
|
|
||||||
export async function getPRDiff() {
|
|
||||||
const resp = await axios.get(api(`/repos/${GITEA_REPOSITORY}/pulls/${PR_NUMBER}.diff`), { headers: headers(), timeout: 60000, httpsAgent });
|
|
||||||
return filterDiff(resp.data, [
|
|
||||||
'.gitea/',
|
'.gitea/',
|
||||||
'.github/',
|
'.github/',
|
||||||
'README.md',
|
'README.md',
|
||||||
@@ -57,7 +51,45 @@ export async function getPRDiff() {
|
|||||||
'package-lock.json',
|
'package-lock.json',
|
||||||
'src/package-lock.json',
|
'src/package-lock.json',
|
||||||
'dist/',
|
'dist/',
|
||||||
]);
|
];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 解析 .reviewignore 文字為排除前綴陣列(gitignore 風格)。
|
||||||
|
* 規則:每行一個路徑前綴,trim 後略過空行與 `#` 開頭的註解行。
|
||||||
|
* @param {string} text - .reviewignore 檔案內容。
|
||||||
|
* @returns {string[]} 排除前綴清單。
|
||||||
|
*/
|
||||||
|
export function parseReviewIgnore(text) {
|
||||||
|
return String(text || '')
|
||||||
|
.split('\n')
|
||||||
|
.map(l => l.trim())
|
||||||
|
.filter(l => l && !l.startsWith('#'));
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 從被審 PR 的 head ref 取得 `.reviewignore` 並解析為排除清單。
|
||||||
|
* 檔案不存在或為空時退回 {@link DEFAULT_REVIEW_IGNORE}。
|
||||||
|
* @returns {Promise<string[]>} 套用於 diff 過濾的排除前綴清單。
|
||||||
|
*/
|
||||||
|
export async function getReviewIgnore() {
|
||||||
|
const patterns = parseReviewIgnore(await getFileContentAtRef('.reviewignore'));
|
||||||
|
if (patterns.length > 0) {
|
||||||
|
line(`已套用 .reviewignore:${patterns.length} 條排除規則`);
|
||||||
|
return patterns;
|
||||||
|
}
|
||||||
|
return DEFAULT_REVIEW_IGNORE;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 取得目前 PR 的完整 Git diff,並依 `.reviewignore`(讀不到時用內建預設)排除不需審查的路徑。
|
||||||
|
* 透過 Gitea `GET /repos/{repo}/pulls/{index}.diff`(純文字 diff),授權使用 GITEA_TOKEN。
|
||||||
|
* @returns {Promise<string>} 過濾後的 diff 文字。
|
||||||
|
* @throws {Error} 當 Gitea 取 diff 的 API 請求失敗(網路錯誤、逾時或非 2xx 狀態)時拋出 axios 例外。
|
||||||
|
*/
|
||||||
|
export async function getPRDiff() {
|
||||||
|
const patterns = await getReviewIgnore();
|
||||||
|
const resp = await axios.get(api(`/repos/${GITEA_REPOSITORY}/pulls/${PR_NUMBER}.diff`), { headers: headers(), timeout: 60000, httpsAgent });
|
||||||
|
return filterDiff(resp.data, patterns);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
+33
@@ -6,6 +6,39 @@ import { getLLMConfig } from './config.js';
|
|||||||
import { recordUsage } from './usage.js';
|
import { recordUsage } from './usage.js';
|
||||||
import { line } from './log.js';
|
import { line } from './log.js';
|
||||||
|
|
||||||
|
// 每個 LLM CLI 呼叫(角色分析、補行號等)都是一個獨立子行程。預設「不限制」併發(全部同時跑);
|
||||||
|
// 若機器資源不足或撞到提供者限流,可用 AI_ASSISTANT_CONCURRENCY 設一個正整數當上限。
|
||||||
|
// 0 / 未設定 / 非正整數 → 不限制。
|
||||||
|
export const LLM_CONCURRENCY = Number(process.env.AI_ASSISTANT_CONCURRENCY) || 0;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 對 items 並行執行 async fn(保序回傳),加速多個獨立的 LLM 子行程呼叫。
|
||||||
|
*
|
||||||
|
* limit 為同時執行上限;`limit <= 0`、非數字或大於項目數時「不限制」(全部並行)。
|
||||||
|
* fn 需自行處理例外(內部 try/catch);本函式不會因單一項目 reject 而中斷其餘工作。
|
||||||
|
* @template T, R
|
||||||
|
* @param {T[]} items - 要處理的項目。
|
||||||
|
* @param {number} limit - 同時執行的上限;<=0/非數字表示不限制。
|
||||||
|
* @param {(item: T, index: number) => Promise<R>} fn - 對每個項目執行的 async 函式。
|
||||||
|
* @returns {Promise<R[]>} 與 items 對應(同索引)的結果陣列。
|
||||||
|
*/
|
||||||
|
export async function mapWithConcurrency(items, limit, fn) {
|
||||||
|
const list = Array.isArray(items) ? items : [];
|
||||||
|
const results = new Array(list.length);
|
||||||
|
if (list.length === 0) return results;
|
||||||
|
const n = Number(limit);
|
||||||
|
const workers = (!Number.isFinite(n) || n <= 0) ? list.length : Math.min(n, list.length);
|
||||||
|
let cursor = 0;
|
||||||
|
async function run() {
|
||||||
|
while (cursor < list.length) {
|
||||||
|
const i = cursor++;
|
||||||
|
results[i] = await fn(list[i], i);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
await Promise.all(Array.from({ length: workers }, run));
|
||||||
|
return results;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* 將既有 system/user prompt 合併成一次 CLI 呼叫用的輸入。
|
* 將既有 system/user prompt 合併成一次 CLI 呼叫用的輸入。
|
||||||
*/
|
*/
|
||||||
|
|||||||
+11
-4
@@ -9,6 +9,7 @@ import { getRunUsage, getRateLimit, fetchAccountQuota, formatUsageStats, formatU
|
|||||||
import { cloneRepo, commitAndPush, getRepoState } from './git.js';
|
import { cloneRepo, commitAndPush, getRepoState } from './git.js';
|
||||||
import { validateJSONArrayFile, ensureJSONArrayFileExists } from './json.js';
|
import { validateJSONArrayFile, ensureJSONArrayFileExists } from './json.js';
|
||||||
import { runPreflight } from './preflight.js';
|
import { runPreflight } from './preflight.js';
|
||||||
|
import { mapWithConcurrency, LLM_CONCURRENCY } from './llm.js';
|
||||||
import { section, step, line, input, output, result, warn, error } from './log.js';
|
import { section, step, line, input, output, result, warn, error } from './log.js';
|
||||||
|
|
||||||
const WORKSPACE = process.env.GITHUB_WORKSPACE || '/workspace';
|
const WORKSPACE = process.env.GITHUB_WORKSPACE || '/workspace';
|
||||||
@@ -120,15 +121,21 @@ async function main() {
|
|||||||
} catch (e) {
|
} catch (e) {
|
||||||
warn(`角色介紹 comment 發布失敗(繼續執行): ${e.message}`);
|
warn(`角色介紹 comment 發布失敗(繼續執行): ${e.message}`);
|
||||||
}
|
}
|
||||||
|
// 各角色以獨立 LLM 子行程並行分析(併發上限見 LLM_CONCURRENCY),單一角色失敗僅 warn 後跳過。
|
||||||
const newFindings = [];
|
const newFindings = [];
|
||||||
let fulfilledAnalyses = 0;
|
let fulfilledAnalyses = 0;
|
||||||
for (const role of roles) {
|
const roleResults = await mapWithConcurrency(roles, LLM_CONCURRENCY, async (role) => {
|
||||||
try {
|
try {
|
||||||
const findings = await analyzeWithRole(role, diff);
|
return await analyzeWithRole(role, diff);
|
||||||
fulfilledAnalyses += 1;
|
|
||||||
newFindings.push(...findings);
|
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
warn(`[${role.name}] 分析失敗(跳過): ${e.message}`);
|
warn(`[${role.name}] 分析失敗(跳過): ${e.message}`);
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
for (const findings of roleResults) {
|
||||||
|
if (findings) {
|
||||||
|
fulfilledAnalyses += 1;
|
||||||
|
newFindings.push(...findings);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (fulfilledAnalyses === 0) {
|
if (fulfilledAnalyses === 0) {
|
||||||
|
|||||||
+29
-1
@@ -1,7 +1,7 @@
|
|||||||
import { describe, it, afterEach, mock } from 'node:test';
|
import { describe, it, afterEach, mock } from 'node:test';
|
||||||
import assert from 'node:assert/strict';
|
import assert from 'node:assert/strict';
|
||||||
import axios from 'axios';
|
import axios from 'axios';
|
||||||
import { getPRDiff, filterDiff, postComment, postPullReviewComment, postPullReview, getCommitMessageBySha, getBranchHeadCommitMessage, shouldSkipBotCommit, getBotReviewOutcome, listPullReviews, getPullReviewComments, listAllReviewComments, resolvePullReviewComment, getFileContentAtRef } from '../gitea.js';
|
import { getPRDiff, filterDiff, parseReviewIgnore, getReviewIgnore, DEFAULT_REVIEW_IGNORE, postComment, postPullReviewComment, postPullReview, getCommitMessageBySha, getBranchHeadCommitMessage, shouldSkipBotCommit, getBotReviewOutcome, listPullReviews, getPullReviewComments, listAllReviewComments, resolvePullReviewComment, getFileContentAtRef } from '../gitea.js';
|
||||||
|
|
||||||
afterEach(() => mock.restoreAll());
|
afterEach(() => mock.restoreAll());
|
||||||
|
|
||||||
@@ -259,3 +259,31 @@ describe('filterDiff', () => {
|
|||||||
assert.ok(result.includes('src/main.js'));
|
assert.ok(result.includes('src/main.js'));
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('parseReviewIgnore', () => {
|
||||||
|
it('parses prefixes, skipping blanks and comments', () => {
|
||||||
|
const text = '# comment\n\n.gitea/\n dist/ \n# another\nREADME.md\n';
|
||||||
|
assert.deepEqual(parseReviewIgnore(text), ['.gitea/', 'dist/', 'README.md']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns an empty array for empty/nullish input', () => {
|
||||||
|
assert.deepEqual(parseReviewIgnore(''), []);
|
||||||
|
assert.deepEqual(parseReviewIgnore(null), []);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('getReviewIgnore', () => {
|
||||||
|
it('uses patterns fetched from .reviewignore when present', async () => {
|
||||||
|
mock.method(axios, 'get', async () => ({
|
||||||
|
data: { content: Buffer.from('a/\nb/\n# c\n').toString('base64'), encoding: 'base64' },
|
||||||
|
}));
|
||||||
|
const patterns = await getReviewIgnore();
|
||||||
|
assert.deepEqual(patterns, ['a/', 'b/']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to the default list when .reviewignore is missing/empty', async () => {
|
||||||
|
mock.method(axios, 'get', async () => ({ data: {} }));
|
||||||
|
const patterns = await getReviewIgnore();
|
||||||
|
assert.deepEqual(patterns, DEFAULT_REVIEW_IGNORE);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
+40
-1
@@ -3,7 +3,7 @@ import assert from 'node:assert/strict';
|
|||||||
import { mkdtemp, writeFile, chmod, rm, readFile } from 'fs/promises';
|
import { mkdtemp, writeFile, chmod, rm, readFile } from 'fs/promises';
|
||||||
import { tmpdir } from 'os';
|
import { tmpdir } from 'os';
|
||||||
import { join } from 'path';
|
import { join } from 'path';
|
||||||
import { extractBalancedJSON, extractJSONText, extractMeaningfulError } from '../llm.js';
|
import { extractBalancedJSON, extractJSONText, extractMeaningfulError, mapWithConcurrency } from '../llm.js';
|
||||||
|
|
||||||
const ENV_KEYS = [
|
const ENV_KEYS = [
|
||||||
'AI_ASSISTANT_CLI', 'MODEL', 'OPENCODE_MODEL', 'PATH', 'AI_ASSISTANT_TIMEOUT_MS', 'AI_ASSISTANT_MAX_BUFFER',
|
'AI_ASSISTANT_CLI', 'MODEL', 'OPENCODE_MODEL', 'PATH', 'AI_ASSISTANT_TIMEOUT_MS', 'AI_ASSISTANT_MAX_BUFFER',
|
||||||
@@ -273,3 +273,42 @@ describe('extractMeaningfulError', () => {
|
|||||||
assert.equal(extractMeaningfulError(null), '');
|
assert.equal(extractMeaningfulError(null), '');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('mapWithConcurrency', () => {
|
||||||
|
it('回傳與輸入同索引對應的結果(保序)', async () => {
|
||||||
|
const out = await mapWithConcurrency([1, 2, 3, 4], 2, async (n) => n * 10);
|
||||||
|
assert.deepEqual(out, [10, 20, 30, 40]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('遵守併發上限(同時執行數不超過 limit)', async () => {
|
||||||
|
let active = 0, peak = 0;
|
||||||
|
const wait = () => new Promise(r => setTimeout(r, 5));
|
||||||
|
await mapWithConcurrency([1, 2, 3, 4, 5, 6], 2, async () => {
|
||||||
|
active += 1; peak = Math.max(peak, active);
|
||||||
|
await wait();
|
||||||
|
active -= 1;
|
||||||
|
});
|
||||||
|
assert.ok(peak <= 2, `peak=${peak} 應 <= 2`);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('limit 大於項目數時仍全部執行', async () => {
|
||||||
|
const out = await mapWithConcurrency(['a', 'b'], 10, async (s) => s.toUpperCase());
|
||||||
|
assert.deepEqual(out, ['A', 'B']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('limit<=0 表示不限制(全部同時並行)', async () => {
|
||||||
|
let active = 0, peak = 0;
|
||||||
|
const wait = () => new Promise(r => setTimeout(r, 5));
|
||||||
|
await mapWithConcurrency([1, 2, 3, 4, 5], 0, async () => {
|
||||||
|
active += 1; peak = Math.max(peak, active);
|
||||||
|
await wait();
|
||||||
|
active -= 1;
|
||||||
|
});
|
||||||
|
assert.equal(peak, 5, `peak=${peak} 應等於項目數(不限制)`);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('空輸入回傳空陣列', async () => {
|
||||||
|
assert.deepEqual(await mapWithConcurrency([], 3, async () => 1), []);
|
||||||
|
assert.deepEqual(await mapWithConcurrency(null, 3, async () => 1), []);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user