fix(diagnostics): 處理 ai review findings #32

Open
jiantw83 wants to merge 85 commits from develop into master
2 changed files with 154 additions and 0 deletions
Showing only changes of commit 4c2b6d4265 - Show all commits
+11
View File
@@ -185,5 +185,16 @@
"endLine": 81,
"problem": "新增的 AI CLI 失敗診斷只在失敗與 debug 模式執行,但沒有測試覆蓋其邊界:`res`/`error` 缺值、逾時與 signal、debug 大小寫及空白、控制字元單行化、500 字截斷,以及多種 token/URL 帳密遮罩。這些失敗路徑若回歸,可能使診斷完全缺失或將未遮罩內容寫入 CI log。",
"reason": "Paladin:可排除(與歷史 finding 重複)。歷史 finding 已記錄 AI CLI 失敗診斷在退出狀態、空輸出、單行化、截斷及相關邊界缺少測試;本條是對同一測試缺口的細項枚舉。"
},
{
"addedAt": "2026/07/20 15:57:46",
"prNumber": 6,
"reviewer": "Mage",
"severity": "嚴重",
"file": "src/index.js",
"startLine": 268,
"endLine": 268,
"problem": "舊審查留言仍在本回合結果完整發布前就被標記為過時;建議把清理舊留言移到本輪所有必要結果均成功產生之後,避免失敗重跑留下「舊結果已清除、新結果不完整」的中間狀態。",
"reason": "維護者裁示(議題 #8 留言 #4838,系統管理員):此功能用途為把前一輪 PR 產生的訊息標記為已解決/過時,讓審查人員辨識舊訊息,因此刻意在本回合結果之前執行,不可延後,否則會誤導審查人員。依維護者決定不採納此修改,記為排除以免後續審查重複提出。"
}
]
@@ -0,0 +1,143 @@
{
"generatedAt": "2026/07/20 15:57:46",
"commitSha": "a6534ee0764fe2363ebbb5525586ad3868dc6720",
"prNumber": 6,
"tool": {
"name": "code-review-resolve",
"version": "0.0.8",
"model": "(工具預設)"
},
"findings": [
{
"id": "F001",
"reviewer": "Assassin",
"focus": "security",
"badge": "🗡️",
"severity": "嚴重",
"file": "src/lib/review.js",
"startLine": 72,
"endLine": 81,
"problem": "開啟 ACTIONS_STEP_DEBUG=true 後,會把攻擊者可間接操控的 AI CLI stderrstdout(經黑名單式 redactSecrets 遮罩)寫入長期保存的 CI log。短 token、JWT、含標點或空白的密碼、非典型金鑰及 PII 仍可能繞過遮罩。建議即使除錯模式也只記錄退出碼/signal/逾時/診斷 ID,若需原文則寫入受限、短期保存、需授權取得的安全 artifact。",
"suggestion": "CI log 不輸出 AI CLI 原文,即使 debug 模式;需要原文時改寫入存取受限的安全 artifact,並套用允許清單式結構化診斷與 PII/機密掃描。此為安全性與可除錯性的政策取捨:現行已刻意保留 debug-gated 遮罩輸出,是否完全移除需維護者裁示。",
"suggestedCode": "function agentFailureDetail(res) {\n const err = res && res.error;\n if (err && err.killed) return '已逾時終止';\n if (err && typeof err.code === 'number') return `exit ${err.code}`;\n if (err && err.signal) return `signal ${err.signal}`;\n return 'AI CLI 執行失敗(原始輸出已隱藏)';\n}"
},
{
"id": "F002",
"reviewer": "Leo",
"focus": "maintainability",
"badge": "🧰",
"severity": "警告",
"file": "src/index.js",
"startLine": 176,
"endLine": 218,
"problem": "main() 內新增 issueBuffer、可變 issue、postComment 與 ensureIssueCreated 閉包,後續多處依 ctx.createIssue 分支。留言路由、issue 生命週期、標籤、相依關係與審查編排共享同一批可變狀態;未來增加發布目的地或重試策略時須同步理解並修改整個超長主流程,測試也只能透過 main() 間接覆蓋。",
"suggestion": "抽出具明確介面的發布器(如 PrReviewPublisher 與 IssueReviewPublisher),封裝留言暫存、issue 建立、沖刷、問題明細與收束關聯;main() 只呼叫一致的 publishContextpublishFindingsfinalize,便於分別注入假的 Gitea client 測試兩種模式並移除散落的模式判斷。屬大範圍重構+設計取捨,需維護者確認方向。",
"suggestedCode": ""
},
{
"id": "F003",
"reviewer": "Maya",
"focus": "testing",
"badge": "🧪",
"severity": "警告",
"file": "src/index.js",
"startLine": 184,
"endLine": 245,
"problem": "建問題模式新增「issue 建立前暫存留言、建立後依序沖刷」的狀態流程,但沒有對應測試。尚未驗證 createIssue 或沖刷途中拋錯、空 buffer、重複呼叫、以及一般/建問題模式留言目的地是否正確;失敗路徑可能造成 issue 已建立但內容不完整或留言誤發到 PR。",
"suggestion": "補主流程整合測試並 mock Gitea API,驗證:一般模式直接發 PR 且記錄留言 id;issue 未建立時只暫存;建立後依序沖刷並清空 buffer;createIssue 與第 N 則沖刷留言失敗時以失敗結束且不再發布後續內容。屬測試架構決策(專案目前無測試框架)。",
"suggestedCode": ""
},
{
"id": "F004",
"reviewer": "Maya",
"focus": "testing",
"badge": "🧪",
"severity": "警告",
"file": "src/index.js",
"startLine": 309,
"endLine": 395,
"problem": "建問題模式核心分支被大幅改寫,但沒有測試驗證各種 findings 組合與 API 失敗行為。kept.length===0、只有嚴重、只有警告/建議、兩者皆有,以及標籤挑選/issue 建立/PR 回貼連結/相依 API 失敗等路徑尚未試煉,無法確認「無問題完全靜默」「有問題才建 issue」及相依失敗僅降級等契約成立。",
"suggestion": "以參數化測試覆蓋 findings 四種組合,斷言 API 呼叫順序/次數/issue number/統計;並分別讓 listLabelsselectLabelscreateIssue/問題留言/PR 連結留言/addIssueDependency 拋錯,驗證哪些中止、哪些僅記警告續行;零 findings 時斷言所有寫入 API 皆不呼叫。屬測試架構決策。",
"suggestedCode": ""
},
{
"id": "F005",
"reviewer": "Maya",
"focus": "testing",
"badge": "🧪",
"severity": "警告",
"file": "src/lib/gitea.js",
"startLine": 171,
"endLine": 215,
"problem": "新增的問題相依 API 包裝沒有測試驗證 endpoint、HTTP method 與 payload。特別是 addIssueDependency 容易顛倒的「PR 相依於 issue」方向未被斷言;若 index 或 body 欄位放反,合併阻擋語意會相反。(原併列的 addLabelsToIssue 已於本次移除。)",
"suggestion": "mock 底層 API,對相依關係使用不同的 PR/issue 編號,精確斷言 URL 指向 PR、body.index 指向阻擋來源 issue,並覆蓋 API 非 2xx 時錯誤原樣往上拋出的案例。屬測試架構決策。",
"suggestedCode": ""
},
{
"id": "F006",
"reviewer": "Maya",
"focus": "testing",
"badge": "🧪",
"severity": "警告",
"file": "src/lib/gitrepo.js",
"startLine": 103,
"endLine": 157,
"problem": "resolveMergeBase 新增多階段 fetch/重試策略,卻沒有測試鎖定淺層與失敗路徑:首次成功提早返回、非 shallow 不 unshallow、某次 fetch 失敗後仍嘗試下一策略、補抓成功立即停止、全部失敗時診斷與 cause 是否完整;易因呼叫順序或 off-by-one 在 runner 上才暴露。",
"suggestion": "將 git 執行器注入或 stub,建立表格化案例覆蓋首次成功/unshallow 後成功/deepen base 後成功/deepen HEAD 後成功/各 fetch 個別失敗/全部失敗;逐案斷言 git 參數與順序、成功後不再額外 fetch,並檢查最終錯誤含各策略成敗摘要且保留首次錯誤為 cause。屬可測性重構+測試架構決策。",
"suggestedCode": ""
},
{
"id": "F007",
"reviewer": "Maya",
"focus": "testing",
"badge": "🧪",
"severity": "警告",
"file": "src/lib/gitrepo.js",
"startLine": 249,
"endLine": 289,
"problem": "推送流程新增 pushToken 分支,但沒有測試驗證兩套認證策略:有 PAT 時略過 origin、PAT 推送失敗不退回其他 token、無 PAT 時 origin 成功不重試、origin 失敗才用一般 token;也沒有案例保護含憑證資訊不出現在錯誤或測試輸出中。(本次已將認證改經 env 傳入並遮蔽 push 錯誤,測試仍待補。)",
"suggestion": "mock git 執行器與 URLenv 組裝,補測 pushToken 有值/空、origin 成功/失敗、PAT 推送失敗及含特殊字元等案例;斷言 push 目標與呼叫次數,並確保任何拋出的錯誤、log 或快照都不含原始 token。屬測試架構決策。",
"suggestedCode": ""
},
{
"id": "F008",
"reviewer": "Maya",
"focus": "testing",
"badge": "🧪",
"severity": "警告",
"file": "src/lib/review.js",
"startLine": 16,
"endLine": 80,
"problem": "redactSecretsagentFailureDetail 直接決定 CI 日誌是否洩漏內容及失敗診斷是否可用,但沒有測試覆蓋。空值、控制字元、Authorization/Bearer、URL 帳密、各種 token 樣式、截斷,以及 ACTIONS_STEP_DEBUG 大小寫與未啟用時隱藏 stdout/stderr 等邊界尚未驗證。",
"suggestion": "為這兩個純函式補單元測試(必要時受控匯出或抽獨立模組),以假憑證逐一測試遮罩規則與換行注入並斷言輸出不含原始秘密;保存還原 ACTIONS_STEP_DEBUG 驗證預設/true/混合大小寫/逾時/exit codesignal/無 error/超長輸出。屬測試架構決策。",
"suggestedCode": ""
},
{
"id": "F009",
"reviewer": "Bard",
"focus": "style",
"badge": "🎼",
"severity": "建議",
"file": "action.yml",
"startLine": 47,
"endLine": 61,
"problem": "push-token 的用途與退回行為在區塊註解、欄位描述、required 與 default 註解中反覆說明,且單行 description 過長,資訊雖完整但重複,日後修改語意易只改到一處。",
"suggestion": "保留一段「為何需要 PAT」的必要背景,其餘讓欄位名稱、required、default 自行表意,將 description 收斂成呼叫端真正需要知道的契約。註:本專案採 doc-funcs 高密度註解慣例,是否精簡屬慣例取捨,需維護者確認。",
"suggestedCode": " # 專用推送 PAT;以 PAT 推送可重新觸發 CI。留空時沿用 token。\n push-token:\n description: '推送審查結果 commit 的 PAT(留空時沿用 token'\n required: false\n default: ''"
},
{
"id": "F010",
"reviewer": "Bard",
"focus": "style",
"badge": "🎼",
"severity": "建議",
"file": "src/index.js",
"startLine": 203,
"endLine": 226,
"problem": "ensureIssueCreated 之名帶有「已存在便沿用」的冪等語意,實際卻無條件建立新 issue 並悄悄改寫外層 issue;名稱、行為與副作用不一致,閱讀呼叫處易形成錯誤預期。",
"suggestion": "若此函式只允許呼叫一次,改用直接表達「建立並沖刷暫存留言」的名稱並回傳建立結果,由呼叫端明確指派 issue,讓資料流一眼可見。註:本項與 F002(抽出 publisher 大重構)指向同一段核心流程、維護者正審視中,宜與該重構一併處理,避免重複改動。",
"suggestedCode": "const createIssueAndFlushBuffer = async (labelIds = []) => {\n const createdIssue = await gitea.createIssue(ctx, {\n title: ctx.prTitle || `AI Code ReviewPR #${ctx.prNumber}`,\n body: templates.issueBody({ prNumber: ctx.prNumber, prBody: ctx.prBody }),\n labels: labelIds,\n });\n for (const body of issueBuffer) {\n await gitea.createCommentOnIssue(ctx, createdIssue.number, body);\n }\n issueBuffer.length = 0;\n return createdIssue;\n};\nissue = await createIssueAndFlushBuffer(labelIds);"
}
],
"excluded": []
}