fix(diagnostics): 處理 ai review findings #32
@@ -59,19 +59,6 @@
|
|||||||
"problem": "推送流程新增 pushToken 分支，但沒有測試驗證兩套認證策略：有 PAT 時略過 origin、PAT 推送失敗不退回其他 token、無 PAT 時 origin 成功不重試、origin 失敗才用一般 token；也沒有案例保護含憑證資訊不出現在錯誤或測試輸出中。（本次已將認證改經 env 傳入並遮蔽 push 錯誤，測試仍待補。）",
|
"problem": "推送流程新增 pushToken 分支，但沒有測試驗證兩套認證策略：有 PAT 時略過 origin、PAT 推送失敗不退回其他 token、無 PAT 時 origin 成功不重試、origin 失敗才用一般 token；也沒有案例保護含憑證資訊不出現在錯誤或測試輸出中。（本次已將認證改經 env 傳入並遮蔽 push 錯誤，測試仍待補。）",
|
||||||
"suggestion": "mock git 執行器與 URL／env 組裝，補測 pushToken 有值／空、origin 成功／失敗、PAT 推送失敗及含特殊字元等案例；斷言 push 目標與呼叫次數，並確保任何拋出的錯誤、log 或快照都不含原始 token。屬測試架構決策。",
|
"suggestion": "mock git 執行器與 URL／env 組裝，補測 pushToken 有值／空、origin 成功／失敗、PAT 推送失敗及含特殊字元等案例；斷言 push 目標與呼叫次數，並確保任何拋出的錯誤、log 或快照都不含原始 token。屬測試架構決策。",
|
||||||
"suggestedCode": ""
|
"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: ''"
|
|
||||||
}
|
}
|
||||||
],
|
],
|
||||||
"excluded": []
|
"excluded": []
|
||||||
|
|||||||
@@ -120,20 +120,6 @@
|
|||||||
"suggestedCode": "",
|
"suggestedCode": "",
|
||||||
"sourceIssue": 15
|
"sourceIssue": 15
|
||||||
},
|
},
|
||||||
{
|
|
||||||
"id": "F015",
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "src/lib/gitea.js",
|
|
||||||
"startLine": 190,
|
|
||||||
"endLine": 197,
|
|
||||||
"problem": "`dependency` 作為參數名太薄，與 `issueNumber` 並列時看不出它也是 issue 編號。讀到 `addIssueDependency(ctx, ctx.prNumber, issue.number)` 時，語意要靠上下文補拍子。",
|
|
||||||
"suggestion": "改用更完整的名稱，例如 `dependencyIssueNumber` 或 `blockingIssueNumber`，讓「誰被誰阻擋」在簽名裡就清楚成形。",
|
|
||||||
"suggestedCode": "```\nfunction addIssueDependency(ctx, issueNumber, dependencyIssueNumber) {\n return api(ctx, 'POST', `/repos/${ctx.owner}/${ctx.repo}/issues/${issueNumber}/dependencies`, {\n index: dependencyIssueNumber,\n owner: ctx.owner,\n repo: ctx.repo,\n });\n}\n```",
|
|
||||||
"sourceIssue": 15
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
"id": "F022",
|
"id": "F022",
|
||||||
"reviewer": "Leo",
|
"reviewer": "Leo",
|
||||||
@@ -148,34 +134,6 @@
|
|||||||
"suggestedCode": "",
|
"suggestedCode": "",
|
||||||
"sourceIssue": 18
|
"sourceIssue": 18
|
||||||
},
|
},
|
||||||
{
|
|
||||||
"id": "F023",
|
|
||||||
"reviewer": "Mage",
|
|
||||||
"focus": "",
|
|
||||||
"badge": "🔮",
|
|
||||||
"severity": "警告",
|
|
||||||
"file": "src/index.js",
|
|
||||||
"startLine": 374,
|
|
||||||
"endLine": 387,
|
|
||||||
"problem": "建問題模式下只要有任何保留 finding 就會建立 issue，且後續一律把 PR 設為相依於該 issue；但收尾結果仍是 `severe.length === 0 ? 'success' : 'failure'`。最小情境：攻擊方只產生 1 條「建議」，`severe.length` 為 0，action commit `[success]`，但 PR 被 issue dependency 擋住無法合併。這讓「success＝可通過」與「非嚴重問題也阻擋合併」兩個語義互相矛盾。",
|
|
||||||
"suggestion": "明確對齊語義：若只有嚴重問題才應阻擋合併，則只在 `severe.length > 0` 時建立 dependency；若所有保留問題都要阻擋合併，則 result/exit code 不應只看嚴重問題。",
|
|
||||||
"suggestedCode": "```\nif (severe.length > 0) {\n try {\n await gitea.addIssueDependency(ctx, ctx.prNumber, issue.number);\n log('建問題', 'INF', `已將 PR #${ctx.prNumber} 設為相依於 issue #${issue.number}。`);\n } catch (err) {\n log('建問題', 'WRN', `設定 PR 相依失敗（可能未啟用「問題相依」功能）：${err.message}。`);\n }\n}\n```",
|
|
||||||
"sourceIssue": 18
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"id": "F024",
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "src/index.js",
|
|
||||||
"startLine": 179,
|
|
||||||
"endLine": 181,
|
|
||||||
"problem": "`issueBuffer` 的旋律太含糊：讀者會以為裡面放的是 issue，實際上暫存的是尚未送出的留言 body。命名沒有把資料形狀唱清楚。",
|
|
||||||
"suggestion": "改成能描述內容與用途的名稱，例如 `pendingIssueCommentBodies`，並同步調整註解與迴圈變數。",
|
|
||||||
"suggestedCode": "```\nconst pendingIssueCommentBodies = [];\nlet issue = null;\n```",
|
|
||||||
"sourceIssue": 18
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
"id": "F025",
|
"id": "F025",
|
||||||
"reviewer": "Assassin",
|
"reviewer": "Assassin",
|
||||||
@@ -204,34 +162,6 @@
|
|||||||
"suggestedCode": "",
|
"suggestedCode": "",
|
||||||
"sourceIssue": 19
|
"sourceIssue": 19
|
||||||
},
|
},
|
||||||
{
|
|
||||||
"id": "F027",
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "src/index.js",
|
|
||||||
"startLine": 181,
|
|
||||||
"endLine": 185,
|
|
||||||
"problem": "`issue` 這個變數名太素，像樂譜上只寫「音符」卻不說是哪一聲部。此處承載的是建問題模式建立出的追蹤 issue，後面還會與 PR issue 編號、Gitea issue API 參數交錯出現，名稱過泛會讓閱讀節奏變濁。",
|
|
||||||
"suggestion": "改成能表明角色的名稱，例如 `trackingIssue`。對應的 `ensureIssueCreated` 也可改為 `ensureTrackingIssueCreated`，讓閉包狀態與用途一眼對上。",
|
|
||||||
"suggestedCode": "```\nconst issueBuffer = [];\nlet trackingIssue = null;\n\nconst postComment = async (body) => {\n if (ctx.createIssue) {\n if (trackingIssue) return gitea.createCommentOnIssue(ctx, trackingIssue.number, body);\n issueBuffer.push(body);\n return null;\n }\n const created = await gitea.createIssueComment(ctx, body);\n currentRunCommentIds.add(created.id);\n return created;\n};\n```",
|
|
||||||
"sourceIssue": 19
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"id": "F029",
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "src/lib/templates.js",
|
|
||||||
"startLine": 388,
|
|
||||||
"endLine": 405,
|
|
||||||
"problem": "`issueLinkComment` 產生的是 PR 上唯一的建問題模式回貼留言，但名稱少了 PR 的聲部；同檔已有 `issueBody`、`issueFindingComment`，乍看會以為這也是 issue 內留言模板，命名層次不夠分明。",
|
|
||||||
"suggestion": "改名為 `prIssueLinkComment` 或 `trackingIssueLinkComment`，讓模板的投遞位置與用途直接寫在名稱裡，避免與 issue 內文、issue finding 留言混成一團。",
|
|
||||||
"suggestedCode": "```\nfunction prIssueLinkComment({ issueNumber, issueUrl, severeCount, otherCount }) {\n return `${MARK}\n## 🔍 AI Code Review｜已建立追蹤問題\n\n本次審查結果已彙整到 issue [#${issueNumber}](${issueUrl})（🔴 嚴重 ${severeCount} 條、🟠🔵 警告＋建議 ${otherCount} 條），請至該問題追蹤與討論。`;\n}\n```",
|
|
||||||
"sourceIssue": 19
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
"id": "F031",
|
"id": "F031",
|
||||||
"reviewer": "Leo",
|
"reviewer": "Leo",
|
||||||
@@ -246,34 +176,6 @@
|
|||||||
"suggestedCode": "",
|
"suggestedCode": "",
|
||||||
"sourceIssue": 21
|
"sourceIssue": 21
|
||||||
},
|
},
|
||||||
{
|
|
||||||
"id": "F032",
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "src/index.js",
|
|
||||||
"startLine": 183,
|
|
||||||
"endLine": 224,
|
|
||||||
"problem": "`issue` 與 `issueBuffer` 的命名過於泛泛；在 Gitea 裡 PR 也是 issue，追蹤問題也是 issue，單靠 `issue` 這個名字無法唱出它究竟是哪一個聲部。",
|
|
||||||
"suggestion": "建議改成更具語義的名稱，例如 `trackingIssue`、`trackingIssueCommentBuffer`，讓讀者不用回讀 create-issue 模式的整段脈絡。",
|
|
||||||
"suggestedCode": "```\nlet trackingIssue = null;\nconst trackingIssueCommentBuffer = [];\n```",
|
|
||||||
"sourceIssue": 21
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"id": "F033",
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "src/lib/gitea.js",
|
|
||||||
"startLine": 190,
|
|
||||||
"endLine": 194,
|
|
||||||
"problem": "`addIssueDependency(ctx, issueNumber, dependency)` 的兩個參數名稱太相似，且 `dependency` 少了 issue 語義。這支 API 的方向性本來就容易讀錯，命名再模糊就像兩個音符共用同一個名字。",
|
|
||||||
"suggestion": "建議把參數改成能表達方向的名稱，例如 `blockedIssueNumber` 與 `blockingIssueNumber`，呼叫端也會更清楚是誰被誰擋住。",
|
|
||||||
"suggestedCode": "```\nfunction addIssueDependency(ctx, blockedIssueNumber, blockingIssueNumber) {\n return api(ctx, 'POST', `/repos/${ctx.owner}/${ctx.repo}/issues/${blockedIssueNumber}/dependencies`, {\n index: blockingIssueNumber,\n owner: ctx.owner,\n repo: ctx.repo,\n });\n}\n```",
|
|
||||||
"sourceIssue": 21
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
"id": "F040",
|
"id": "F040",
|
||||||
"reviewer": "Assassin",
|
"reviewer": "Assassin",
|
||||||
|
|||||||
@@ -7,141 +7,7 @@
|
|||||||
"version": "codex-cli 0.144.6",
|
"version": "codex-cli 0.144.6",
|
||||||
"model": "gpt-5.5"
|
"model": "gpt-5.5"
|
||||||
},
|
},
|
||||||
"findings": [
|
"findings": [],
|
||||||
{
|
|
||||||
"reviewer": "Mage",
|
|
||||||
"focus": "logic",
|
|
||||||
"badge": "🔮",
|
|
||||||
"severity": "嚴重",
|
|
||||||
"file": "src/index.js",
|
|
||||||
"startLine": 149,
|
|
||||||
"endLine": 153,
|
|
||||||
"problem": "這個變更把「本輪審查」改成即使有嚴重問題也回傳 0，並假設後續由 `[ai-review-bot][failure]` 結果 commit 觸發下一輪 CI 再失敗。最小重現：workflow 仍依 action.yml 常見用法傳入自動 token（或任一不會觸發 synchronize CI 的 token）→ AI 找到 1 條嚴重問題 → 本輪成功建立留言與 push 結果 commit，但該 push 不觸發下一輪 → 沒有任何檢查讀到 `[failure]` commit，PR 最終呈現通過。這是未驗證的外部時序假設；嚴重 finding 會被靜默放行。",
|
|
||||||
"suggestion": "不要讓失敗判定完全依賴下一輪 CI。若 `severe.length > 0`，本輪在完成留言與結果落地後仍應回傳 1；或至少提供明確 input 控制 direct-fail，並在無法驗證 token 會觸發 CI 時預設直接 fail。",
|
|
||||||
"suggestedCode": "",
|
|
||||||
"id": "F009",
|
|
||||||
"verdicts": {
|
|
||||||
"Paladin": {
|
|
||||||
"exclude": false,
|
|
||||||
"reason": "保留。此條指控嚴重 finding 需仰賴下一輪 CI 才失敗，且自動 token 可能不觸發 CI，與既有 pushToken 測試缺口或 PAT 風險 finding 不屬同一處同一失效模式；未能排除。"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"reviewer": "Mage",
|
|
||||||
"focus": "logic",
|
|
||||||
"badge": "🔮",
|
|
||||||
"severity": "警告",
|
|
||||||
"file": "src/lib/review.js",
|
|
||||||
"startLine": 707,
|
|
||||||
"endLine": 718,
|
|
||||||
"problem": "`postOthersToIssue` 以並行方式送出多則 issue 留言時，issue 上的實際留言順序取決於 API 回應與資料庫寫入完成順序，不保證等於已排序的 findings 順序。最小重現：兩條建議分別位於 `a.js:10` 與 `a.js:20`，第二個 API 較快完成，issue 會先出現第 20 行問題，再出現第 10 行問題；使用者逐檔逐行處理時順序會錯亂。",
|
|
||||||
"suggestion": "若 issue 留言順序是介面契約，逐則 `await gitea.createCommentOnIssue(...)` 發送；若要保留並行，需在每則留言標題加入穩定序號，例如 `2/5`，讓非同步完成不破壞閱讀順序。",
|
|
||||||
"suggestedCode": "",
|
|
||||||
"id": "F011",
|
|
||||||
"verdicts": {
|
|
||||||
"Paladin": {
|
|
||||||
"exclude": false,
|
|
||||||
"reason": "保留。歷史 findings 未涵蓋 postOthersToIssue 並行送出導致 issue 留言順序不穩定；目前也無排除事項可套用，故保留。"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "style",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "action.yml",
|
|
||||||
"startLine": 22,
|
|
||||||
"endLine": 24,
|
|
||||||
"problem": "manifest 的註解忽然奏起一整段實作細節：PAT、CI 觸發、主程式步驟 1 全擠在 input 說明旁，和同檔其他「介面用途」型註解的節奏不一致。讀者只是想知道 token 該填什麼，卻被迫聽完流程旁白。",
|
|
||||||
"suggestion": "把 action.yml 留給介面契約；細節移到 README 或主流程文件。此處可濃縮成「建議使用可觸發 CI 的 PAT」即可。",
|
|
||||||
"suggestedCode": "# 建議傳入能觸發 CI 的 PAT；自動 token 推送結果 commit 時可能不會再觸發 workflow。",
|
|
||||||
"id": "F002",
|
|
||||||
"verdicts": {
|
|
||||||
"Paladin": {
|
|
||||||
"exclude": false,
|
|
||||||
"reason": "保留。已知排除事項與歷史 findings 主要涵蓋 push-token 說明重複或 PAT 安全風險；本條指向 action.yml 中 token input 註解混入主流程步驟細節，並非同一處同一問題，未能排除。"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "style",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "readme.md",
|
|
||||||
"startLine": 40,
|
|
||||||
"endLine": 47,
|
|
||||||
"problem": "mermaid 圖的節點 ID 旋律走岔了：畫面標示是 3→4→5→...→8→2→9，但節點名稱卻用 S3、S4、S2 來回跳。即使流程語意想表達「步驟 2 延後」，節點 ID 與視覺順序不一致，會讓維護者在對照圖與文字時多繞一圈。",
|
|
||||||
"suggestion": "讓節點 ID 維持閱讀順序，將真正的流程步驟放在節點文字裡。例如用 N2、N3 或 A、B 這類中性 ID，避免 S2 看起來像應該排在 S1 後面。",
|
|
||||||
"suggestedCode": "S1 -->|未命中| N3[3 偵測 AI 工具並留言]\n N3 --> N4[4 讀 .reviewignore 整理 diff 並留言]\n N4 --> N5[5 攻擊方登場留言]\n N5 --> N6[6 攻擊方 sub agent 並行找問題]\n N6 --> N7[7 防守方登場留言]\n N7 --> N8[8 防守方裁決 → 保存 findings ＋ 誤判回寫 exclusions.json]\n N8 --> N2[2 延後將舊留言標記解決（成功產生結果後才執行）]\n N2 --> S9[9 嚴重問題逐條掛行留言]",
|
|
||||||
"id": "F003",
|
|
||||||
"verdicts": {
|
|
||||||
"Paladin": {
|
|
||||||
"exclude": false,
|
|
||||||
"reason": "保留。歷史 findings 雖有流程步驟硬編碼與文件同步成本問題，但本條聚焦 mermaid 節點 ID 與視覺流程順序不一致，屬不同可讀性指控，未命中既有排除。"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "style",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "src/index.js",
|
|
||||||
"startLine": 122,
|
|
||||||
"endLine": 139,
|
|
||||||
"problem": "`main()` 的 JSDoc 從函式說明變成流程章回。步驟、例外模式、issue 建立時機、相依關係、commit 規則全塞在同一段，和程式下方已存在的分段註解重複，讀起來像同一旋律被兩把琴同時彈奏。",
|
|
||||||
"suggestion": "JSDoc 保留函式職責、回傳值與關鍵模式差異即可；完整 10 步驟流程交給 README 或下方區塊註解。這會讓 `main()` 開頭更快進入正題。",
|
|
||||||
"suggestedCode": "",
|
|
||||||
"id": "F004",
|
|
||||||
"verdicts": {
|
|
||||||
"Paladin": {
|
|
||||||
"exclude": false,
|
|
||||||
"reason": "保留。歷史 findings 有步驟編號散落與 main() 內閉包 JSDoc 過重等問題，但本條指向 main() 函式 JSDoc 與下方流程註解重複，範圍與主張不完全相同，證據不足以排除。"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "style",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "src/lib/diagnostics.js",
|
|
||||||
"startLine": 45,
|
|
||||||
"endLine": 47,
|
|
||||||
"problem": "`res` 這個參數名太短促，和檔內其他 `text`、`parts`、`stderr`、`stdout` 這些直白命名相比顯得含糊。診斷工具本該讓人少猜一點，這裡卻讓讀者先猜它是哪一種 result。",
|
|
||||||
"suggestion": "改用 `result` 或 `agentResult`，讓函式簽名本身就說清楚資料來源。",
|
|
||||||
"suggestedCode": "function agentFailureDetail(agentResult) {\n const parts = [];\n const err = agentResult && agentResult.error;",
|
|
||||||
"id": "F005",
|
|
||||||
"verdicts": {
|
|
||||||
"Paladin": {
|
|
||||||
"exclude": false,
|
|
||||||
"reason": "保留。未見已知排除事項或歷史 finding 涵蓋 diagnostics.js 中 res 參數命名過短的問題；屬新的命名可讀性指控。"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "style",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "test/gitea.test.js",
|
|
||||||
"startLine": 8,
|
|
||||||
"endLine": 8,
|
|
||||||
"problem": "`fn` 是一個太倉促的縮寫，放在測試輔助函式裡尤其刺耳；同一行已有 `handler` 這種完整命名，`fn` 顯得像漏拍的音符。",
|
|
||||||
"suggestion": "改成 `callback` 或 `run`，讓呼叫意圖更清楚，也和此專案偏完整語意的命名風格一致。",
|
|
||||||
"suggestedCode": "function withFetchStub(handler, callback) {",
|
|
||||||
"id": "F006",
|
|
||||||
"verdicts": {
|
|
||||||
"Paladin": {
|
|
||||||
"exclude": false,
|
|
||||||
"reason": "保留。未見已知排除事項或歷史 finding 涵蓋 test/gitea.test.js 中 fn 測試輔助參數命名問題；屬新的命名可讀性指控。"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
],
|
|
||||||
"excluded": [
|
"excluded": [
|
||||||
{
|
{
|
||||||
"reviewer": "Assassin",
|
"reviewer": "Assassin",
|
||||||
|
|||||||
Reference in New Issue
Block a user