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

Open
jiantw83 wants to merge 85 commits from develop into master
Showing only changes of commit 216bc39255 - Show all commits
@@ -8,63 +8,6 @@
"model": "gpt-5.5" "model": "gpt-5.5"
}, },
"findings": [ "findings": [
{
"reviewer": "Assassin",
"focus": "security",
"badge": "🗡️",
"severity": "嚴重",
"file": "src/index.js",
"startLine": 344,
"endLine": 348,
"problem": "建問題模式在 `addIssueDependency` 失敗時只記錄警告,接著本輪仍會回傳 0;若 `commitFindings` 後續因沒有實際 diff 可提交而沒有產生 `[failure]` 結果 commit,攻擊者只要讓嚴重 finding 被搬到追蹤 issue,且目標 Gitea 未啟用 issue dependencies 或 token 權限不足,就會 fail-open:PR 既沒有相依阻擋,也沒有失敗檢查阻擋合併。",
"suggestion": "有嚴重問題時,相依關係設定失敗應視為阻擋條件:要嘛直接回傳 1,要嘛確認 failure 結果 commit 已成功產生後才允許本輪回傳 0。不要把阻擋機制失效降級成純警告。",
"suggestedCode": "",
"id": "F001",
"verdicts": {
"Paladin": {
"exclude": false,
"reason": "保留。此條不是單純重複既有「相依 API 失敗降級缺測試」,而是指控嚴重 finding 搬到 issue 後,dependency 失敗可能使阻擋機制失效;已知排除事項未涵蓋此安全語義。"
}
}
},
{
"reviewer": "Mage",
"focus": "logic",
"badge": "🔮",
"severity": "警告",
"file": "src/index.js",
"startLine": 330,
"endLine": 338,
"problem": "在建問題模式下,這段新增的 PR 回貼 issue 連結會在每次審查有保留問題時都新增一則 PR 留言,但同一流程前面明確跳過 `resolveOldComments`(建問題模式不清理 PR 舊留言)。最小重現:PR 第一次審查建立 issue #10 並在 PR 留連結;後續推新 commit 再跑一次,建立 issue #11 並再留一則連結。PR 上會同時存在 #10 與 #11,舊 issue 可能已過時,讀者無法判斷哪個才是目前審查結果。",
"suggestion": "建問題模式也應對本 action 先前的 PR 連結留言做過時標記,或在新增連結前查找並更新既有連結留言。若要避免碰觸 issue 內的審查內容,清理範圍可限制在 PR 上含 `MARK` 且標題為「已建立追蹤問題」的留言。",
"suggestedCode": "",
"id": "F009",
"verdicts": {
"Paladin": {
"exclude": false,
"reason": "保留。已知排除事項只裁示舊審查留言在本輪結果前標過時的時機;本條指控建問題模式跳過 PR 舊連結留言清理,導致多個追蹤 issue 連結並存,未被既有排除涵蓋。"
}
}
},
{
"reviewer": "Bard",
"focus": "style",
"badge": "🎼",
"severity": "建議",
"file": "src/lib/diagnostics.js",
"startLine": 55,
"endLine": 63,
"problem": "`agentFailureDetail` 裡的 `stderr` 與 `stdout` 區塊幾乎同譜重奏:取值、slice、redact、判斷、push 只差欄位名。這種重複雖小,卻讓後續若要調整遮罩或長度時容易改一半走調。",
"suggestion": "建議抽出小 helper,例如 `appendRedactedOutput(parts, label, value)`,讓 stderr/stdout 共用同一段處理節奏。",
"suggestedCode": "",
"id": "F005",
"verdicts": {
"Paladin": {
"exclude": false,
"reason": "保留。此條針對 diagnostics.js 中 stderr/stdout 處理重複的維護性問題,歷史 findings 主要是安全診斷缺測試與外洩風險,並非同一指控。"
}
}
},
{ {
"reviewer": "Bard", "reviewer": "Bard",
"focus": "style", "focus": "style",
@@ -83,25 +26,6 @@
"reason": "保留。歷史 findings 主要涵蓋 resolveMergeBase 缺測試與診斷不足,本條指向函式內策略編排、診斷組裝與錯誤包裝混雜的可維護性問題,未被既有排除事項完整涵蓋。" "reason": "保留。歷史 findings 主要涵蓋 resolveMergeBase 缺測試與診斷不足,本條指向函式內策略編排、診斷組裝與錯誤包裝混雜的可維護性問題,未被既有排除事項完整涵蓋。"
} }
} }
},
{
"reviewer": "Bard",
"focus": "style",
"badge": "🎼",
"severity": "建議",
"file": "src/lib/gitrepo.js",
"startLine": 317,
"endLine": 346,
"problem": "`pushWithCredential` 的 JSDoc 已經很完整,但正文註解再次長篇解釋 checkout token、PAT、extraheader 清空等細節;文件與程式內註解重複奏同一段旋律,反而稀釋真正需要看的程式碼。",
"suggestion": "保留 JSDoc 的背景說明,函式內註解縮成操作提示即可,例如只說明「先清空 checkout extraheader,再注入本次 PAT header」。",
"suggestedCode": "",
"id": "F004",
"verdicts": {
"Paladin": {
"exclude": false,
"reason": "保留。既有排除事項雖有 push-token manifest 說明重複,但未涵蓋 pushWithCredential 函式內 JSDoc 與正文註解重複;證據不足以判定為重複或誤報。"
}
}
} }
], ],
"excluded": [ "excluded": [