old-actions/code-review
2
0
Fork 0
Code Pull Requests Actions Releases 2 Activity

feat(ai-review 對話收斂): 讀 PR review 留言判斷解決狀態並收斂 findings #42

Merged
jiantw83 merged 68 commits from ai-review-resolve/20260623-110950 into develop 2026-06-23 08:29:38 +00:00
Conversation 28 Commits 68 Files Changed 29
+31 -13
1 changed files with 31 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 5bee498e4b - Show all commits
app/resolve.js
+24 -6
View File
10
 @@ -49,6 +49,7 @@ export function groupConversations(comments) {
const groups = new Map();
for (const c of comments || []) {
const filePath = typeof c?.path === 'string' ? c.path : '';
if (!filePath) continue; // 無檔案路徑的留言無法定位，跳過以免併入共用群組
admin marked this conversation as resolved Outdated
admin commented 2026-06-23 03:33:16 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🟡 警告
審查員：Assassin
問題：在 parseBotReviewComment 函式中，從 Gitea comment 內文解析出的 problem 和 suggestion 欄位，若包含惡意 HTML 或 JavaScript 程式碼，且這些內容在後續的處理或顯示中未經適當的輸出編碼，可能導致跨網站指令碼（XSS）攻擊。
建議：確保所有從外部來源解析出的字串（特別是 problem 和 suggestion）在任何將其渲染到網頁或其他使用者介面的地方，都必須經過嚴格的上下文相關輸出編碼（例如 HTML 實體編碼、JavaScript 字串編碼等），以防止 XSS 攻擊。

**嚴重等級**：🟡 警告 **審查員**：Assassin **問題**：在 `parseBotReviewComment` 函式中，從 Gitea comment 內文解析出的 `problem` 和 `suggestion` 欄位，若包含惡意 HTML 或 JavaScript 程式碼，且這些內容在後續的處理或顯示中未經適當的輸出編碼，可能導致跨網站指令碼（XSS）攻擊。 **建議**：確保所有從外部來源解析出的字串（特別是 `problem` 和 `suggestion`）在任何將其渲染到網頁或其他使用者介面的地方，都必須經過嚴格的上下文相關輸出編碼（例如 HTML 實體編碼、JavaScript 字串編碼等），以防止 XSS 攻擊。
admin commented 2026-06-23 03:38:19 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🟡 警告
審查員：Assassin
問題：在 parseBotReviewComment 函式中，從 Gitea comment 內文解析出的 problem 和 suggestion 欄位若包含惡意內容且未經適當輸出編碼，可能導致 XSS 攻擊。
建議：確保所有從外部來源解析出的字串在渲染到任何介面時，都必須經過嚴格的上下文相關輸出編碼（例如 HTML 實體編碼），以防止 XSS 攻擊。

**嚴重等級**：🟡 警告 **審查員**：Assassin **問題**：在 `parseBotReviewComment` 函式中，從 Gitea comment 內文解析出的 `problem` 和 `suggestion` 欄位若包含惡意內容且未經適當輸出編碼，可能導致 XSS 攻擊。 **建議**：確保所有從外部來源解析出的字串在渲染到任何介面時，都必須經過嚴格的上下文相關輸出編碼（例如 HTML 實體編碼），以防止 XSS 攻擊。
const lineNum = Number(c?.position) || Number(c?.original_position) || 0;
const key = `${filePath}|${lineNum}`;
if (!groups.has(key)) {
14
 @@ -129,10 +130,17 @@ export async function reconcileConversations(deps = {}) {
line(`對話收斂: 對話總數=${conversations.length} 已解決/不可處理=${alreadyResolved} 待判斷=${open.length}`);
if (open.length === 0) return { ...EMPTY };
admin marked this conversation as resolved Outdated
admin commented 2026-06-23 03:16:44 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🔴 嚴重
審查員：Maya
問題：函式 reconcileConversations 在取得單一檔案內容 (getFileContent) 失敗時，會中斷整個對話收斂流程。這會導致即使只有一個檔案出錯，整個 PR 的收斂都無法完成。
建議：請修改 reconcileConversations，在 fileCache.set(filePath, await getFileContent(filePath)) 的迴圈中，為 getFileContent 加上 try-catch 區塊。當單一檔案取得失敗時，應記錄警告並將該檔案的內容視為空字串，而不是中斷整個流程，以確保其他檔案的處理不受影響。

**嚴重等級**：🔴 嚴重 **審查員**：Maya **問題**：函式 `reconcileConversations` 在取得單一檔案內容 (`getFileContent`) 失敗時，會中斷整個對話收斂流程。這會導致即使只有一個檔案出錯，整個 PR 的收斂都無法完成。 **建議**：請修改 `reconcileConversations`，在 `fileCache.set(filePath, await getFileContent(filePath))` 的迴圈中，為 `getFileContent` 加上 `try-catch` 區塊。當單一檔案取得失敗時，應記錄警告並將該檔案的內容視為空字串，而不是中斷整個流程，以確保其他檔案的處理不受影響。
// 並行取得各檔案最新內容；單一檔案失敗時視為空字串，不中斷整體流程
const fileCache = new Map();
for (const filePath of [...new Set(open.map(c => c.path).filter(Boolean))]) {
const filePaths = [...new Set(open.map(c => c.path).filter(Boolean))];
await Promise.all(filePaths.map(async (filePath) => {
try {
fileCache.set(filePath, await getFileContent(filePath));
} catch (e) {
warn(`取得檔案內容失敗（視為空）: ${filePath} error=${e.message}`);
fileCache.set(filePath, '');
}
admin marked this conversation as resolved Outdated
admin commented 2026-06-23 05:17:01 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🔴 嚴重
審查員：Maya
問題：reconcileConversations 核心流程中，對於 getFileContent 失敗或內容為空的處理邏輯，直接降級為空字串並視為未解決，但若檔案內容實際上非空且未解決，這可能導致判斷偏差。
建議：補測試案例，模擬 getFileContent 拋出錯誤時，reconcileConversations 是否正確地將對話保留為未解決，且後續統計數字（carriedFindings）是否正確。

**嚴重等級**：🔴 嚴重 **審查員**：Maya **問題**：`reconcileConversations` 核心流程中，對於 `getFileContent` 失敗或內容為空的處理邏輯，直接降級為空字串並視為未解決，但若檔案內容實際上非空且未解決，這可能導致判斷偏差。 **建議**：補測試案例，模擬 `getFileContent` 拋出錯誤時，`reconcileConversations` 是否正確地將對話保留為未解決，且後續統計數字（`carriedFindings`）是否正確。
}));
admin marked this conversation as resolved Outdated
admin commented 2026-06-23 03:16:44 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🟡 警告
審查員：Maya
問題：函式 reconcileConversations 缺少對 judge 拋出錯誤情境的明確測試。雖然程式碼有 try-catch 處理，但應有專門的測試案例來驗證此失敗路徑的行為。
建議：請新增測試案例，模擬 judge 函式拋出錯誤時，確認 reconcileConversations 能正確捕獲錯誤，記錄警告，並將所有待判斷的對話都視為未解決（即 verdicts 應全部為 resolved: false）。

**嚴重等級**：🟡 警告 **審查員**：Maya **問題**：函式 `reconcileConversations` 缺少對 `judge` 拋出錯誤情境的明確測試。雖然程式碼有 `try-catch` 處理，但應有專門的測試案例來驗證此失敗路徑的行為。 **建議**：請新增測試案例，模擬 `judge` 函式拋出錯誤時，確認 `reconcileConversations` 能正確捕獲錯誤，記錄警告，並將所有待判斷的對話都視為未解決（即 `verdicts` 應全部為 `resolved: false`）。
admin commented 2026-06-23 03:38:19 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🟡 警告
審查員：Maya
問題：缺少關鍵邊界條件與異常路徑的測試案例。包含 judge 拋出錯誤、chatFn 解析異常、levelRaw 或 suggestion 空值、getFileContent 失敗以及混合正確/錯誤的判斷數據等場景。
建議：請在 app/resolve.test.js 中新增這些邊界條件的測試案例，確保系統在面對 AI 異常輸出、API 失敗、或輸入欄位缺失時，仍能穩健處理並符合預期行為。

**嚴重等級**：🟡 警告 **審查員**：Maya **問題**：缺少關鍵邊界條件與異常路徑的測試案例。包含 `judge` 拋出錯誤、`chatFn` 解析異常、`levelRaw` 或 `suggestion` 空值、`getFileContent` 失敗以及混合正確/錯誤的判斷數據等場景。 **建議**：請在 `app/resolve.test.js` 中新增這些邊界條件的測試案例，確保系統在面對 AI 異常輸出、API 失敗、或輸入欄位缺失時，仍能穩健處理並符合預期行為。
admin commented 2026-06-23 04:59:43 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🟡 警告
審查員：Maya
問題：函式 reconcileConversations 缺少對 judge 拋出錯誤情境的明確測試。雖然程式碼有 try-catch 處理，但應有專門的測試案例來驗證此失敗路徑的行為。
建議：請新增測試案例，模擬 judge 函式拋出錯誤時，確認 reconcileConversations 能正確捕獲錯誤，記錄警告，並將所有待判斷的對話都視為未解決（即 verdicts 應全部為 resolved: false）。

**嚴重等級**：🟡 警告 **審查員**：Maya **問題**：函式 `reconcileConversations` 缺少對 `judge` 拋出錯誤情境的明確測試。雖然程式碼有 `try-catch` 處理，但應有專門的測試案例來驗證此失敗路徑的行為。 **建議**：請新增測試案例，模擬 `judge` 函式拋出錯誤時，確認 `reconcileConversations` 能正確捕獲錯誤，記錄警告，並將所有待判斷的對話都視為未解決（即 `verdicts` 應全部為 `resolved: false`）。
admin commented 2026-06-23 05:17:01 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🟡 警告
審查員：Maya
問題：函式 reconcileConversations 缺少對 judge 拋出錯誤情境的明確測試。雖然程式碼有 try-catch 處理，但應有專門的測試案例來驗證此失敗路徑的行為。
建議：請新增測試案例，模擬 judge 函式拋出錯誤時，確認 reconcileConversations 能正確捕獲錯誤，記錄警告，並將所有待判斷的對話都視為未解決（即 verdicts 應全部為 resolved: false）。

**嚴重等級**：🟡 警告 **審查員**：Maya **問題**：函式 `reconcileConversations` 缺少對 `judge` 拋出錯誤情境的明確測試。雖然程式碼有 `try-catch` 處理，但應有專門的測試案例來驗證此失敗路徑的行為。 **建議**：請新增測試案例，模擬 `judge` 函式拋出錯誤時，確認 `reconcileConversations` 能正確捕獲錯誤，記錄警告，並將所有待判斷的對話都視為未解決（即 `verdicts` 應全部為 `resolved: false`）。
const items = open.map((c, idx) => ({
admin marked this conversation as resolved Outdated
admin commented 2026-06-23 03:33:16 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🔴 嚴重
審查員：Assassin
問題：在 judgeConversationsResolved 函式中，thread（來自 Gitea comment 內容）和 code（來自 PR 檔案內容）被直接拼接進傳給 LLM 的 payload 中。如果攻擊者能夠控制這些內容，他們可以透過注入惡意指令來劫持 LLM 的行為，例如使其始終將特定問題判斷為已解決，或嘗試從 LLM 獲取敏感資訊（提示詞注入）。
建議：對所有傳遞給 LLM 的外部輸入（如 thread 和 code）進行嚴格的淨化和隔離。考慮使用結構化輸入而非直接拼接字串，並在 LLM 提示詞中明確指示其忽略任何試圖改變其行為的指令。對於敏感操作，應建立多層驗證機制，不單純依賴 LLM 的判斷。

**嚴重等級**：🔴 嚴重 **審查員**：Assassin **問題**：在 `judgeConversationsResolved` 函式中，`thread`（來自 Gitea comment 內容）和 `code`（來自 PR 檔案內容）被直接拼接進傳給 LLM 的 `payload` 中。如果攻擊者能夠控制這些內容，他們可以透過注入惡意指令來劫持 LLM 的行為，例如使其始終將特定問題判斷為已解決，或嘗試從 LLM 獲取敏感資訊（提示詞注入）。 **建議**：對所有傳遞給 LLM 的外部輸入（如 `thread` 和 `code`）進行嚴格的淨化和隔離。考慮使用結構化輸入而非直接拼接字串，並在 LLM 提示詞中明確指示其忽略任何試圖改變其行為的指令。對於敏感操作，應建立多層驗證機制，不單純依賴 LLM 的判斷。
admin commented 2026-06-23 03:38:19 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🔴 嚴重
審查員：Assassin
問題：LLM 提示詞注入風險：在 judgeConversationsResolved 函式中，外部來源的 thread 和 code 被直接拼接進傳給 LLM 的 payload 中，攻擊者可能注入惡意指令來劫持 LLM 行為。
建議：對所有傳遞給 LLM 的外部輸入進行嚴格的淨化和隔離。使用結構化輸入而非直接拼接字串，並在提示詞中明確指示 AI 忽略任何試圖下達指令的內容，僅對邏輯進行判斷。對於敏感操作，應建立多層驗證機制。

**嚴重等級**：🔴 嚴重 **審查員**：Assassin **問題**：LLM 提示詞注入風險：在 `judgeConversationsResolved` 函式中，外部來源的 `thread` 和 `code` 被直接拼接進傳給 LLM 的 `payload` 中，攻擊者可能注入惡意指令來劫持 LLM 行為。 **建議**：對所有傳遞給 LLM 的外部輸入進行嚴格的淨化和隔離。使用結構化輸入而非直接拼接字串，並在提示詞中明確指示 AI 忽略任何試圖下達指令的內容，僅對邏輯進行判斷。對於敏感操作，應建立多層驗證機制。
idx,
1
 @@ -153,19 +161,29 @@ export async function reconcileConversations(deps = {}) {
const resolvedFindings = [];
const carriedFindings = [];
// 並行 resolve 所有 AI 判定已解決的對話（allSettled：個別失敗不中斷其他）
const resolveTargets = open
.map((c, i) => ({ c, i }))
.filter(({ i }) => resolvedSet.has(i));
const settled = await Promise.allSettled(
resolveTargets.map(({ c }) => resolveComment(c.commentIds[0])),
);
const resolveOutcome = new Map();
resolveTargets.forEach(({ i }, j) => resolveOutcome.set(i, settled[j]));
admin marked this conversation as resolved Outdated
admin commented 2026-06-23 03:16:44 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🔴 嚴重
審查員：Rogue
問題：又來了！reconcileConversations 函式在迴圈裡對每個需要解決的對話依序呼叫 resolveComment。這又是一個 N+1 查詢問題，如果有很多對話需要解決，會導致 N_open 次遠端 API 呼叫依序執行，效率極差。
建議：改用 Promise.allSettled 來並行發送所有 resolveComment 的請求。這樣可以大幅減少等待時間，讓對話的解決幾乎同時完成，即使部分失敗也不會中斷其他請求。

**嚴重等級**：🔴 嚴重 **審查員**：Rogue **問題**：又來了！`reconcileConversations` 函式在迴圈裡對每個需要解決的對話依序呼叫 `resolveComment`。這又是一個 N+1 查詢問題，如果有很多對話需要解決，會導致 `N_open` 次遠端 API 呼叫依序執行，效率極差。 **建議**：改用 `Promise.allSettled` 來並行發送所有 `resolveComment` 的請求。這樣可以大幅減少等待時間，讓對話的解決幾乎同時完成，即使部分失敗也不會中斷其他請求。
admin commented 2026-06-23 03:38:19 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🔵 建議
審查員：Rogue
問題：Promise.allSettled 的結果處理邏輯過於冗長，產生不必要的中間變數。
建議：優化處理邏輯，直接在迴圈內處理或使用更緊湊的寫法。

**嚴重等級**：🔵 建議 **審查員**：Rogue **問題**：`Promise.allSettled` 的結果處理邏輯過於冗長，產生不必要的中間變數。 **建議**：優化處理邏輯，直接在迴圈內處理或使用更緊湊的寫法。
let resolvedCount = 0;
for (let i = 0; i < open.length; i++) {
const c = open[i];
if (resolvedSet.has(i)) {
try {
await resolveComment(c.commentIds[0]);
const outcome = resolveOutcome.get(i);
if (outcome?.status === 'fulfilled') {
resolvedCount += 1;
admin marked this conversation as resolved Outdated
admin commented 2026-06-23 03:33:16 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🔴 嚴重
審查員：Assassin
問題：在 reconcileConversations 函式中，從外部 Gitea comment 取得的 c.path（檔案路徑）未經額外驗證或淨化，直接傳遞給了 getFileContent（即 getFileContentAtRef）。由於 getFileContentAtRef 存在路徑穿越漏洞，攻擊者可以透過在 PR 中建立惡意檔案名稱，並在該檔案上留言，來觸發路徑穿越，讀取伺服器上的任意檔案。
建議：在將 c.path 傳遞給 getFileContent 之前，必須對其進行嚴格的白名單驗證，確保它只包含預期的檔案名稱字元，且不包含任何路徑穿越序列（例如 .. 或 /）。或者，確保 getFileContentAtRef 的路徑處理是絕對安全的，不允許任何形式的路徑穿越。

**嚴重等級**：🔴 嚴重 **審查員**：Assassin **問題**：在 `reconcileConversations` 函式中，從外部 Gitea comment 取得的 `c.path`（檔案路徑）未經額外驗證或淨化，直接傳遞給了 `getFileContent`（即 `getFileContentAtRef`）。由於 `getFileContentAtRef` 存在路徑穿越漏洞，攻擊者可以透過在 PR 中建立惡意檔案名稱，並在該檔案上留言，來觸發路徑穿越，讀取伺服器上的任意檔案。 **建議**：在將 `c.path` 傳遞給 `getFileContent` 之前，必須對其進行嚴格的白名單驗證，確保它只包含預期的檔案名稱字元，且不包含任何路徑穿越序列（例如 `..` 或 `/`）。或者，確保 `getFileContentAtRef` 的路徑處理是絕對安全的，不允許任何形式的路徑穿越。
admin commented 2026-06-23 03:38:19 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🔴 嚴重
審查員：Assassin
問題：在 reconcileConversations 函式中，從外部 Gitea comment 取得的 c.path（檔案路徑）未經額外驗證或淨化，直接傳遞給了 getFileContent（即 getFileContentAtRef）。由於 getFileContentAtRef 存在路徑穿越漏洞，攻擊者可以透過在 PR 中建立惡意檔案名稱，並在該檔案上留言，來觸發路徑穿越，讀取伺服器上的任意檔案。
建議：在將 c.path 傳遞給 getFileContent 之前，必須對其進行嚴格的白名單驗證，確保它只包含預期的檔案名稱字元，且不包含任何路徑穿越序列（例如 .. 或 /）。或者，確保 getFileContentAtRef 的路徑處理是絕對安全的，不允許任何形式的路徑穿越。

**嚴重等級**：🔴 嚴重 **審查員**：Assassin **問題**：在 `reconcileConversations` 函式中，從外部 Gitea comment 取得的 `c.path`（檔案路徑）未經額外驗證或淨化，直接傳遞給了 `getFileContent`（即 `getFileContentAtRef`）。由於 `getFileContentAtRef` 存在路徑穿越漏洞，攻擊者可以透過在 PR 中建立惡意檔案名稱，並在該檔案上留言，來觸發路徑穿越，讀取伺服器上的任意檔案。 **建議**：在將 `c.path` 傳遞給 `getFileContent` 之前，必須對其進行嚴格的白名單驗證，確保它只包含預期的檔案名稱字元，且不包含任何路徑穿越序列（例如 `..` 或 `/`）。或者，確保 `getFileContentAtRef` 的路徑處理是絕對安全的，不允許任何形式的路徑穿越。
if (c.botFinding) resolvedFindings.push({ ...c.botFinding, is_new: false });
ok(`對話已解決並 resolve: ${c.path}:${c.line}`);
continue;
} catch (e) {
warn(`resolve 對話失敗（保留為未解決）: ${c.path}:${c.line} error=${e.message}`);
}
if (outcome?.status === 'rejected') {
warn(`resolve 對話失敗（保留為未解決）: ${c.path}:${c.line} error=${outcome.reason?.message}`);
}
admin marked this conversation as resolved Outdated
admin commented 2026-06-23 03:38:19 +00:00
Outdated
Review
Copy Link
Copy Source

嚴重等級：🟡 警告
審查員：Mage
問題：在 reconcileConversations 函式中，並行（Promise.all）呼叫 resolveComment，即使個別呼叫失敗，也僅在 settled 中記錄為 rejected 並印出 warn。然而，若 resolveComment 失敗是因為 Authorization token 過期或權限不足，後續所有的 resolve 呼叫都會失敗，此時程式碼沒有對這些特定的錯誤進行分類處理。
建議：應判斷 outcome.reason 的錯誤類型。若是連線/權限相關的嚴重錯誤，應立即停止後續的 resolve 嘗試，避免在已知無法成功的情況下發出無效請求。

**嚴重等級**：🟡 警告 **審查員**：Mage **問題**：在 `reconcileConversations` 函式中，並行（`Promise.all`）呼叫 `resolveComment`，即使個別呼叫失敗，也僅在 `settled` 中記錄為 `rejected` 並印出 `warn`。然而，若 `resolveComment` 失敗是因為 `Authorization` token 過期或權限不足，後續所有的 `resolve` 呼叫都會失敗，此時程式碼沒有對這些特定的錯誤進行分類處理。 **建議**：應判斷 `outcome.reason` 的錯誤類型。若是連線/權限相關的嚴重錯誤，應立即停止後續的 `resolve` 嘗試，避免在已知無法成功的情況下發出無效請求。
pushCarried(carriedFindings, c);
}
3