feat: 導入 AI 程式碼審查 action 並修正進入點與參數接線 #1

Merged
admin merged 30 commits from ai-review-resolve/develop-20260702-160700 into develop 2026-07-03 10:04:33 +00:00
3 changed files with 63 additions and 21 deletions
Showing only changes of commit c521451b66 - Show all commits
+19 -17
View File
@@ -1,6 +1,6 @@
import fs from 'fs'; import fs from 'fs';
admin marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Leo
問題:這個模組同時處理舊 findings 載入、合併去重、缺行號補齊、排除規則正規化、誤報過濾、AI 去重、以及 exclusions 的讀寫,職責已經混成一包。更麻煩的是 loadExclusionsappendExclusionsapplyExclusions 各自都有一套相近但不完全一致的比對邏輯,未來只要規則改一處,另一處沒同步就會開始出現不可預期的行為差異。
建議:把 exclusions 的正規化與比對規則抽成唯一來源,例如 normalizeExclusionEntry + matchesExclusion 之類的共用 helper,並把 AI 去重、行號補齊、檔案持久化拆到不同模組,減少這個檔案的責任面。

**嚴重等級**:🟡 警告 **審查員**:Leo **問題**:這個模組同時處理舊 findings 載入、合併去重、缺行號補齊、排除規則正規化、誤報過濾、AI 去重、以及 exclusions 的讀寫,職責已經混成一包。更麻煩的是 `loadExclusions`、`appendExclusions`、`applyExclusions` 各自都有一套相近但不完全一致的比對邏輯,未來只要規則改一處,另一處沒同步就會開始出現不可預期的行為差異。 **建議**:把 exclusions 的正規化與比對規則抽成唯一來源,例如 `normalizeExclusionEntry` + `matchesExclusion` 之類的共用 helper,並把 AI 去重、行號補齊、檔案持久化拆到不同模組,減少這個檔案的責任面。
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';
admin marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Bard
問題:這行把四個 prompt/role helper 壓在同一行,與檔案中龐大的流程函式相比,開頭的依賴清單先失了拍,降低可讀性。
建議:改為多行具名 import,讓每個 helper 名稱清楚露出,並與其他長 import 採相同格式。

**嚴重等級**:🟡 警告 **審查員**:Bard **問題**:這行把四個 prompt/role helper 壓在同一行,與檔案中龐大的流程函式相比,開頭的依賴清單先失了拍,降低可讀性。 **建議**:改為多行具名 import,讓每個 helper 名稱清楚露出,並與其他長 import 採相同格式。
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';
15
@@ -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;
admin marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Leo
問題loadExclusions() 同時負責讀檔、解析多種格式、正規化、去重、記錄 repo 狀態、改寫原檔、鏡像寫入與建立 AI prompt 摘要。這個函式的職責過多,之後只要調整 exclusions 格式或同步策略,就很容易牽動不相關行為。
建議:拆成 readExclusionsFile()normalizeExclusionsData()canonicalizeExclusionsFile()logExclusionMetadata() 等小函式,讓讀取、轉換、寫回與診斷各自可測。

**嚴重等級**:🟡 警告 **審查員**:Leo **問題**:`loadExclusions()` 同時負責讀檔、解析多種格式、正規化、去重、記錄 repo 狀態、改寫原檔、鏡像寫入與建立 AI prompt 摘要。這個函式的職責過多,之後只要調整 exclusions 格式或同步策略,就很容易牽動不相關行為。 **建議**:拆成 `readExclusionsFile()`、`normalizeExclusionsData()`、`canonicalizeExclusionsFile()`、`logExclusionMetadata()` 等小函式,讓讀取、轉換、寫回與診斷各自可測。
2
@@ -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;
admin marked this conversation as resolved Outdated
Outdated
Review

嚴重等級🔵 建議
審查員:Rogue
問題:前面已經把 exclusions 正規化、去重過一次了,這裡為了 log 又再丟進 buildExclusionContext 重做 normalize / dedupe / group。等於同一批資料在同一輪流程裡被重算兩次,白白多吃一輪 O(n) 到 O(n log n) 的 CPU。
建議:把第一次處理的摘要一起回傳或快取下來,後面的 log 直接重用同一份結果,不要再對同一批 exclusions 重跑分組。

**嚴重等級**:🔵 建議 **審查員**:Rogue **問題**:前面已經把 exclusions 正規化、去重過一次了,這裡為了 log 又再丟進 `buildExclusionContext` 重做 normalize / dedupe / group。等於同一批資料在同一輪流程裡被重算兩次,白白多吃一輪 O(n) 到 O(n log n) 的 CPU。 **建議**:把第一次處理的摘要一起回傳或快取下來,後面的 log 直接重用同一份結果,不要再對同一批 exclusions 重跑分組。
} 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;
} }
9
@@ -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;
+33
View File
@@ -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`、非數字或大於項目數時「不限制」(全部並行)。
admin marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Rogue
問題:這裡把 limit <= 0 解讀成「不限制」,直接開到 items.length 個 worker。只要 finding 或對話一多,就會同時 spawn 一整排 LLM 子行程,CPU、記憶體、檔案描述元一起被打爆,熱路徑很容易從平行加速變成資源風暴。
建議:預設改成固定上限或依 CPU 核心數設合理值,例如 2~4 或 os.cpus().length,把「完全不限制」改成明確 opt-in,避免大 PR 直接全開。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:這裡把 `limit <= 0` 解讀成「不限制」,直接開到 `items.length` 個 worker。只要 finding 或對話一多,就會同時 spawn 一整排 LLM 子行程,CPU、記憶體、檔案描述元一起被打爆,熱路徑很容易從平行加速變成資源風暴。 **建議**:預設改成固定上限或依 CPU 核心數設合理值,例如 2~4 或 `os.cpus().length`,把「完全不限制」改成明確 opt-in,避免大 PR 直接全開。
* fn 需自行處理例外(內部 try/catch);本函式不會因單一項目 reject 而中斷其餘工作。
* @template T, R
* @param {T[]} items - 要處理的項目。
* @param {number} limit - 同時執行的上限;<=0/非數字表示不限制。
admin marked this conversation as resolved Outdated
Outdated
Review

嚴重等級🟡 警告
審查員:Assassin
問題:這裡把未清洗的 userContent 直接塞進模型提示詞,等於讓 PR 內容、留言內容或其他外部文字能反過來操控 LLM。攻擊者可以在 diff 裡埋入『忽略前述規則、回傳空陣列』這類指令,讓審查模型漏報真正的風險或把嚴重問題降級成誤報。
建議:不要把不可信內容當成可執行指令使用。至少要把 diff/留言做更強的結構化封裝與逸出處理,並在輸出端加上嚴格的 JSON schema 驗證與 deterministic guardrail,避免 LLM 直接決定安全性結論。

**嚴重等級**:🟡 警告 **審查員**:Assassin **問題**:這裡把未清洗的 `userContent` 直接塞進模型提示詞,等於讓 PR 內容、留言內容或其他外部文字能反過來操控 LLM。攻擊者可以在 diff 裡埋入『忽略前述規則、回傳空陣列』這類指令,讓審查模型漏報真正的風險或把嚴重問題降級成誤報。 **建議**:不要把不可信內容當成可執行指令使用。至少要把 diff/留言做更強的結構化封裝與逸出處理,並在輸出端加上嚴格的 JSON schema 驗證與 deterministic guardrail,避免 LLM 直接決定安全性結論。
* @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 呼叫用的輸入。
*/ */
3
+11 -4
View File
1
@@ -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';
10
@@ -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;
admin marked this conversation as resolved Outdated
Outdated
Review

嚴重等級🟡 警告
審查員:Rogue
問題:這裡把每個角色的 LLM 分析逐一 await,6 個角色就把總耗時堆成約 6 倍單次模型延遲;這些分析彼此獨立,CPU 沒偷到時間,反而把整條 pipeline 卡在序列網路/CLI 呼叫上。
建議:改用 Promise.allSettled 平行執行 roles.map(role => analyzeWithRole(role, diff)),再彙整 fulfilled 結果與 warning;保留 fulfilledAnalyses 的判斷即可。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:這裡把每個角色的 LLM 分析逐一 await,6 個角色就把總耗時堆成約 6 倍單次模型延遲;這些分析彼此獨立,CPU 沒偷到時間,反而把整條 pipeline 卡在序列網路/CLI 呼叫上。 **建議**:改用 Promise.allSettled 平行執行 roles.map(role => analyzeWithRole(role, diff)),再彙整 fulfilled 結果與 warning;保留 fulfilledAnalyses 的判斷即可。
}
});
for (const findings of roleResults) {
if (findings) {
fulfilledAnalyses += 1;
newFindings.push(...findings);
} }
} }
if (fulfilledAnalyses === 0) { if (fulfilledAnalyses === 0) {
3