ai-review-resolve/develop-20260702-160700
develop
導入 AI 程式碼審查 Gitea Node action 的完整實作,並修正原範本殘留的結構問題,讓 action 可實際運作。
feat
src/
fix
action.yml
main
src/index.js
src/main.js
config.js
GITHUB_EVENT_PATH
with:
INPUT_*
with: token
chore
model
node-template
package.json
src/package.json
node_modules
npm install
token
NODE_TLS_REJECT_UNAUTHORIZED=0
.gitea/ai-review/findings.json
node --test
🔍 服務:codex 模型:gpt-5.4-mini
🔍 服務:codex 模型:gpt-5.5
本次審查(codex / gpt-5.5,共 40 次呼叫)
剩餘可用
剩餘可用:無法計算百分比(未支援 codex 額度查詢)
@@ -0,0 +8,4 @@
const LEVEL_LABEL = { critical: '嚴重', warning: '警告', info: '建議' };
const LEVEL_ORDER = ['critical', 'warning', 'info'];
// 預先把等級對應到排序索引,bySeverity 排序時直接 O(1) 取值,省去每次比較的 includes + indexOf 掃描。
const LEVEL_RANK = new Map(LEVEL_ORDER.map((level, index) => [level, index]));
嚴重等級:🟡 警告 審查員:Bard 問題:大量私有輔助函式都配上篇幅很長的 JSDoc,許多內容只是重述程式碼表面行為,註解的聲量蓋過了旋律本身。 建議:保留公開 API 或非直覺決策的文件即可;私有小函式改用簡短註解,或讓函式命名本身說明用途。
@@ -0,0 +22,4 @@
function findingRow(f) {
if (!f) return '';
return `| ${LEVEL_EMOJI[f.level] || ''} ${LEVEL_LABEL[f.level] || f.level} | ${f.role} | ${f.location} | ${f.suggestion} |`;
}
嚴重等級:🔵 建議 審查員:Maya 問題:Markdown 表格列直接嵌入 role、location、suggestion,但測試沒有覆蓋 suggestion 含 |、換行或 Markdown 特殊字元時的輸出。這不是要求現在一定要改格式,而是目前缺少案例確認表格在真實 LLM 輸出下不會被破壞。 建議:補一個 comment body 格式測試,輸入 suggestion 含 pipe、換行與粗體符號,斷言輸出的 Markdown 結構符合預期;若目前行為會破表,應先定義轉義或替換規則再測。
|
@@ -0,0 +123,4 @@
* 過濾出新問題(is_new 不等於 false 者)。
*
* @param {Array<{ is_new?: boolean }>} findings 審查問題陣列。
* @returns {Array<object>} 新問題子集合。
嚴重等級:🔵 建議 審查員:Rogue 問題:countBy 用 filter(predicate).length 只為了計數卻配置中間陣列;formatFindingsStats/formatFindingsStatsLine 每列又重複掃多次,雖然 findings 通常不大,但這是在白白丟記憶體與掃描週期。 建議:改成單趟 reduce 統計 new/old × level 的計數表,或讓 countBy 用 for-of 累加數字、不建立 filter 結果陣列。
@@ -0,0 +1,127 @@
import https from 'https';
import fs from 'fs';
import { execFileSync } from 'child_process';
嚴重等級:🔵 建議 審查員:Bard 問題:註解說「需要內部服務相容時才使用 getInsecureHttpsAgent()」,下一行卻在模組載入時全域設定 TLS 環境變數,文件與程式碼唱了不同旋律。 建議:讓註解忠實描述目前行為,或把全域設定移到明確命名的初始化函式;至少避免文件暗示這是選擇性使用。
@@ -0,0 +4,4 @@
// 本 action 會連接自架 Gitea / OpenCode,部署環境可能使用內部 CA 或自簽憑證。
// 對外部服務請優先使用預設 TLS 驗證;需要內部服務相容時才使用 getInsecureHttpsAgent()。
process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';
嚴重等級:🔴 嚴重 審查員:Assassin 問題:這裡把 NODE_TLS_REJECT_UNAUTHORIZED 全域設為 0,等於讓整個 Node 程序放棄 TLS 憑證驗證。攻擊者只要能站到 runner 與 Gitea/LLM/任何 HTTPS API 之間,就能用偽造憑證攔截或竄改 diff、review 結果、token 驗證流程,甚至偷走 Authorization header。 建議:移除全域停用 TLS 的設定。若內部自簽 CA 是必要情境,請改用可設定的 CA bundle(例如 NODE_EXTRA_CA_CERTS)或僅對明確允許的內部 host 使用專用 agent,且預設必須啟用憑證驗證。
NODE_TLS_REJECT_UNAUTHORIZED
0
NODE_EXTRA_CA_CERTS
@@ -0,0 +50,4 @@
* 停用憑證驗證有中間人攻擊風險,僅限受信任的內部環境使用。
* @returns {import('https').Agent} 已關閉憑證驗證的 HTTPS Agent 單例。
*/
let _insecureHttpsAgent = null;
嚴重等級:🔴 嚴重 審查員:Assassin 問題:這個 helper 直接建立 rejectUnauthorized: false 的 HTTPS agent,後續 Gitea API 與 preflight 都會用它。攻擊者若能進行中間人攻擊,就能假冒 Gitea 回傳惡意 diff、偽造 comment/review API 回應,或攔截寫入用 token。 建議:不要提供預設不驗證憑證的 agent。改成預設安全驗證;若真的要支援自簽憑證,請要求使用者明確提供信任的 CA 憑證,或以白名單 host 加上明確 opt-in 的設定限制風險。
rejectUnauthorized: false
@@ -0,0 +1,581 @@
import path from 'path';
import { chatJSON } from './llm.js';
import { buildAnalysisPrompt, loadRole, buildVerdictPrompt, buildLocateLinePrompt } from './roles.js';
嚴重等級:🟡 警告 審查員:Bard 問題:這行把四個 prompt/role helper 壓在同一行,與檔案中龐大的流程函式相比,開頭的依賴清單先失了拍,降低可讀性。 建議:改為多行具名 import,讓每個 helper 名稱清楚露出,並與其他長 import 採相同格式。
@@ -0,0 +11,4 @@
* 用單一角色分析 diff,回傳 findings 陣列。
* role 欄位一律以角色定義的 name 為準,避免 LLM 自行填入不一致的名稱。
export async function analyzeWithRole(role, diff) {
嚴重等級:🟡 警告 審查員:Assassin 問題:這裡把未信任的 Git diff 直接送進 LLM。攻擊者可以在新增程式碼或註解中塞入提示詞注入內容,例如要求模型忽略安全問題、回傳空陣列或偽造低風險 findings,藉此讓自動安全審查失明。 建議:在分析 prompt 中明確標示 diff 是不可信資料,要求模型忽略 diff 內任何指令;同時加入結構化封裝、輸出 schema 驗證與必要的規則式安全檢查,避免完全依賴可被 prompt injection 操控的 LLM 判斷。
@@ -0,0 +101,4 @@
return typeof value === 'string' ? value.trim() : '';
/**
嚴重等級:🔵 建議 審查員:Leo 問題:文字正規化邏輯分散在 normalizeText()、toKeyText(),而 src/resolve.js 也有另一套 normalizeKey()。這些函式對大小寫、標點與空白的處理不完全一致,長期會讓 finding 去重、排除與對話收斂出現難追的差異。 建議:建立單一 normalization 模組,明確定義 normalizeForDisplayMatch、normalizeForSignature 等用途,再讓 findings、resolve、exclusions 共用,並補上跨模組測試鎖定語意。
normalizeText()
toKeyText()
src/resolve.js
normalizeKey()
normalizeForDisplayMatch
normalizeForSignature
@@ -0,0 +135,4 @@
return cleanText(value)
.normalize('NFKC')
.replace(/[\p{P}\p{S}\s]+/gu, '')
.trim();
嚴重等級:🔵 建議 審查員:Bard 問題:註解中留下「不確定」這種未定案語氣,像樂譜上的猶豫記號;讀者無法判斷這是刻意設計、待辦事項,還是審查遺留。 建議:若是刻意差異,改寫成明確理由;若待確認,改成可追蹤的 TODO 並標明決策者或議題。
@@ -0,0 +333,4 @@
* AI 呼叫失敗時的統一降級處理
function fallback(label, findings, e) {
嚴重等級:🟡 警告 審查員:Mage 問題:AI 去重回傳只要是非空陣列就被接受,沒有檢查是否比原始 findings 更多。最小重現:原本 3 筆 findings,LLM 異常回傳 20 筆或加入不存在的 location,這裡會直接採用並進入發布與失敗判定,導致憑空產生問題或讓 workflow 誤失敗。 建議:去重結果應驗證每筆都能對應回原始 finding,且數量不得大於輸入;無法對應或數量異常時應降級回原始 findings,或只保留 origMap 命中的項目。
origMap
@@ -0,0 +378,4 @@
pending += 1;
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)}`;
let located = null;
嚴重等級:🟡 警告 審查員:Leo 問題:loadExclusions() 同時負責讀檔、解析多種格式、正規化、去重、記錄 repo 狀態、改寫原檔、鏡像寫入與建立 AI prompt 摘要。這個函式的職責過多,之後只要調整 exclusions 格式或同步策略,就很容易牽動不相關行為。 建議:拆成 readExclusionsFile()、normalizeExclusionsData()、canonicalizeExclusionsFile()、logExclusionMetadata() 等小函式,讓讀取、轉換、寫回與診斷各自可測。
loadExclusions()
readExclusionsFile()
normalizeExclusionsData()
canonicalizeExclusionsFile()
logExclusionMetadata()
@@ -0,0 +379,4 @@
for (let attempt = 1; attempt <= maxAttempts && located == null; attempt++) {
嚴重等級:🔵 建議 審查員:Rogue 問題:loadExclusions 前面已經 normalizeExclusionEntry + dedupeExclusions,這裡又呼叫 buildExclusionContext(exclusions) 重新 normalize、dedupe、group 一輪,只為了 log groups 數;排除規則多時會多跑一趟 O(e log e) 的整理成本。 建議:讓 buildExclusionContext 可接受已正規化/已去重的 exclusions,或直接在 loadExclusions 重用現有 exclusions 進行 group 統計,避免重複正規化與排序。
@@ -0,0 +413,4 @@
* 呼叫 LLM 進行語意去重,失敗時降級回傳原始 findings
export async function deduplicateWithAI(findings) {
嚴重等級:🟡 警告 審查員:Maya 問題:applyExclusions 的核心比對支援「只有文字、沒有路徑/角色」的排除規則,但現有測試多半靠相同檔案路徑命中,沒有驗證純文字排除、空文字排除、大小寫/標點差異等邊界。這條排除規則的最脆弱分支還沒被測到。 建議:補上 applyExclusions 的邊界測試:只有 suggestion/title 文字沒有 location 的 exclusion 應如何比對;空文字 exclusion 不應意外排除全部;標點、空白、大小寫正規化後相同的文字應依預期排除。
@@ -0,0 +427,4 @@
return result.map(r => origMap.get(`${r.location}|${String(r.suggestion).slice(0, 50)}`) ?? r);
throw new Error('AI 回傳空陣列');
} catch (e) {
嚴重等級:🟡 警告 審查員:Rogue 問題:applyExclusions 在 findings × exclusions 的巢狀比對裡,每遇到一條 exclusion 就重算同一個 finding 的 normalizeText;F 筆 finding、E 條 exclusion 會做最多 F×E 次正規化與正則替換,這是很明顯的 CPU 浪費。 建議:先把 findings 預處理成含 fPath、normalizedFindingText 的陣列,exclusions 也先補齊 normalizedExclusionText,再做比對;同一筆文字只正規化一次。
@@ -0,0 +463,4 @@
line(`讀取排除問題檔案: ${fullPath}`);
line(`來源分支狀態: branch=${branch} commit=${shortSha} commit_time=${commitTime}`);
line(`檔案資訊: bytes=${stat.size} mtime=${formatFileTime(stat.mtimeMs)} raw=${rawCount} normalized=${exclusions.length} path=${path.relative(workspace, fullPath) || fullPath}`);
if (sourceFormat !== 'array') {
嚴重等級:🟡 警告 審查員:Mage 問題:這裡優先使用 ex.textKey,但 textKey 是由 toKeyText() 產生的無分隔且未轉小寫文字,而 findingText 是 normalizeText() 產生的小寫、以空白分隔文字。最小重現:排除文字 Update tests 會變成 Updatetests,finding suggestion 會變成 update tests,兩邊互相 includes 都不成立,導致純文字排除規則失效。 建議:排除條目與 finding 應使用同一套正規化函式比對;例如改存並使用 normalizeText(ex.text || ex.suggestion || ex.title || ''),或讓 finding 也轉成同樣的 compact/lowercase key。
ex.textKey
textKey
Update tests
Updatetests
update tests
includes
normalizeText(ex.text || ex.suggestion || ex.title || '')
@@ -0,0 +468,4 @@
if (mirrorWorkspace && path.resolve(mirrorWorkspace) !== path.resolve(workspace)) {
const mirrorPath = path.join(mirrorWorkspace, EXCLUSIONS_PATH);
fs.mkdirSync(path.dirname(mirrorPath), { recursive: true });
writeCanonicalExclusions(mirrorPath, normalizedSource);
嚴重等級:🔴 嚴重 審查員:Mage 問題:當排除條目有 location 或 role 時,這裡直接把文字比對結果短路成 true。最小重現:exclusions.json 只有 { "location": "app/a.js:10", "original_finding": "誤報 A" },新的 finding 是 app/a.js:99 且 suggestion 完全不同,仍會因同檔案而被排除,導致真問題被靜默丟掉。 建議:不要用 exPath || ex.role ? true : textMatches 跳過文字比對;應至少要求位置精確匹配到同一行,或在同檔/同角色時仍必須通過 textMatches,例如 return locationMatches && roleMatches && textMatches,並明確定義 suggestion 空白時才是萬用規則。
{ "location": "app/a.js:10", "original_finding": "誤報 A" }
app/a.js:99
exPath || ex.role ? true : textMatches
textMatches
return locationMatches && roleMatches && textMatches
@@ -0,0 +223,4 @@
try {
await withAskpass(workspace, async credEnv => {
run(['config', 'user.email', 'ai-review[bot]@gitea'], repoDir);
嚴重等級:🔵 建議 審查員:Bard 問題:_sourceRoot` 的參數文件寫著「不確定,待確認」,讓公開函式簽名帶著未完成的旁白,破壞 API 文件的一致與可信度。 建議:若參數已不使用,移除它;若為相容性保留,明確寫成 deprecated/compatibility note,不要留下模糊語句。
@@ -0,0 +1,294 @@
import axios from 'axios';
import { GITEA_TOKEN, GITEA_COMMENT_TOKEN, GITEA_SERVER_URL, GITEA_REPOSITORY, PR_NUMBER, PR_HEAD_SHA, PR_HEAD_BRANCH, getInsecureHttpsAgent } from './config.js';
嚴重等級:🟡 警告 審查員:Bard 問題:Gitea 設定匯入一口氣列出八個名稱,行寬過長,讓讀者難以快速分辨這個模組真正依賴哪些環境值。 建議:將具名 import 拆成多行,必要時依 token、repo/PR、TLS helper 等語意排序。
@@ -0,0 +92,4 @@
const resp = await axios.get(api(`/repos/${GITEA_REPOSITORY}/branches/${encodeURIComponent(branch)}`), {
headers: headers(),
timeout: 30000,
嚴重等級:🟡 警告 審查員:Maya 問題:shouldSkipBotCommit 目前只看到命中 bot marker 的測試,缺少「commit API 失敗、分支查詢失敗、sha/branch 都沒有 marker」時應回 false 的失敗與保守路徑驗證。這是避免 workflow 誤跳過審查的關鍵判斷,不能只測快樂路徑。 建議:新增測試讓 getCommitMessageBySha / getBranchHeadCommitMessage 對應的 axios 呼叫拋錯或回一般 commit message,斷言 shouldSkipBotCommit 回 false,且不會把查詢失敗誤判成 bot commit。
@@ -0,0 +236,4 @@
* 取得目前 PR 上所有 review 的行內 comment 並展平為單一陣列。
* 單一 review 取 comment 失敗時記錄警告並略過,不中斷整體流程;最後輸出統計日誌。
* @returns {Promise<object[]>} 所有行內 comment 的展平陣列。
嚴重等級:🟡 警告 審查員:Rogue 問題:這裡逐一 await 每個 review 的 comments,PR review 一多就變成 N 次遠端呼叫的線性延遲累加;例如 30 個 review 就是 30 個 round-trip 排隊等,時間都被網路空轉偷走。 建議:把 reviews.map(review => getPullReviewComments(review.id).catch(...)) 丟進 Promise.all 或 Promise.allSettled 平行抓取,再 flat 結果;單筆失敗仍可記 warn 後略過。
@@ -0,0 +10,4 @@
* 並去除前後空白,使內容可直接交給 JSON.parse。
* 屬純函式、無副作用;常用於將 LLM 回傳結果正規化後再行解析。
嚴重等級:🔵 建議 審查員:Leo 問題:stripCodeFence() 與 src/llm.js 內的 stripOuterFence() 幾乎是同一個功能,未來如果要支援更多 fence 格式或修 bug,兩邊需要同步修改,容易產生行為漂移。 建議:抽成共用的 JSON/text utility,例如 src/text.js 或 src/json.js 匯出單一 fence 清理函式,讓 LLM JSON 解析與 JSON repair 共用同一套邏輯。
stripCodeFence()
src/llm.js
stripOuterFence()
src/text.js
src/json.js
@@ -0,0 +88,4 @@
reject(new Error(`${provider} CLI 逾時 (${timeout}ms)`));
}, timeout);
const append = (kind, chunk) => {
嚴重等級:🟡 警告 審查員:Maya 問題:runAssistantCLI 有 timeout 與 maxBuffer 兩條重要失敗路徑,但目前測試只覆蓋 CLI 非零退出,沒有驗證逾時會 kill 子程序並拒絕、輸出超過限制會中止且不產生未處理的重複 reject。這些是 CI 上最常見的失敗情境。 建議:新增 llm 測試:用假的 CLI sleep 超過 AI_ASSISTANT_TIMEOUT_MS,斷言錯誤訊息包含逾時;再用大量 stdout/stderr 超過 AI_ASSISTANT_MAX_BUFFER,斷言錯誤訊息正確且測試過程沒有 unhandled rejection。
@@ -0,0 +1,229 @@
import { GITEA_REPOSITORY, PR_NUMBER, PR_HEAD_BRANCH, PR_BASE_BRANCH, getLLMConfig, FINDINGS_PATH, EXCLUSIONS_PATH } from './config.js';
嚴重等級:🟡 警告 審查員:Bard 問題:這一行 import 把大量設定常數擠成長長一串,讀起來像沒有換氣的樂句,與後續同檔案多個長 import 一起讓檔案開頭難以掃描。 建議:將多項具名 import 改成多行排列,並依來源模組分組維持一致節奏,例如每個匯入項目獨立一行。
@@ -0,0 +15,4 @@
* AI Code Review Pipeline 的總指揮(orchestrator)。
嚴重等級:🟡 警告 審查員:Bard 問題:main() 前的 JSDoc 幾乎把整條 pipeline 逐步重寫一次,和函式內 Step 註解重複,維護時很容易變成兩份會走調的文件。 建議:縮短為高階摘要與退出規則;Step 細節留在程式碼附近,避免文件與實作雙重維護。
@@ -0,0 +33,4 @@
* 流程階段(Step1~Step11):
* - Step1 啟動:讀取 repo / PR / 分支等基本參數。
* - Step2 前置驗證:`runPreflight`,未通過則 exit 1。
* - Step3 自動提交檢查:偵測上輪 bot `[failure]`(exit 1)或本次為 bot 自動提交(exit 0 跳過)。
嚴重等級:🟡 警告 審查員:Leo 問題:main() 把前置驗證、bot commit 判斷、對話收斂、角色分析、合併去重、排除、發布、JSON 驗證、commit/push 與 gate 全部塞在同一個 190 行左右的函式裡,且中間散落多個 process.exit()。六個月後要改其中任一步驟時,很難隔離副作用,也不容易針對單一階段寫單元測試。 建議:將每個 Step 拆成可注入相依、回傳明確結果的函式,例如 runAnalysisStep()、runFilteringStep()、runPublishStep();最外層再統一把結果轉成 exit code,讓流程控制與業務邏輯分離。
main()
process.exit()
runAnalysisStep()
runFilteringStep()
runPublishStep()
@@ -0,0 +115,4 @@
input(`LLM=${provider}/${model};角色=[${roles.map(r => r.name).join(', ')}];diff=${diff.length} 字元`);
await postComment(getRoleIntro(roles) + `\n\n> 🔍 服務:${provider} 模型:${model}`);
嚴重等級:🟡 警告 審查員:Maya 問題:Step5 的角色分析與流程分支是整個 action 的核心,但目前測試沒有覆蓋 main orchestrator:例如所有角色分析都失敗時應 exit 1、部分角色失敗時仍繼續、diff 為空時 exit 0、critical finding 最後應讓 workflow 失敗。這些行為沒有被驗證,等於 pipeline 成敗判斷還沒通過試煉。 建議:補上 main 流程層級測試,透過 mock getPRDiff、loadRoles、analyzeWithRole、postFindingsReview、process.exit 等相依,至少覆蓋:diff 空、全部分析失敗、部分分析失敗但繼續、產生 critical 後 exit 1、無 critical 後正常通過。
@@ -0,0 +129,4 @@
newFindings.push(...findings);
warn(`[${role.name}] 分析失敗(跳過): ${e.message}`);
嚴重等級:🟡 警告 審查員:Rogue 問題:這裡把每個角色的 LLM 分析逐一 await,6 個角色就把總耗時堆成約 6 倍單次模型延遲;這些分析彼此獨立,CPU 沒偷到時間,反而把整條 pipeline 卡在序列網路/CLI 呼叫上。 建議:改用 Promise.allSettled 平行執行 roles.map(role => analyzeWithRole(role, diff)),再彙整 fulfilled 結果與 warning;保留 fulfilledAnalyses 的判斷即可。
@@ -0,0 +163,4 @@
step('Step7', '排除規則與誤報過濾');
if (reconcile.excludedFindings.length > 0) {
// 以 repoDir 為主(即將提交回去的來源分支副本),WORKSPACE 為鏡像;
// 順序須與下方 loadExclusions 一致,否則會讀到空的 WORKSPACE 而把既有排除規則覆蓋掉。
嚴重等級:🟡 警告 審查員:Maya 問題:Step7 會把 reconcile.excludedFindings 追加到 exclusions,接著再載入並套用排除規則,但目前缺少整合測試驗證「誤報對話 → 寫入 exclusions → 後續 findings 被排除」這條關鍵路徑。若 append/load/apply 任一環節接錯 workspace 或 mirror,單元測試不一定會抓到。 建議:補一個接近流程層級的測試,mock reconcileConversations 回傳 excludedFindings,準備一筆會被排除的新 finding,驗證 appendExclusions 寫入的檔案被 loadExclusions 讀到,且最後 save/post 的 filtered findings 不含該誤報。
@@ -0,0 +218,4 @@
result(false, `發現 ${criticalCount} 個嚴重問題,workflow 失敗(exit 1)`);
section('Pipeline 結束');
process.exit(1);
嚴重等級:🟡 警告 審查員:Mage 問題:若 Step6 的 cloneRepo() 失敗,repoDir 會是 undefined,但這裡仍呼叫 commitAndPush(WORKSPACE, repoDir || WORKSPACE, ...)。最小重現:遠端 clone 因分支不存在或網路錯誤失敗後,流程降級繼續,最後 Step10 會在 /workspace 這個非 git repo 執行 git config/status/commit,錯誤只被 commitAndPush 吞掉;findings/exclusions 已發布但不會被持久化到 PR 分支,下一輪會遺失記憶狀態。 建議:Step10 應在 repoDir 不存在時明確跳過 commit/push 並標記持久化失敗,或讓 clone 失敗成為會終止流程的錯誤;不要把 WORKSPACE 當成 repoDir fallback。
cloneRepo()
repoDir
commitAndPush(WORKSPACE, repoDir || WORKSPACE, ...)
/workspace
git config/status/commit
commitAndPush
@@ -0,0 +89,4 @@
const finding = parseBotReviewComment(body);
if (finding) g.botFinding = { ...finding, location: lineNum ? `${filePath}:${lineNum}` : filePath };
嚴重等級:🟡 警告 審查員:Mage 問題:對話分組行號只讀 position 或 original_position,但新增留言發布時使用的是 new_position。若 Gitea 回傳 review comment 只帶 new_position,最小重現:同一檔案第 10 行與第 20 行兩則未解決 bot comment 都沒有 position,兩者會被合併成 file|0,只解析第一個 finding,後續 resolved/open/false_positive 判斷會套錯問題。 建議:分組行號應納入 new_position,例如 Number(c?.position) || Number(c?.new_position) || Number(c?.original_position) || 0,並針對缺行號的 comment 避免把同檔不同對話合併成同一組。
position
original_position
new_position
file|0
Number(c?.position) || Number(c?.new_position) || Number(c?.original_position) || 0
@@ -0,0 +250,4 @@
line: c.line,
thread: c.thread,
code: codeWindow(fileCache.get(c.path) || '', c.line),
}));
嚴重等級:🔴 嚴重 審查員:Assassin 問題:這裡會把 PR 上所有未解決的 review comment ID 全部送去 resolve,而不是只處理 AI Review bot 自己建立的 thread。攻擊者只要開 PR 觸發這個 action,就可能讓 bot 關閉人類審查者留下的安全疑慮或阻擋性對話,繞過人工審查流程。 建議:只 resolve 可證明由本 bot 建立且格式符合預期的 comment,例如檢查作者、固定 marker、review body 簽章或 botFinding 解析結果;人類留言與未知格式留言不得自動關閉。
import yaml from 'js-yaml';
import { warn } from './log.js';
const ROLES_DIR = path.join(fileURLToPath(import.meta.url), '..', 'prompts', 'roles');
嚴重等級:🔵 建議 審查員:Leo 問題:ROLES_DIR 用 fileURLToPath(import.meta.url) 直接接 .. 來推目錄,雖然目前可運作,但語意上把檔案路徑當目錄路徑處理,未來搬檔或重構時不直覺。 建議:先用 path.dirname(fileURLToPath(import.meta.url)) 取得目前模組目錄,再組 prompts/roles,讓路徑意圖清楚且不依賴 .. 抵銷檔名的技巧。
ROLES_DIR
fileURLToPath(import.meta.url)
..
path.dirname(fileURLToPath(import.meta.url))
prompts/roles
本次審查(codex / gpt-5.4-mini,共 50 次呼叫)
@@ -0,0 +37,4 @@
* 取得 finding 等級的人類可讀字串(emoji + 中文標籤),已去除頭尾空白。
嚴重等級:🟡 警告 審查員:Mage 問題:這裡把 file:0 也視為有效行號;最小重現:只要上游傳進 app/foo.js:0,parseLocation() 會回傳 line=0,後續 postPullReviewComment 會帶著 new_position: 0 發到 Gitea,通常會被拒絕或定位失敗。也就是說,0 行號沒有被當成缺值處理。 建議:把行號門檻改成 > 0,0 與負數都應視為無效;同時讓需要行號的呼叫端把這種情況當作缺行號,重新定位或降級處理。
file:0
app/foo.js:0
parseLocation()
postPullReviewComment
new_position: 0
> 0
@@ -0,0 +208,4 @@
path: loc.file,
body: reviewCommentBody(f),
new_position: loc.line,
};
嚴重等級:🟡 警告 審查員:Maya 問題:postFindingsReview() 的主快樂路徑有測到,但它在批次 review 失敗後還有第二層降級邏輯:先重送 summary-only review,若 summary 也失敗才改走一般 comment,再逐筆補行內 comment。這條失敗鏈現在沒有被驗證,等於最重要的容錯行為只被程式碼描述,沒有被試煉。 建議:補測 postReview({ body, comments }) 先丟錯、再讓 postReview({ body, comments: [] }) 也丟錯的情境,斷言最後會呼叫 postIssue(body),且原本的 comments 仍會逐筆走 postInline。如果要更完整,也順便補 postOldFindingsComment() 與 postNewNonCriticalComment() 的篩選與空集合跳過案例。
postFindingsReview()
postReview({ body, comments })
postReview({ body, comments: [] })
postIssue(body)
postInline
postOldFindingsComment()
postNewNonCriticalComment()
@@ -0,0 +28,4 @@
// 取值優先序:有對應 `with:` 輸入的欄位一律 INPUT_* 優先(使用端明確傳入的值最權威,
// 蓋過環境中剛好存在的 ambient env)→ 專用 env(相容舊 Docker 版與測試)→ 內建預設。
// 無對應輸入的欄位(server url / repository / PR_*)則走專用 env → runner 內建 / 事件 payload。
// 使用端 workflow 只需傳 `with: token`。
嚴重等級:🟡 警告 審查員:Maya 問題:這裡是整個 action 讀取 INPUT_*、GITEA_* 與事件 payload 的入口,但測試只覆蓋了 getLLMConfig(),沒有把 GITEA_TOKEN、GITEA_COMMENT_TOKEN、PR_NUMBER、PR_HEAD_SHA 這些環境與 payload 的優先序鎖住。特別是 comment token 退回主 token、以及 event 檔讀不到時回到空值的情境,都是 CI 最容易因環境差異壞掉的地方。 建議:新增 config 相關測試,分別用假 process.env 和暫存 event payload 檔驗證:INPUT_* 會蓋過 ambient env、GITEA_COMMENT_TOKEN 缺值時會 fallback 到主 token、GITEA_EVENT_PATH / GITHUB_EVENT_PATH 讀取失敗時不會拋錯且回傳預設值。
GITEA_*
getLLMConfig()
GITEA_TOKEN
GITEA_COMMENT_TOKEN
PR_NUMBER
PR_HEAD_SHA
process.env
GITEA_EVENT_PATH
@@ -0,0 +108,4 @@
* @returns {string} 正規化後、以單一空白分隔的字串(可能為空字串)。
* @remarks 用於 finding 與排除條目文字的雙向「包含」比對(applyExclusions、appendExclusions)。
* 因為比對常對同一段文字重複呼叫(findings × exclusions 笛卡爾積),
* 以模組層級 Map 對「字串輸入」做 memoization,避免重複跑 NFKC/正則替換。
嚴重等級:🟡 警告 審查員:Leo 問題:normalizeText 與 toKeyText 兩套正規化規則不一致,前者會轉小寫,後者不會,而且註解還直接寫了「不確定」。這會讓排除、去重、比對在不同路徑出現微妙分歧,半年後很難追出到底是哪個標準才是正確來源。 建議:把文字正規化抽成單一共用 helper,讓大小寫是否敏感變成明確參數或不同命名的意圖函式,並補上覆蓋兩種路徑的測試,避免未來兩套規則繼續漂移。
normalizeText
toKeyText
嚴重等級:🔴 嚴重 審查員:Mage 問題:這個排除條件只要命中 location 或 role,就直接放行,不再檢查文字內容;最小重現:同一個 app/a.js 裡有一筆「誤報」排除後,app/a.js 的其他不同 finding 也會一起被濾掉。結果是單一排除項目可以吞掉整個檔案的有效問題。 建議:不要在有 location 或 role 時跳過文字比對;至少要同時驗證檔案、角色與正規化後的問題文字都匹配才排除,或改成以更穩定的 finding 簽章精準比對。
location
role
app/a.js
@@ -0,0 +410,4 @@
return findings.map(({ level, role, location, problem, suggestion }) => ({ level, role, location, problem, suggestion }));
嚴重等級:🟡 警告 審查員:Rogue 問題:這裡對每一筆 finding 都用 exclusions.some(...) 線性掃完整份排除清單,還在內層反覆做文字正規化,複雜度直接變成 O(findings × exclusions)。排除規則一多,這段會把 CPU 週期浪費在重複比對上。 建議:先把 exclusions 依 filePath、role 或正規化後的 textKey 建索引/分桶,再做比對;這樣可以把熱路徑從雙層掃描降到接近線性。
exclusions.some(...)
filePath
@@ -0,0 +219,4 @@
export async function commitAndPush(workspace, repoDir, _spawnSync = spawnSync, _sourceRoot = null, reviewOutcome = 'success') {
const run = makeRunner(_spawnSync);
const pushToken = GITEA_COMMENT_TOKEN || GITEA_TOKEN;
嚴重等級:🟡 警告 審查員:Leo 問題:commitAndPush 把 repo 對齊、檔案複製、stage、commit、push、失敗降級全部塞在一起,還保留了一個目前沒用到的 _sourceRoot 參數。這種 API 會越長越像腳本,之後要改 staging 規則或推送策略時,維護者很難快速定位應該改哪一段。 建議:把它拆成 syncRepo、stageReviewFiles、createCommit、pushCommit 幾個步驟,再由一個很薄的 orchestrator 串起來;同時移除或真正使用 _sourceRoot,避免留下誤導性的簽章。
_sourceRoot
syncRepo
stageReviewFiles
createCommit
pushCommit
@@ -0,0 +83,4 @@
* 取得指定分支 head commit 的訊息(先查 `GET /repos/{repo}/branches/{branch}` 取 SHA,再查該 commit)。
* 失敗或 branch 為空時不拋例外,記錄警告並回傳空字串。
嚴重等級:🔴 嚴重 審查員:Mage 問題:這裡只要 commit 或 branch 訊息包含 [ai-review-bot] 就回傳 true,但沒有區分 [success] 與 [failure];最小重現:上一輪 bot commit 是 [ai-review-bot][failure],而 getCommitMessageBySha 讀取失敗時,流程會被當成「可跳過」直接結束,原本應該讓 workflow 失敗的訊號被吃掉。 建議:讓這個函式回傳結構化結果,例如 success / failure / unknown,或至少在偵測到 [failure] 時明確回傳失敗狀態,交由 main() 先處理失敗再決定是否跳過。
commit
branch
[ai-review-bot]
[success]
[failure]
[ai-review-bot][failure]
getCommitMessageBySha
success / failure / unknown
@@ -0,0 +85,4 @@
* @param {string} fullPath 欲驗證的 JSON 檔案完整路徑。
* @param {string} label 檔案標籤,用於日誌與提示訊息。
* @param {(fullPath: string, label: string, rawText: string) => Promise<string>} [repairer=repairJSONArrayWithAI]
* 可注入的修復函式,預設使用 repairJSONArrayWithAI;便於測試替換。
嚴重等級:🟡 警告 審查員:Mage 問題:這裡只檢查 JSON.parse 能不能過,沒有確認解析結果一定是陣列;最小重現:repairer 回傳 {} 或檔案本身就是 {} 時,函式仍會回報 valid 並寫回磁碟,但後續程式都把它當陣列讀取,最後會悄悄被當成空資料或造成形狀錯誤。 建議:在驗證成功前先檢查 Array.isArray(parsed),只有真正的陣列才算通過;修復後也要同樣做陣列檢查,否則就丟錯並保留原檔。
JSON.parse
{}
Array.isArray(parsed)
@@ -0,0 +18,4 @@
'</system>',
'',
'<user>',
userContent,
嚴重等級:🟡 警告 審查員:Assassin 問題:這裡把未清洗的 userContent 直接塞進模型提示詞,等於讓 PR 內容、留言內容或其他外部文字能反過來操控 LLM。攻擊者可以在 diff 裡埋入『忽略前述規則、回傳空陣列』這類指令,讓審查模型漏報真正的風險或把嚴重問題降級成誤報。 建議:不要把不可信內容當成可執行指令使用。至少要把 diff/留言做更強的結構化封裝與逸出處理,並在輸出端加上嚴格的 JSON schema 驗證與 deterministic guardrail,避免 LLM 直接決定安全性結論。
userContent
@@ -0,0 +63,4 @@
const stderr = String(e.stderr || '').trim();
const stdout = String(e.stdout || '').trim();
return extractMeaningfulError(stderr || stdout || e.message || String(e));
嚴重等級:🟡 警告 審查員:Maya 問題:runAssistantCLI() 目前只有成功與一般失敗的測試,沒有覆蓋 timeout、maxBuffer 超限、以及 opencode 分支建立的暫存 prompt 檔在例外發生時是否確實清理。這些都是外部 CLI 整合最常出問題的失敗路徑,沒有測到就很難確定不會留下殘檔或把流程卡死。 建議:補 fake CLI 測試,讓子程序超時、輸出超過 AI_ASSISTANT_MAX_BUFFER、以及 opencode 在 spawn/close 前後失敗,分別斷言會回傳對應錯誤,且暫存目錄與 prompt.md 會被清掉。
runAssistantCLI()
maxBuffer
opencode
AI_ASSISTANT_MAX_BUFFER
spawn
close
prompt.md
@@ -0,0 +41,4 @@
* - Step7 過濾:套用排除規則 + 防守方 AI 誤報裁決。
* - Step8 發布:寫入 findings、組裝使用量,發布 Gitea Review(失敗則降級繼續)。
* - Step9 JSON 驗證:驗證 findings/exclusions 檔,格式錯誤 exit 1,缺檔則建立空陣列檔。
* - Step10 記憶區 commit/push:依是否有 critical 計算 reviewOutcome 後推回來源分支。
嚴重等級:🔴 嚴重 審查員:Maya 問題:這個 main() 是整個 action 的流程總管,但目前沒有任何整合測試或端到端測試去驗證 Step3~Step11 的分支切換與 process.exit() 行為。像是前置驗證失敗、偵測到 bot 自動提交、diff 為空、所有角色分析都失敗、JSON 驗證失敗、出現 critical finding、以及 commit/push 降級路徑,現在都只靠人工推演,實際接線後一旦流程順序或退出條件出錯,現有單元測試抓不到。 建議:補一組 main.test.js,把各模組依賴都 mock 掉,分別覆蓋 runPreflight=false、shouldSkipBotCommit=true、getPRDiff=''、分析全失敗、JSON 驗證拋錯、filtered 含 critical、push 失敗但流程不中斷等分支,並斷言對應的 exit code、呼叫順序與關鍵 log。
main.test.js
runPreflight=false
shouldSkipBotCommit=true
getPRDiff=''
@@ -0,0 +51,4 @@
* 降級處理:Step4 對話收斂、Step5 角色介紹 comment 與個別角色分析、Step6 clone repo、
* Step8 Review 發布等非致命步驟失敗時,僅 `warn` 後繼續執行。
async function main() {
嚴重等級:🟡 警告 審查員:Leo 問題:main() 已經變成整條 pipeline 的超級入口,11 個 step、exit 判斷、資料收集、排序/過濾與發布全部擠在同一個函式裡。未來只要某一步的前置條件改了,維護者就得在這個巨型函式裡追完整條狀態流,認知負擔很高。 建議:把每個 step 拆成獨立函式並回傳明確的 context,讓 main() 只負責流程編排與最終 exit 決策;這樣之後新增步驟或調整順序時,不會把整條 pipeline 綁死在同一個函式裡。
@@ -0,0 +95,4 @@
// Step5 角色分析:載入角色、取 diff,讓各角色平行產生 findings
step('Step5', '角色分析產生 findings');
const { provider, apiKeys, baseURL, model } = getLLMConfig();
嚴重等級:🟡 警告 審查員:Rogue 問題:這裡把每個角色的 LLM 分析用 for...of + await 串成一條龍,角色數一多就把總等待時間從「最慢那個角色」拉成「全部角色耗時相加」。每多一個角色,就白白多吃一輪模型呼叫延遲,熱路徑會被拖得很明顯。 建議:改成平行發出各角色分析,例如先 Promise.all 收集結果,再依原順序合併與排序;如果擔心單一失敗中斷,搭配 Promise.allSettled 保留容錯。
for...of + await
Promise.all
Promise.allSettled
@@ -0,0 +149,4 @@
data = await resp.json();
return { ok: false, error: `codex 模型清單回應解析失敗: ${e.message}` };
嚴重等級:🟡 警告 審查員:Rogue 問題:前置驗證的 Gitea token、comment token、git remote、LLM 驗證彼此沒有相依,卻被拆成連續等待。每個步驟都可能卡網路與 30 秒級 timeout,最差會把啟動時間疊成多倍,白白浪費整段等待。 建議:把互不相依的檢查改成並行執行,至少讓 token / remote / LLM 這幾項同時跑,只保留必要的 env 檢查先行。
@@ -0,0 +180,4 @@
* 對話收斂主流程:取得 PR 所有行內 review comment,
* 先把**每一個未解決的 comment**(依 comment id 去重,含無 path/position 者)一律呼叫 Gitea resolve API 關閉
* (findings.json 為唯一待辦來源,下次 review 依其重貼 comment);
嚴重等級:🟡 警告 審查員:Leo 問題:reconcileConversations 同時負責收 comment、分組、關閉、讀檔、AI 裁決、結果分類與降級處理,職責太多而且彼此耦合。任何一個小規則變動,都得先看完整條流程,單元測試也很難只鎖定某一段行為。 建議:拆成幾個可測的純函式與薄編排層,例如 groupConversations、closeOpenComments、buildJudgeItems、applyVerdicts 分開處理,讓主流程只保留資料流轉與錯誤收斂。
reconcileConversations
groupConversations
closeOpenComments
buildJudgeItems
applyVerdicts
@@ -0,0 +210,4 @@
// 要關閉的 comment:有 id 且尚未被 resolve(不依賴 path|line 分組,確保每個獨立 thread 都關到,含無 path/position 者)
const unresolvedCommentIds = [...new Set(
(comments || []).filter(c => c?.id != null && !c?.resolver).map(c => c.id),
)];
嚴重等級:🟡 警告 審查員:Mage 問題:findingSig 只用檔案路徑加上 suggestion 來識別問題,忽略了 problem、role,也沒有留下任何穩定的 thread 識別;最小重現:同一個 a.js 內有兩條都建議「加上 null 檢查」但其實是不同位置的 finding,先解掉其中一條後,另一條也會被當成同一筆而被 dropResolvedFindings / addCarriedFindings 誤合併或誤刪。 建議:把識別鍵改成更穩定的組合,例如檔案路徑 + 正規化後的 problem + suggestion + role,或直接使用可追蹤的 thread/issue id;不要只靠 suggestion 斷言是不是同一個問題。
findingSig
suggestion
problem
a.js
dropResolvedFindings
addCarriedFindings
本次審查(codex / gpt-5.4-mini,共 43 次呼叫)
@@ -0,0 +99,4 @@
function reviewCommentBody(f) {
return [
`**嚴重等級**:${levelText(f)}`,
嚴重等級:🔵 建議 審查員:Rogue 問題:統計表與單行摘要各欄位都用 filter(...).length 重掃多次,同一批 findings 會被走 4 到 8 次。資料量一大,連 log 文字本身都開始吃不必要的掃描成本。 建議:改成單次迴圈同時累加 critical / warning / info / 未分類計數,再把結果組成表格和摘要;一次走完就好,別讓統計自己變熱點。
filter(...).length
@@ -0,0 +230,4 @@
const comments = sortedComments.filter(f => f.is_new !== false).map(toReviewComment).filter(Boolean);
const body = buildReviewSummary(summaryFindings, usageSection);
await postReview({ body, comments });
嚴重等級:🟡 警告 審查員:Maya 問題:postFindingsReview 的降級流程有兩層:先嘗試批次 review,再失敗時改成 summary-only,最後 summary-only 也失敗才退回一般 comment。現在的測試只驗到第一層失敗後、第二層成功的情境,沒有驗證 summary-only 也失敗時是否真的會呼叫 postIssue(body),這是最脆弱的 fallback 路徑之一。 建議:新增一個測試讓第一次 postReview({comments}) 失敗、第二次 postReview({comments: []}) 也失敗,然後斷言 postIssue(body) 有被呼叫,且 inline comments 仍會逐筆嘗試送出。
postFindingsReview
postReview({comments})
postReview({comments: []})
@@ -0,0 +263,4 @@
for (const targetDir of targets) {
const fullPath = path.join(targetDir, FINDINGS_PATH);
fs.mkdirSync(path.dirname(fullPath), { recursive: true });
fs.writeFileSync(fullPath, JSON.stringify(findings, null, 2) + '\n', 'utf8');
嚴重等級:🔵 建議 審查員:Maya 問題:postOldFindingsComment 與緊接著的 postNewNonCriticalComment 都是這次新加的對外 comment 發布行為,但目前沒有專門測試它們的空陣列早退、標題文字與表格內容。這會讓 comment 分流邏輯只靠間接測試支撐,回歸時很容易漏掉。 建議:補這兩個函式的單元測試:至少驗證空陣列時不會送 comment、非空時 body 內容包含正確標題與表格,且 postOldFindingsComment 只收舊問題、postNewNonCriticalComment 只收新非 critical 問題。
postOldFindingsComment
postNewNonCriticalComment
@@ -0,0 +24,4 @@
const EVENT = readEventPayload();
const PR = EVENT.pull_request || {};
嚴重等級:🟡 警告 審查員:Bard 問題:這段註解已經跟著介面走音了。它宣稱使用端「只需傳 with: token」,但這次 action 其實已新增 comment_token 與 model 等輸入,註解仍停留在舊旋律,容易讓讀者誤判介面現況。 建議:把這組說明改成與目前 inputs 一致,明確列出 token、comment_token、model 的優先序與用途;如果無法精簡,就直接移到 README 或設計文件,避免在程式中留下過時註記。
comment_token
@@ -0,0 +224,4 @@
if (group.samples.length < 2 && exclusion.text) group.samples.push(exclusion.text);
return [...groups.values()]
嚴重等級:🟡 警告 審查員:Mage 問題:這個抽取器一旦命中目標檔案,就一路把後面的 diff 全部帶進去,沒有在下一個 diff --git 區塊時停下來。最小重現:diff 同時有 a.js 和 b.js,要補 a.js 的行號時,送給 LLM 的內容會混進 b.js 的 hunks,結果很容易定位到錯的行,或讓模型把別檔的內容誤認成目標檔上下文。 建議:在開始捕捉後,遇到下一個 diff --git 就應該停止,只回傳目前檔案那一段;找不到目標檔時再退回整份 diff。
diff --git
b.js
@@ -0,0 +274,4 @@
if (group.paths.length > 0) parts.push(`paths=${group.paths.join(', ')}`);
if (group.roles.length > 0) parts.push(`roles=${group.roles.join(', ')}`);
if (group.samples.length > 0) parts.push(`samples=${group.samples.join(' | ')}`);
return `- ${parts.join(' ; ')}`;
嚴重等級:🟡 警告 審查員:Maya 問題:deduplicateWithAI 是新的核心語意去重流程,但目前完全沒有直接測試它的成功與失敗分支。尤其是 LLM 回傳排序不同、夾雜幻覺項目、回傳空陣列或超量結果時,程式會改走保守 fallback,這些都是很容易壞掉但現在沒被驗證的邊界。 建議:替 deduplicateWithAI 補測兩類情境:一是 stub chatJSON 回傳重排後的重複項與一筆幻覺項,確認只保留能對應回原始 findings 的資料;二是回傳空陣列或不合法結果時,確認會保守回傳原始 findings。
deduplicateWithAI
chatJSON
@@ -0,0 +290,4 @@
嚴重等級:🟡 警告 審查員:Rogue 問題:每筆缺行號的 finding 都是單獨處理,還在每筆裡串行重試最多 3 次 LLM 查詢。N 筆問題就會膨脹成 N×3 次遠端呼叫與等待,幾十筆時延遲會直接被放大好幾倍。 建議:把行號定位改成可平行的批次流程,例如同檔或同角色一起送出後用 Promise.allSettled 收結果;重試只保留在單筆失敗時,別讓每個 finding 都自己拖慢整條管線。
@@ -0,0 +349,4 @@
const m = /^(.+?):(\d+)(?:-\d+)?$/.exec(s);
return m ? Number(m[2]) : null;
嚴重等級:🔴 嚴重 審查員:Assassin 問題:這裡會直接讀取 PR 工作樹中的 .gitea/ai-review/exclusions.json 當成可信排除來源。攻擊者可以先在分支裡放一份藏在 .gitea/ 下的 exclusions 檔,利用被忽略的路徑把自己的問題先排除掉,讓後續的 findings 被靜默吃掉。 建議:把 exclusions 視為 bot 自己管理的狀態,不要從 PR head 的工作樹直接信任既有內容;應該改成只讀受保護來源,或在載入前驗證檔案確實由 bot 生成且未被 PR 作者預先植入。
.gitea/ai-review/exclusions.json
.gitea/
@@ -0,0 +390,4 @@
if (located != null) {
f.location = `${file}:${located}`;
resolved += 1;
嚴重等級:🔵 建議 審查員:Rogue 問題:前面已經把 exclusions 正規化、去重過一次了,這裡為了 log 又再丟進 buildExclusionContext 重做 normalize / dedupe / group。等於同一批資料在同一輪流程裡被重算兩次,白白多吃一輪 O(n) 到 O(n log n) 的 CPU。 建議:把第一次處理的摘要一起回傳或快取下來,後面的 log 直接重用同一份結果,不要再對同一批 exclusions 重跑分組。
buildExclusionContext
@@ -0,0 +439,4 @@
* 讀取排除問題檔案(從來源分支的 cloned repoDir 中的 EXCLUSIONS_PATH)
嚴重等級:🟡 警告 審查員:Rogue 問題:這裡每一筆 finding 都要跟整包 exclusions 做一次 .some(),而且內層還反覆跑 normalizeText 和字串包含比對,資料一多就直接變成 O(F×E) 的熱點。像 300 筆 finding 配 500 筆 exclusions,會吃掉 15 萬次以上的比對與正規化,CPU 和字串配置都在浪費。 建議:先把 exclusions 在載入時一次正規化並依 filePath / role / textKey 建索引,讓過濾改成近似 O(F);至少把 normalizeText 移到內層迴圈外,避免同一段字串被重算成百上千次。
.some()
filePath / role / textKey
return String(text || '')
.split('\n')
.map(l => l.trim())
.filter(l => l && !l.startsWith('#'));
嚴重等級:🔴 嚴重 審查員:Assassin 問題:這裡直接從 PR head 讀取 .reviewignore,再拿它當成排除規則。攻擊者可以在自己的分支塞入排除條目,讓 bot 故意跳過包含惡意變更的檔案或整個目錄,等於自己決定哪些地方不被審查。 建議:不要信任 PR head 裡的 .reviewignore 來決定安全掃描範圍;改從受保護的 base branch 或 maintainer 管控的位置讀取,且要與固定的預設排除清單合併,而不是讓它覆蓋預設規則。
.reviewignore
@@ -0,0 +66,4 @@
// Step3 自動提交檢查:判斷本次 PR head 是否為 bot 自動提交
嚴重等級:🟡 警告 審查員:Mage 問題:這裡先檢查 head SHA 對應的訊息是否為 failure,但如果 SHA 查詢失敗或是空值,後面的 shouldSkipBotCommit() 仍可能只看到分支 head 上的 [ai-review-bot] 標記就直接跳過。最小重現:getCommitMessageBySha() 因 Gitea API 暫時失敗回空字串,而分支 head 正好是 [ai-review-bot][failure],流程就會 exit 0,等於把本來應該失敗的 bot commit 當成可跳過的自動提交。 建議:把「是否跳過」和「是否 failure」分開判斷,或讓 helper 回傳解析出的 outcome;只允許 success 標記走 skip,failure 標記不論 SHA/branch 來源都應優先讓流程失敗。
shouldSkipBotCommit()
getCommitMessageBySha()
const body = typeof c?.body === 'string' ? c.body : '';
if (body) g.bodies.push(body);
if (c?.resolver) g.resolved = true;
if (!g.botFinding) {
嚴重等級:🔵 建議 審查員:Rogue 問題:這個 codeWindow 每遇到一筆 open conversation 就對整份檔案內容再 split('\n') 一次。若同一個檔案有多條 thread,O(L) 的切割和陣列配置會被重複吃掉,明明同一份內容卻一直重複解剖。 建議:先把檔案內容預先切成行陣列並快取,或讓 codeWindow 直接吃已分割好的 lines;這樣同檔多條對話就不用重複掃描整份內容。
codeWindow
split('\n')
@@ -0,0 +168,4 @@
* @returns {string} 形如 `新: 嚴重1 / 警告0 / 建議2 / 無法標示0;舊: ...` 的單行字串。
* @remarks 供 {@link postFindingsReview} 在 log 輸出統計時呼叫。內容與 {@link formatFindingsStats} 一致,僅格式為單行純文字。
export function formatFindingsStatsLine(findings) {
嚴重等級:🟡 警告 審查員:Maya 問題:postFindingsReview 的救援路徑只測到「批次 review 失敗後,改發逐筆 inline comment」這一段,卻沒有驗證第二次 postReview({ comments: [] }) 也失敗時,會正確降級到 postIssue(body)。這條路徑是 Gitea review API 整個故障時保住摘要的最後保險絲,沒測到的話,真正出事時很容易靜默漏報。 建議:新增一個雙重失敗測試:第一次 postReview 因 comments 拋錯、第二次 postReview 也拋錯,最後斷言有呼叫 postIssue,而且 inline comments 仍會依序嘗試發布。
postReview({ comments: [] })
postReview
postIssue
@@ -0,0 +211,4 @@
嚴重等級:🟡 警告 審查員:Maya 問題:這裡新增了 postOldFindingsComment 與 postNewNonCriticalComment 兩條公開的留言分流路徑,但現有測試只驗證了 postNewCriticalComments 與 postFindingsReview,完全沒有案例確認這兩個函式的篩選條件、空陣列時是否跳過、以及輸出的 Markdown 內容是否真的只包含對應的 findings。這種分流邏輯一旦算錯,就會發生該發的沒發、或不該公告的問題被貼出去。 建議:補上這兩個函式的單元測試:空陣列時不呼叫 postComment;postOldFindingsComment 只送出 is_new === false 的項目;postNewNonCriticalComment 只送出 is_new 且 level !== 'critical' 的項目;再斷言 comment 標題與表格列數都符合預期。
postNewCriticalComments
postComment
is_new === false
is_new
level !== 'critical'
@@ -0,0 +1,588 @@
嚴重等級:🟡 警告 審查員:Leo 問題:這個模組同時處理舊 findings 載入、合併去重、缺行號補齊、排除規則正規化、誤報過濾、AI 去重、以及 exclusions 的讀寫,職責已經混成一包。更麻煩的是 loadExclusions、appendExclusions、applyExclusions 各自都有一套相近但不完全一致的比對邏輯,未來只要規則改一處,另一處沒同步就會開始出現不可預期的行為差異。 建議:把 exclusions 的正規化與比對規則抽成唯一來源,例如 normalizeExclusionEntry + matchesExclusion 之類的共用 helper,並把 AI 去重、行號補齊、檔案持久化拆到不同模組,減少這個檔案的責任面。
loadExclusions
appendExclusions
applyExclusions
normalizeExclusionEntry
matchesExclusion
@@ -0,0 +319,4 @@
return true;
});
const merged = [...oldFindings, ...deduped];
ok(`合併結果: 舊=${oldFindings.length} 新(去重後)=${deduped.length} 總計=${merged.length}`);
嚴重等級:🟡 警告 審查員:Rogue 問題:每一筆缺行號的 finding 都重新呼叫 extractFileDiff(diff, file) 掃完整份 diff,若同一檔案有 k 筆問題,就會重複做 k 次整份 diff 解析,浪費量是 O(k × diff長度)。 建議:先把 diff 依檔名切成快取 Map,一次掃描建立好 file -> fileDiff,後續同檔 finding 直接共用已切好的片段。
extractFileDiff(diff, file)
file -> fileDiff
@@ -0,0 +110,4 @@
const normalized = repaired.endsWith('\n') ? repaired : `${repaired}\n`;
// 先驗證修復結果是否為合法 JSON;無效就在寫檔前丟出,避免用毀損內容覆寫原檔。
JSON.parse(normalized);
fs.writeFileSync(fullPath, normalized, 'utf8');
嚴重等級:🟡 警告 審查員:Mage 問題:這裡只檢查 JSON.parse(normalized) 能不能成功,沒有確認修復後的內容真的是陣列。最小重現是 AI 把 findings.json 修成 { "a": 1 },函式會照樣寫回檔案並回報成功,但下一輪讀取時 readJSONArray 會把它當成非陣列而視為空值,等於把資料靜默吃掉。 建議:在寫檔前先 const parsed = JSON.parse(normalized),再加上 Array.isArray(parsed) 檢查;不是陣列就直接丟錯,不要覆寫原檔。
JSON.parse(normalized)
findings.json
{ "a": 1 }
readJSONArray
const parsed = JSON.parse(normalized)
@@ -0,0 +53,4 @@
'</user>',
].join('\n');
嚴重等級:🟡 警告 審查員:Bard 問題:cliArgs 把不同提供者的參數拼湊在同一個分支裡,還讓 opencode 走了另一套文字輸入路線,整個 helper 的節奏忽然一分為二。讀起來像兩個介面硬塞進同一支笛子。 建議:拆成各提供者各自的 argv builder,或至少把 prompt 輸入方式抽成獨立 helper,讓每個分支只處理一種責任,結構會更俐落。
cliArgs
@@ -0,0 +14,4 @@
const WORKSPACE = process.env.GITHUB_WORKSPACE || '/workspace';
嚴重等級:🟡 警告 審查員:Maya 問題:main() 整個流程目前沒有任何直接測試,只能靠零散的子函式單測推測結果;但這裡包含多個關鍵分支與 process.exit 行為,例如 preflight 失敗、bot 自動提交跳過、空 diff 提早結束、JSON 驗證失敗、以及偵測到 critical 後結束失敗。只要接線順序或退出碼改壞,現有測試不會第一時間抓到。 建議:補一組整合測試,把 runPreflight、getPRDiff、reconcileConversations、validateJSONArrayFile、commitAndPush 以 stub 注入,逐一覆蓋 Step3/5/9/11 的 exit 0/1 分支,至少驗證 process.exit 與主要副作用被正確觸發。
process.exit
runPreflight
getPRDiff
validateJSONArrayFile
@@ -0,0 +155,4 @@
warn(`clone repo 失敗(繼續執行): ${e.message}`);
const repoState = repoDir ? getRepoState(repoDir) : null;
if (repoState) line(`repo: branch=${repoState.branch || 'detached'} commit=${repoState.shortSha || 'unknown'}`);
嚴重等級:🔴 嚴重 審查員:Assassin 問題:這裡直接載入 PR 工作樹中的 .gitea/ai-review/exclusions.json 當成既有排除規則。攻擊者可以先在 PR 內預埋一份排除清單,因為 .gitea/ 又被預設排除於 diff 之外,這些惡意排除不會被審查到,卻會被流程直接拿來吞掉真正的 findings,形成靜默的審查繞過。 建議:不要從 PR head 讀取可由提交者任意修改的 exclusions;只接受由受信任 bot、受保護分支或外部持久化儲存產生的排除資料,並驗證來源身分與 commit marker,避免使用者自行預埋排除規則。
@@ -0,0 +71,4 @@
* 對話只要任一則 comment 帶有 resolver 即視為已解決;同時嘗試解析出該對話對應的 bot finding。
export function groupConversations(comments) {
const groups = new Map();
嚴重等級:🟡 警告 審查員:Mage 問題:這裡用 path + line 當唯一群組鍵,且只保留第一筆 botFinding。最小重現是同一個檔案同一行同時被兩個角色指出不同問題,groupConversations 會把它們合成同一組,後來的那筆 finding 會被吞掉,導致後續關閉、回寫或保留時少掉一個問題。 建議:不要只用 path + line 折疊所有 comment;至少要保留同一組內的所有 botFinding,或改成以 comment id / finding 本身為單位處理,再在最後階段做去重。
path + line
botFinding
@@ -0,0 +198,4 @@
let comments;
comments = await listComments();
嚴重等級:🔵 建議 審查員:Leo 問題:reconcileConversations() 同時在做 comment 分組、關閉遠端 review、讀檔、抽 code window、AI 裁決、再把結果拆成 resolved / excluded / carried 三條路徑,流程很完整,但也很難局部理解或替換。未來任何一段判斷要調整,都得先吞下整個函式的心智負擔,維護門檻偏高。 建議:把它拆成幾個可單獨測試的步驟,例如 collectOpenConversations()、loadConversationCode()、judgeConversationVerdicts()、mapVerdictsToFindings(),讓主流程只保留編排,不要把資料轉換與外部副作用全塞在一起。
reconcileConversations()
collectOpenConversations()
loadConversationCode()
judgeConversationVerdicts()
mapVerdictsToFindings()
@@ -0,0 +209,4 @@
export async function fetchAccountQuota(provider, config = {}, deps = {}) {
const get = deps.get || axios.get;
const strategy = QUOTA_STRATEGIES[provider];
if (!strategy) return { available: false, reason: `未支援 ${provider} 額度查詢` };
嚴重等級:🔵 建議 審查員:Mage 問題:這個百分比計算只擋了 limit <= 0,沒有擋 remaining < 0。最小重現是 resolveRemainingPercent({ available: true, used: 150, limit: 100 }, null) 或 remaining = -1,會算出負百分比,讓使用量摘要出現不合理的 -50% 之類結果,和函式註解宣告的「負數視為無法計算」不一致。 建議:把 remaining < 0 也納入無效值判斷,直接回 null;若你想保守顯示,可另外在輸出層把負值夾到 0,但不要讓計算層回傳負百分比。
limit <= 0
remaining < 0
resolveRemainingPercent({ available: true, used: 150, limit: 100 }, null)
remaining = -1
-50%
null
本次審查(codex / gpt-5.4-mini,共 69 次呼叫)
* `role`、`location`、`suggestion` 直接內嵌字串(未定義時會輸出 undefined 字樣)。傳入 null/undefined 時回傳空字串(已防呆,不會拋例外)。
* @returns {string} 形如 `| 🔴 嚴重 | role | location | suggestion |` 的表格列字串;`f` 為空值時回傳空字串。
* @remarks 內部輔助函式,供 {@link buildTable} 逐列組裝表格使用,本身不含換行。
嚴重等級:🔵 建議 審查員:Leo 問題:Markdown 表格列直接把 role、location、suggestion 原樣插進去,沒有處理 |、換行或其他會破壞表格結構的字元。現在看起來能跑,但只要 LLM 產出一個含管線符號的建議,表格格式就會裂掉,後續維護者會一直在修奇怪的留言排版。 建議:在組表格列前先對欄位做 Markdown escape,至少處理 | 與換行;如果內容本來就可能很長,改成每列用 code block 或純段落格式會更穩。
@@ -0,0 +72,4 @@
return { file: match[1], line: Number(match[2]) };
/** 行內 comment 內容:等級/審查員/建議 */
嚴重等級:🟡 警告 審查員:Mage 問題:這裡把 file:0 當成有效行號回傳。後續 postFindingsReview 和行內 critical comment 會把它送進 Gitea,但 new_position = 0 並不是有效 diff 行號,結果不是 API 拒絕,就是整筆 comment 被降級/略過。最小重現:LLM 回 app/a.js:0,流程仍會嘗試建立行內註解。 建議:在 parseLocation 只接受 line > 0,0 或負數都應視為無法定位,回傳 null 讓流程走補行號或一般 comment 的降級路徑。
new_position = 0
app/a.js:0
parseLocation
line > 0
@@ -0,0 +150,4 @@
export function formatFindingsStats(findings) {
const oldFindings = findings.filter(f => f.is_new === false);
const newFindings = newFindingsOnly(findings);
const row = (label, items) => `| ${label} | ${countBy(items, f => f.level === 'critical')} 筆 | ${countBy(items, f => f.level === 'warning')} 筆 | ${countBy(items, f => f.level === 'info')} 筆 | ${countBy(items, isUnclassified)} 筆 |`;
嚴重等級:🟡 警告 審查員:Leo 問題:這裡開始對 is_new 的解讀就和前面的統計邏輯不一致了:!f.is_new 會把 undefined 當成舊問題,但同檔前面的 newFindingsOnly() 又把 undefined 當新問題。之後 formatFindingsStats、舊問題留言、新問題留言會各自走不同分類,未來只要上游少填一個欄位,結果就會悄悄分岔,很難追。 建議:抽出單一的 isNewFinding(f) / isOldFinding(f) 判斷 helper,所有統計、留言、持久化都只用同一套規則;同時把 undefined 到底算新還算舊明確定義並寫進註解與測試。
!f.is_new
undefined
newFindingsOnly()
formatFindingsStats
isNewFinding(f)
isOldFinding(f)
@@ -0,0 +183,4 @@
* @returns {string} review 本文(Markdown)。
* @remarks 內部輔助函式,供 {@link postFindingsReview} 產生整批 review 的 body。
function buildReviewSummary(findings, usageSection = '') {
嚴重等級:🔵 建議 審查員:Rogue 問題:這裡先把 commentFindings 全部排序,再過濾掉 is_new === false 的舊問題。等於對一批最後根本不會送出的資料先付一次 O(n log n) 排序成本,舊 finding 越多,這筆白工越大。 建議:先過濾出真正要發布的 new findings,再對那個子集合排序;不要讓舊問題一起吃排序成本。
commentFindings
@@ -0,0 +264,4 @@
ok(`findings 寫入: ${fullPath} (${findings.length} 筆)`);
嚴重等級:🟡 警告 審查員:Maya 問題:postOldFindingsComment 和緊接著的 postNewNonCriticalComment 都是新公開行為,但測試只覆蓋 postNewCriticalComments 與 postFindingsReview,沒有直接驗證這兩個 comment helper 的內容格式、空陣列跳過、以及 is_new/level 過濾是否正確。這會讓留言分流一旦退化,測試完全抓不到。 建議:為 postOldFindingsComment 補一個「有舊問題時會送出一則含表格的 comment、沒舊問題時完全不送」的測試,再為 postNewNonCriticalComment 補一個「只包含新且非 critical 的 finding、critical 不會混進去」的案例,並確認標題與表格列內容。
level
@@ -0,0 +13,4 @@
line(`[${role.name}] 開始分析`);
const findings = await chatJSON(buildAnalysisPrompt(role), `以下是 Git Diff 內容:\n\n${diff}`);
嚴重等級:🟡 警告 審查員:Assassin 問題:這裡把整份 diff 直接塞進 LLM 輸入,沒有做結構化封裝或輸出約束。惡意提交者可以在程式碼註解、字串或檔案內容裡埋 prompt injection,誘導模型少報、漏報,甚至捏造不該存在的問題,讓後續去重與發布流程建立在被污染的判斷上。 建議:把 diff 以嚴格結構化資料傳給模型,並對內容做明確分隔與 escape;同時把 LLM 輸出視為不可信建議,加入 deterministic 驗證與白名單檢查,不要讓單次回應直接驅動刪除或封存問題。
function formatFileTime(mtimeMs) {
if (!Number.isFinite(mtimeMs)) return 'unknown';
return new Date(mtimeMs).toISOString();
嚴重等級:🟡 警告 審查員:Leo 問題:這裡的文字正規化規則和 normalizeText() 不一致,還在註解裡直接寫了「不確定是否預期」。再往下又有 mergeFindings、appendExclusions、applyExclusions 各自用不同簽章做去重/比對,等於同一份排除資料在不同流程可能被視為不同東西。這種規則分裂最容易在半年後變成『怎麼這筆有時候去重,有時候又新增』的維運災難。 建議:把『同一條排除/去重規則』抽成單一 canonical helper,包含大小寫、標點、空白、簽章欄位的定義都集中在一處,所有 load / append / filter / dedupe 共同使用;如果差異是刻意的,也要把原因寫死在命名和測試裡。
mergeFindings
@@ -0,0 +199,4 @@
* 將排除條目依 textKey 分組統計,產生供 AI prompt 使用的群組摘要(含出現次數、涉及路徑與角色、樣本)。
* @param {Array<object>} exclusions - 已正規化(含 textKey、filePath、role、text、fingerprint)的排除條目。
* @returns {Array<{text: string, count: number, paths: string[], roles: string[], samples: string[]}>}
嚴重等級:🟡 警告 審查員:Mage 問題:這個去重 key 把 suggestion 截成前 50 個字元。只要兩筆 finding 在同檔、同角色、同位置,且建議文字前 50 字相同,後面的差異就會被吃掉。最小重現:兩個不同問題的 suggestion 都以相同開頭描述,第二筆會被當成重複直接丟失。 建議:不要用截斷字串當唯一鍵。改成完整 suggestion,或更穩定的 fingerprint(例如 role + location + suggestion hash)。如果真的要縮短,截斷值只能拿來顯示,不能拿來判重。
role + location + suggestion hash
@@ -0,0 +246,4 @@
function buildExclusionContext(exclusions) {
if (exclusions.length === 0) {
return {
rawCount: 0,
嚴重等級:🟡 警告 審查員:Maya 問題:mergeFindings 與 sortByLevel 是 Step6 的核心邏輯,但目前沒有直接測到去重 key 的行為,也沒有測到合併後的排序是否真的維持 critical > warning > info。只靠上層流程的間接測試,對這種資料整理規則不夠穩。 建議:新增針對 mergeFindings 的單元測試,至少覆蓋:舊新 findings 以 role + location + suggestion 前 50 字 去重、相同 key 只保留一筆、不同 key 不會誤合併;再補 sortByLevel 的排序測試,確認未知等級會被排到最後。
sortByLevel
role + location + suggestion 前 50 字
@@ -0,0 +300,4 @@
const stat = fs.statSync(fullPath);
line(`讀取舊 findings 檔案: ${fullPath}`);
line(`舊 findings 檔案資訊: bytes=${stat.size} mtime=${formatFileTime(stat.mtimeMs)} path=${path.relative(workspace, fullPath) || fullPath}`);
} else {
嚴重等級:🟡 警告 審查員:Mage 問題:這裡回填 AI 去重結果時,也用同一個 location + suggestion 前 50 字 當對照鍵。只要兩筆原始 finding 的 key 撞到,origMap 會只留最後一筆,AI 回傳的結果就可能對到錯的原始 finding,或直接被 filter(Boolean) 吃掉。最小重現:同檔同位置兩筆 suggestion 前 50 字相同,去重後會錯配或少一筆。 建議:讓 AI 回傳可追蹤的穩定 id 或 fingerprint,回填時用 id 對照原始資料,不要再依賴截斷 suggestion。若短 key 只是為了節省 token,至少也要另外保留完整對照表。
location + suggestion 前 50 字
filter(Boolean)
@@ -0,0 +403,4 @@
* 將 findings 精簡為僅含 level、role、location、problem、suggestion 的物件,移除多餘欄位以節省 token。
* @param {Array<object>} findings - 完整 findings 陣列。
嚴重等級:🟡 警告 審查員:Rogue 問題:這裡是典型的 findings × exclusions 雙層掃描,而且內層每次還要做字串正規化與比對。排除規則一多就變成 O(F*E) 熱點,幾百筆 finding 配幾百條 exclusions 時,白白重複掃描與正規化的成本會很明顯。 建議:先把 exclusions 預編成索引,例如依 filePath、role、正規化文字建 Map/Set,讓每個 finding 只檢查少量候選,別每次都把整包 exclusions 全掃過一遍。
findings × exclusions
Map/Set
@@ -0,0 +423,4 @@
const result = await chatJSON(systemPrompt, JSON.stringify(toAIPayload(findings)));
// 去重結果數量不得超過輸入(避免 LLM 無中生有),且每筆都必須能對應回原始 finding。
if (Array.isArray(result) && result.length > 0 && result.length <= findings.length) {
const keyOf = f => `${f.location}|${String(f.suggestion).slice(0, 50)}`;
嚴重等級:🟡 警告 審查員:Mage 問題:這裡只要 exPath 或 ex.role 存在,就直接把 textMatches 跳過。實際結果是:同一個檔案、同一個角色的任何其他 finding,只要碰上這筆排除規則就會被整包濾掉,哪怕問題本質完全不同。最小重現:先把 app/a.js 某個誤報加入 exclusions,之後同檔同角色的另一個真問題也會一起消失。 建議:把排除條件改成「路徑、角色、文字」的明確交集,不要在有 exPath 時就略過文字比對。若要容許寬鬆排除,至少也要把正規化後的原文或穩定指紋納進判斷,避免同檔不同問題被誤殺。
exPath
ex.role
@@ -0,0 +68,4 @@
* 從被審 PR 的 head ref 取得 `.reviewignore` 並解析為排除清單。
* 檔案不存在或為空時退回 {@link DEFAULT_REVIEW_IGNORE}。
嚴重等級:🟡 警告 審查員:Assassin 問題:.reviewignore 是從被審查的 PR head 直接讀回來的,提交者自己就能在同一個 PR 裡新增排除規則,把惡意檔案或關鍵目錄整批從 diff 中消失。攻擊者只要加幾條前綴,就能讓這個審查流程根本看不到真正危險的變更。 建議:不要信任 PR 內容裡的 .reviewignore;改從受保護的 base branch、獨立設定檔或固定白名單載入,並禁止同一次 PR 修改忽略規則時自動生效,改為人工覆核。
嚴重等級:🔴 嚴重 審查員:Assassin 問題:這裡只靠 commit 訊息是否包含 [ai-review-bot] 來判斷要不要跳過審查,等於把信任建立在可由任何提交者自行偽造的字串上。攻擊者只要把自己的 PR head commit 訊息改成這個標記,整個審查流程就會被直接略過。 建議:不要用可偽造的 commit message 當跳過依據;改查 Gitea 的提交作者、簽章或機器人帳號身分,或使用只有 bot 自己能產生的不可偽造狀態標記。
* 對 items 並行執行 async fn(保序回傳),加速多個獨立的 LLM 子行程呼叫。
* limit 為同時執行上限;`limit <= 0`、非數字或大於項目數時「不限制」(全部並行)。
嚴重等級:🟡 警告 審查員:Rogue 問題:這裡把 limit <= 0 解讀成「不限制」,直接開到 items.length 個 worker。只要 finding 或對話一多,就會同時 spawn 一整排 LLM 子行程,CPU、記憶體、檔案描述元一起被打爆,熱路徑很容易從平行加速變成資源風暴。 建議:預設改成固定上限或依 CPU 核心數設合理值,例如 2~4 或 os.cpus().length,把「完全不限制」改成明確 opt-in,避免大 PR 直接全開。
items.length
os.cpus().length
* @returns {Promise<void>} 流程正常走完(無嚴重問題)時 resolve;多數結束路徑會直接
* 呼叫 `process.exit()` 結束程序,函式不會以回傳值回報審查結果。
* @throws {Error} 內部未被個別 try/catch 攔截的未預期例外會向上拋出,
* 由頂層 `main().catch(...)` 接住並以 `process.exit(1)` 結束。
嚴重等級:🟡 警告 審查員:Maya 問題:這個 orchestrator 是整條 pipeline 的入口,但目前測試都停在零件層,沒有直接驗證 main() 的關鍵分支:前置驗證失敗、bot 自動提交直接退出、diff 為空直接退出、JSON 驗證失敗退出、以及發現 critical 時的 exit 1。這些流程一旦接線錯了,單元測試還是可能全綠。 建議:補一組 main() 的整合測試,透過依賴注入或 mock 把 runPreflight、getPRDiff、cloneRepo、postFindingsReview、validateJSONArrayFile、commitAndPush 與 process.exit 全部接起來,逐一斷言上述 exit/降級路徑與 side effect 都有被觸發。
cloneRepo
let auth;
auth = JSON.parse(fs.readFileSync(authPath, 'utf8'));
嚴重等級:🔵 建議 審查員:Leo 問題:clientVersion 被硬編成 0.142.5,這種版本字串沒有單一來源,時間一久幾乎一定會過期。到時候 preflight 會因為一個靜態常數失效,維護者還得回頭搜尋到底是哪裡卡住,排查成本很高。 建議:把 client version 提成共用常數或環境變數,或改成由 CLI/設定檔提供;至少在集中位置定義一次,避免多處散落的魔術字串。
clientVersion
0.142.5
export function parseBotReviewComment(body) {
if (typeof body !== 'string' || !body.includes('**')) return null;
const normalized = body.replace(/\r\n/g, '\n');
const levelRaw = fieldValue(normalized, '嚴重等級') || fieldValue(normalized, '等級');
嚴重等級:🔵 建議 審查員:Maya 問題:groupConversations 目前有測 position 與 original_position,但沒有測 new_position 這個常見的 Gitea review comment 欄位。這代表如果 API 回來的是 new_position,對話分組與 bot finding 對位是否正確,現在沒有被試煉過。 建議:補一個只帶 new_position、不帶 position 的 comment fixture,驗證它能正確分組、line 會用 new_position,而且解析出的 botFinding.location 仍然是 file:line。
line
botFinding.location
file:line
嚴重等級:🟡 警告 審查員:Mage 問題:同一個 path|line 的多筆 bot comment 只會保留第一筆 botFinding,後面的 finding 會被靜默丟掉。最小重現:同一行上有兩個不同角色或不同建議的 bot comment,reconcile 時只會帶回第一筆,另一筆不會進入 resolved/excluded/carried 清單。 建議:不要把同一個 path|line 的 thread 壓成單一 finding。至少要把同群組內所有 bot finding 都保留下來,或再加上穩定 fingerprint / comment id 做細分,避免同線多筆問題互相覆蓋。
path|line
@@ -0,0 +166,4 @@
* 安全守衛:判定路徑是否為 repo 內的相對路徑(拒絕絕對路徑、Windows 磁碟機前綴與含 `..` 的路徑穿越)。
嚴重等級:🟡 警告 審查員:Rogue 問題:這段為了產生約 41 行的 codeWindow,先把整個檔案內容從 Gitea 抓回來。大檔案時等於每個 open conversation 都在付整份檔案的網路與記憶體成本,實際只用到一小段片段,浪費量會跟檔案大小線性成長。 建議:改成只取需要的區間或 hunk 內容,或至少在可用本地 repo 時改讀本機檔案再切窗;不要為了少量上下文把整份檔案搬進來。
@@ -0,0 +225,4 @@
if (open.length === 0) {
ok(`對話收斂完成: 關閉 comment=${closedCount} 已修復=0 誤報=0 仍成立=0`);
return { ...EMPTY, closedCount };
嚴重等級:🟡 警告 審查員:Rogue 問題:這個 extractFileDiff 一旦開始抓到目標檔案,就一路把後面的所有 diff 都塞進去,根本沒有在下一個 diff --git 停下來。結果本來只想餵單一檔案的提示,可能膨脹成接近整份 PR diff,每次補行號都在多燒 token 與傳輸時間。 建議:在下一個 diff --git 區塊出現時立刻結束,或直接只擷取目標檔案的 hunk 區段;不要把目標檔後面的 diff 全部一併帶進 prompt。
extractFileDiff
@@ -0,0 +38,4 @@
* 結果快取於模組層級(`cachedRoles`),同一程序生命週期內只讀檔一次;之後即使
* 角色檔有變動也不會重新載入,需重啟程序才會生效。單一檔案解析失敗(壞 YAML、
* 缺 frontmatter 等)只記錄警告並略過,不會中斷其他角色的載入。
嚴重等級:🟡 警告 審查員:Assassin 問題:角色 prompt 直接從目前 checkout 的 src/prompts/roles/*.md 載入,等於把 system prompt 放在可被 PR 修改的位置。攻擊者只要改這些 markdown,就能改寫審查角色的指令,命令模型忽略漏洞、輸出空陣列,或把所有問題打成誤報。 建議:把角色 prompt 移出可被審查分支影響的路徑,例如打包進發佈產物或從受保護分支、簽章校驗後載入;至少要在審查目標分支變更這些檔案時拒絕自動採用。
src/prompts/roles/*.md
@@ -0,0 +178,4 @@
role.body,
]
: ['你是 🛡️ Paladin(聖騎士),公正的裁判。不冤枉無辜的程式碼,也不放水。'];
嚴重等級:🔵 建議 審查員:Maya 問題:buildVerdictPrompt 是防守方裁決流程的 prompt 契約,但目前沒有任何測試確認它會帶入預設 Paladin 人設、exclusionHint、以及 confirmed / false_positive 的輸出格式。這類 prompt 一旦格式偏掉,後面的對話收斂會很難診斷。 建議:補一個針對 buildVerdictPrompt 的測試,分別驗證:未傳 role 時會使用 Paladin 預設敘述、傳入防守角色時會套用角色 body、傳入 exclusionHint 時會原樣出現在 prompt 中,且回傳規格仍要求 confirmed | false_positive。
buildVerdictPrompt
exclusionHint
confirmed / false_positive
confirmed | false_positive
本次審查(codex / gpt-5.4-mini,共 11 次呼叫)
No dependencies set.
The note is not visible to the blocked user.
變更摘要
導入 AI 程式碼審查 Gitea Node action 的完整實作,並修正原範本殘留的結構問題,讓 action 可實際運作。
主要變更
feat):src/下 13 支模組(main / config / gitea / git / llm / findings / resolve / comments / usage / roles / preflight / json / log)+ 12 支測試(279 筆全過)+ 7 個審查角色 prompt。fix):action.yml的main由已刪除的src/index.js改指src/main.js,原本 action 無法啟動。config.js改為多層 fallback,PR 相關參數自 runner 事件 payload(GITHUB_EVENT_PATH)與內建 env 自動帶入;有with:對應的欄位(token / comment_token / model)一律INPUT_*優先。使用端 workflow 只需傳with: token。chore):更新 CI workflow(改用 setup LLM CLI + 執行本 action,補model輸入),移除範本node-template根package.json,改由src/package.json作為單一來源。影響範圍
node_modules(隨 repo 進版控,runner 不需npm install)。token,model為選填(不給則自動偵測 CLI 預設)。風險 / 注意事項
config.js全域關閉 TLS 憑證驗證(NODE_TLS_REJECT_UNAUTHORIZED=0)以相容內部自簽憑證,僅限受信任內網使用。node_modules進版控使 diff 龐大(含依賴共 7196+ 行)。.gitea/ai-review/findings.json不存在)。驗證
node --test:279 pass / 0 fail。INPUT_*蓋過 ambient env。🤖 AI Code Review 團隊
🤖 AI Code Review 團隊
🤖 AI Code Review 團隊
🤖 AI Code Review 團隊
🤖 AI Code Review 團隊
🤖 AI Code Review 團隊
🤖 AI Code Review 團隊
AI Code Review 統計
🤖 AI 助理使用量
本次審查(codex / gpt-5.5,共 40 次呼叫)
剩餘可用
剩餘可用:無法計算百分比(未支援 codex 額度查詢)
@@ -0,0 +8,4 @@const LEVEL_LABEL = { critical: '嚴重', warning: '警告', info: '建議' };const LEVEL_ORDER = ['critical', 'warning', 'info'];// 預先把等級對應到排序索引,bySeverity 排序時直接 O(1) 取值,省去每次比較的 includes + indexOf 掃描。const LEVEL_RANK = new Map(LEVEL_ORDER.map((level, index) => [level, index]));嚴重等級:🟡 警告
審查員:Bard
問題:大量私有輔助函式都配上篇幅很長的 JSDoc,許多內容只是重述程式碼表面行為,註解的聲量蓋過了旋律本身。
建議:保留公開 API 或非直覺決策的文件即可;私有小函式改用簡短註解,或讓函式命名本身說明用途。
@@ -0,0 +22,4 @@function findingRow(f) {if (!f) return '';return `| ${LEVEL_EMOJI[f.level] || ''} ${LEVEL_LABEL[f.level] || f.level} | ${f.role} | ${f.location} | ${f.suggestion} |`;}嚴重等級:🔵 建議
審查員:Maya
問題:Markdown 表格列直接嵌入 role、location、suggestion,但測試沒有覆蓋 suggestion 含
|、換行或 Markdown 特殊字元時的輸出。這不是要求現在一定要改格式,而是目前缺少案例確認表格在真實 LLM 輸出下不會被破壞。建議:補一個 comment body 格式測試,輸入 suggestion 含 pipe、換行與粗體符號,斷言輸出的 Markdown 結構符合預期;若目前行為會破表,應先定義轉義或替換規則再測。
@@ -0,0 +123,4 @@* 過濾出新問題(is_new 不等於 false 者)。** @param {Array<{ is_new?: boolean }>} findings 審查問題陣列。* @returns {Array<object>} 新問題子集合。嚴重等級:🔵 建議
審查員:Rogue
問題:countBy 用 filter(predicate).length 只為了計數卻配置中間陣列;formatFindingsStats/formatFindingsStatsLine 每列又重複掃多次,雖然 findings 通常不大,但這是在白白丟記憶體與掃描週期。
建議:改成單趟 reduce 統計 new/old × level 的計數表,或讓 countBy 用 for-of 累加數字、不建立 filter 結果陣列。
@@ -0,0 +1,127 @@import https from 'https';import fs from 'fs';import { execFileSync } from 'child_process';嚴重等級:🔵 建議
審查員:Bard
問題:註解說「需要內部服務相容時才使用 getInsecureHttpsAgent()」,下一行卻在模組載入時全域設定 TLS 環境變數,文件與程式碼唱了不同旋律。
建議:讓註解忠實描述目前行為,或把全域設定移到明確命名的初始化函式;至少避免文件暗示這是選擇性使用。
@@ -0,0 +4,4 @@// 本 action 會連接自架 Gitea / OpenCode,部署環境可能使用內部 CA 或自簽憑證。// 對外部服務請優先使用預設 TLS 驗證;需要內部服務相容時才使用 getInsecureHttpsAgent()。process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';嚴重等級:🔴 嚴重
審查員:Assassin
問題:這裡把
NODE_TLS_REJECT_UNAUTHORIZED全域設為0,等於讓整個 Node 程序放棄 TLS 憑證驗證。攻擊者只要能站到 runner 與 Gitea/LLM/任何 HTTPS API 之間,就能用偽造憑證攔截或竄改 diff、review 結果、token 驗證流程,甚至偷走 Authorization header。建議:移除全域停用 TLS 的設定。若內部自簽 CA 是必要情境,請改用可設定的 CA bundle(例如
NODE_EXTRA_CA_CERTS)或僅對明確允許的內部 host 使用專用 agent,且預設必須啟用憑證驗證。@@ -0,0 +50,4 @@* 停用憑證驗證有中間人攻擊風險,僅限受信任的內部環境使用。* @returns {import('https').Agent} 已關閉憑證驗證的 HTTPS Agent 單例。*/let _insecureHttpsAgent = null;嚴重等級:🔴 嚴重
審查員:Assassin
問題:這個 helper 直接建立
rejectUnauthorized: false的 HTTPS agent,後續 Gitea API 與 preflight 都會用它。攻擊者若能進行中間人攻擊,就能假冒 Gitea 回傳惡意 diff、偽造 comment/review API 回應,或攔截寫入用 token。建議:不要提供預設不驗證憑證的 agent。改成預設安全驗證;若真的要支援自簽憑證,請要求使用者明確提供信任的 CA 憑證,或以白名單 host 加上明確 opt-in 的設定限制風險。
@@ -0,0 +1,581 @@import fs from 'fs';import path from 'path';import { chatJSON } from './llm.js';import { buildAnalysisPrompt, loadRole, buildVerdictPrompt, buildLocateLinePrompt } from './roles.js';嚴重等級:🟡 警告
審查員:Bard
問題:這行把四個 prompt/role helper 壓在同一行,與檔案中龐大的流程函式相比,開頭的依賴清單先失了拍,降低可讀性。
建議:改為多行具名 import,讓每個 helper 名稱清楚露出,並與其他長 import 採相同格式。
@@ -0,0 +11,4 @@* 用單一角色分析 diff,回傳 findings 陣列。* role 欄位一律以角色定義的 name 為準,避免 LLM 自行填入不一致的名稱。*/export async function analyzeWithRole(role, diff) {嚴重等級:🟡 警告
審查員:Assassin
問題:這裡把未信任的 Git diff 直接送進 LLM。攻擊者可以在新增程式碼或註解中塞入提示詞注入內容,例如要求模型忽略安全問題、回傳空陣列或偽造低風險 findings,藉此讓自動安全審查失明。
建議:在分析 prompt 中明確標示 diff 是不可信資料,要求模型忽略 diff 內任何指令;同時加入結構化封裝、輸出 schema 驗證與必要的規則式安全檢查,避免完全依賴可被 prompt injection 操控的 LLM 判斷。
@@ -0,0 +101,4 @@return typeof value === 'string' ? value.trim() : '';}/**嚴重等級:🔵 建議
審查員:Leo
問題:文字正規化邏輯分散在
normalizeText()、toKeyText(),而src/resolve.js也有另一套normalizeKey()。這些函式對大小寫、標點與空白的處理不完全一致,長期會讓 finding 去重、排除與對話收斂出現難追的差異。建議:建立單一 normalization 模組,明確定義
normalizeForDisplayMatch、normalizeForSignature等用途,再讓 findings、resolve、exclusions 共用,並補上跨模組測試鎖定語意。@@ -0,0 +135,4 @@return cleanText(value).normalize('NFKC').replace(/[\p{P}\p{S}\s]+/gu, '').trim();嚴重等級:🔵 建議
審查員:Bard
問題:註解中留下「不確定」這種未定案語氣,像樂譜上的猶豫記號;讀者無法判斷這是刻意設計、待辦事項,還是審查遺留。
建議:若是刻意差異,改寫成明確理由;若待確認,改成可追蹤的 TODO 並標明決策者或議題。
@@ -0,0 +333,4 @@/*** AI 呼叫失敗時的統一降級處理*/function fallback(label, findings, e) {嚴重等級:🟡 警告
審查員:Mage
問題:AI 去重回傳只要是非空陣列就被接受,沒有檢查是否比原始 findings 更多。最小重現:原本 3 筆 findings,LLM 異常回傳 20 筆或加入不存在的 location,這裡會直接採用並進入發布與失敗判定,導致憑空產生問題或讓 workflow 誤失敗。
建議:去重結果應驗證每筆都能對應回原始 finding,且數量不得大於輸入;無法對應或數量異常時應降級回原始 findings,或只保留
origMap命中的項目。@@ -0,0 +378,4 @@pending += 1;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)}`;let located = null;嚴重等級:🟡 警告
審查員:Leo
問題:
loadExclusions()同時負責讀檔、解析多種格式、正規化、去重、記錄 repo 狀態、改寫原檔、鏡像寫入與建立 AI prompt 摘要。這個函式的職責過多,之後只要調整 exclusions 格式或同步策略,就很容易牽動不相關行為。建議:拆成
readExclusionsFile()、normalizeExclusionsData()、canonicalizeExclusionsFile()、logExclusionMetadata()等小函式,讓讀取、轉換、寫回與診斷各自可測。@@ -0,0 +379,4 @@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)}`;let located = null;for (let attempt = 1; attempt <= maxAttempts && located == null; attempt++) {嚴重等級:🔵 建議
審查員:Rogue
問題:loadExclusions 前面已經 normalizeExclusionEntry + dedupeExclusions,這裡又呼叫 buildExclusionContext(exclusions) 重新 normalize、dedupe、group 一輪,只為了 log groups 數;排除規則多時會多跑一趟 O(e log e) 的整理成本。
建議:讓 buildExclusionContext 可接受已正規化/已去重的 exclusions,或直接在 loadExclusions 重用現有 exclusions 進行 group 統計,避免重複正規化與排序。
@@ -0,0 +413,4 @@/*** 呼叫 LLM 進行語意去重,失敗時降級回傳原始 findings*/export async function deduplicateWithAI(findings) {嚴重等級:🟡 警告
審查員:Maya
問題:applyExclusions 的核心比對支援「只有文字、沒有路徑/角色」的排除規則,但現有測試多半靠相同檔案路徑命中,沒有驗證純文字排除、空文字排除、大小寫/標點差異等邊界。這條排除規則的最脆弱分支還沒被測到。
建議:補上 applyExclusions 的邊界測試:只有 suggestion/title 文字沒有 location 的 exclusion 應如何比對;空文字 exclusion 不應意外排除全部;標點、空白、大小寫正規化後相同的文字應依預期排除。
@@ -0,0 +427,4 @@return result.map(r => origMap.get(`${r.location}|${String(r.suggestion).slice(0, 50)}`) ?? r);}throw new Error('AI 回傳空陣列');} catch (e) {嚴重等級:🟡 警告
審查員:Rogue
問題:applyExclusions 在 findings × exclusions 的巢狀比對裡,每遇到一條 exclusion 就重算同一個 finding 的 normalizeText;F 筆 finding、E 條 exclusion 會做最多 F×E 次正規化與正則替換,這是很明顯的 CPU 浪費。
建議:先把 findings 預處理成含 fPath、normalizedFindingText 的陣列,exclusions 也先補齊 normalizedExclusionText,再做比對;同一筆文字只正規化一次。
@@ -0,0 +463,4 @@line(`讀取排除問題檔案: ${fullPath}`);line(`來源分支狀態: branch=${branch} commit=${shortSha} commit_time=${commitTime}`);line(`檔案資訊: bytes=${stat.size} mtime=${formatFileTime(stat.mtimeMs)} raw=${rawCount} normalized=${exclusions.length} path=${path.relative(workspace, fullPath) || fullPath}`);if (sourceFormat !== 'array') {嚴重等級:🟡 警告
審查員:Mage
問題:這裡優先使用
ex.textKey,但textKey是由toKeyText()產生的無分隔且未轉小寫文字,而 findingText 是normalizeText()產生的小寫、以空白分隔文字。最小重現:排除文字Update tests會變成Updatetests,finding suggestion 會變成update tests,兩邊互相includes都不成立,導致純文字排除規則失效。建議:排除條目與 finding 應使用同一套正規化函式比對;例如改存並使用
normalizeText(ex.text || ex.suggestion || ex.title || ''),或讓 finding 也轉成同樣的 compact/lowercase key。@@ -0,0 +468,4 @@if (mirrorWorkspace && path.resolve(mirrorWorkspace) !== path.resolve(workspace)) {const mirrorPath = path.join(mirrorWorkspace, EXCLUSIONS_PATH);fs.mkdirSync(path.dirname(mirrorPath), { recursive: true });writeCanonicalExclusions(mirrorPath, normalizedSource);嚴重等級:🔴 嚴重
審查員:Mage
問題:當排除條目有 location 或 role 時,這裡直接把文字比對結果短路成 true。最小重現:exclusions.json 只有
{ "location": "app/a.js:10", "original_finding": "誤報 A" },新的 finding 是app/a.js:99且 suggestion 完全不同,仍會因同檔案而被排除,導致真問題被靜默丟掉。建議:不要用
exPath || ex.role ? true : textMatches跳過文字比對;應至少要求位置精確匹配到同一行,或在同檔/同角色時仍必須通過textMatches,例如return locationMatches && roleMatches && textMatches,並明確定義 suggestion 空白時才是萬用規則。@@ -0,0 +223,4 @@try {await withAskpass(workspace, async credEnv => {run(['config', 'user.email', 'ai-review[bot]@gitea'], repoDir);嚴重等級:🔵 建議
審查員:Bard
問題:_sourceRoot` 的參數文件寫著「不確定,待確認」,讓公開函式簽名帶著未完成的旁白,破壞 API 文件的一致與可信度。
建議:若參數已不使用,移除它;若為相容性保留,明確寫成 deprecated/compatibility note,不要留下模糊語句。
@@ -0,0 +1,294 @@import axios from 'axios';import { GITEA_TOKEN, GITEA_COMMENT_TOKEN, GITEA_SERVER_URL, GITEA_REPOSITORY, PR_NUMBER, PR_HEAD_SHA, PR_HEAD_BRANCH, getInsecureHttpsAgent } from './config.js';嚴重等級:🟡 警告
審查員:Bard
問題:Gitea 設定匯入一口氣列出八個名稱,行寬過長,讓讀者難以快速分辨這個模組真正依賴哪些環境值。
建議:將具名 import 拆成多行,必要時依 token、repo/PR、TLS helper 等語意排序。
@@ -0,0 +92,4 @@try {const resp = await axios.get(api(`/repos/${GITEA_REPOSITORY}/branches/${encodeURIComponent(branch)}`), {headers: headers(),timeout: 30000,嚴重等級:🟡 警告
審查員:Maya
問題:shouldSkipBotCommit 目前只看到命中 bot marker 的測試,缺少「commit API 失敗、分支查詢失敗、sha/branch 都沒有 marker」時應回 false 的失敗與保守路徑驗證。這是避免 workflow 誤跳過審查的關鍵判斷,不能只測快樂路徑。
建議:新增測試讓 getCommitMessageBySha / getBranchHeadCommitMessage 對應的 axios 呼叫拋錯或回一般 commit message,斷言 shouldSkipBotCommit 回 false,且不會把查詢失敗誤判成 bot commit。
@@ -0,0 +236,4 @@/*** 取得目前 PR 上所有 review 的行內 comment 並展平為單一陣列。* 單一 review 取 comment 失敗時記錄警告並略過,不中斷整體流程;最後輸出統計日誌。* @returns {Promise<object[]>} 所有行內 comment 的展平陣列。嚴重等級:🟡 警告
審查員:Rogue
問題:這裡逐一 await 每個 review 的 comments,PR review 一多就變成 N 次遠端呼叫的線性延遲累加;例如 30 個 review 就是 30 個 round-trip 排隊等,時間都被網路空轉偷走。
建議:把 reviews.map(review => getPullReviewComments(review.id).catch(...)) 丟進 Promise.all 或 Promise.allSettled 平行抓取,再 flat 結果;單筆失敗仍可記 warn 後略過。
@@ -0,0 +10,4 @@* 並去除前後空白,使內容可直接交給 JSON.parse。** 屬純函式、無副作用;常用於將 LLM 回傳結果正規化後再行解析。*嚴重等級:🔵 建議
審查員:Leo
問題:
stripCodeFence()與src/llm.js內的stripOuterFence()幾乎是同一個功能,未來如果要支援更多 fence 格式或修 bug,兩邊需要同步修改,容易產生行為漂移。建議:抽成共用的 JSON/text utility,例如
src/text.js或src/json.js匯出單一 fence 清理函式,讓 LLM JSON 解析與 JSON repair 共用同一套邏輯。@@ -0,0 +88,4 @@reject(new Error(`${provider} CLI 逾時 (${timeout}ms)`));}, timeout);const append = (kind, chunk) => {嚴重等級:🟡 警告
審查員:Maya
問題:runAssistantCLI 有 timeout 與 maxBuffer 兩條重要失敗路徑,但目前測試只覆蓋 CLI 非零退出,沒有驗證逾時會 kill 子程序並拒絕、輸出超過限制會中止且不產生未處理的重複 reject。這些是 CI 上最常見的失敗情境。
建議:新增 llm 測試:用假的 CLI sleep 超過 AI_ASSISTANT_TIMEOUT_MS,斷言錯誤訊息包含逾時;再用大量 stdout/stderr 超過 AI_ASSISTANT_MAX_BUFFER,斷言錯誤訊息正確且測試過程沒有 unhandled rejection。
@@ -0,0 +1,229 @@import path from 'path';import { GITEA_REPOSITORY, PR_NUMBER, PR_HEAD_BRANCH, PR_BASE_BRANCH, getLLMConfig, FINDINGS_PATH, EXCLUSIONS_PATH } from './config.js';嚴重等級:🟡 警告
審查員:Bard
問題:這一行 import 把大量設定常數擠成長長一串,讀起來像沒有換氣的樂句,與後續同檔案多個長 import 一起讓檔案開頭難以掃描。
建議:將多項具名 import 改成多行排列,並依來源模組分組維持一致節奏,例如每個匯入項目獨立一行。
@@ -0,0 +15,4 @@/*** AI Code Review Pipeline 的總指揮(orchestrator)。*嚴重等級:🟡 警告
審查員:Bard
問題:main() 前的 JSDoc 幾乎把整條 pipeline 逐步重寫一次,和函式內 Step 註解重複,維護時很容易變成兩份會走調的文件。
建議:縮短為高階摘要與退出規則;Step 細節留在程式碼附近,避免文件與實作雙重維護。
@@ -0,0 +33,4 @@* 流程階段(Step1~Step11):* - Step1 啟動:讀取 repo / PR / 分支等基本參數。* - Step2 前置驗證:`runPreflight`,未通過則 exit 1。* - Step3 自動提交檢查:偵測上輪 bot `[failure]`(exit 1)或本次為 bot 自動提交(exit 0 跳過)。嚴重等級:🟡 警告
審查員:Leo
問題:
main()把前置驗證、bot commit 判斷、對話收斂、角色分析、合併去重、排除、發布、JSON 驗證、commit/push 與 gate 全部塞在同一個 190 行左右的函式裡,且中間散落多個process.exit()。六個月後要改其中任一步驟時,很難隔離副作用,也不容易針對單一階段寫單元測試。建議:將每個 Step 拆成可注入相依、回傳明確結果的函式,例如
runAnalysisStep()、runFilteringStep()、runPublishStep();最外層再統一把結果轉成 exit code,讓流程控制與業務邏輯分離。@@ -0,0 +115,4 @@}input(`LLM=${provider}/${model};角色=[${roles.map(r => r.name).join(', ')}];diff=${diff.length} 字元`);try {await postComment(getRoleIntro(roles) + `\n\n> 🔍 服務:${provider} 模型:${model}`);嚴重等級:🟡 警告
審查員:Maya
問題:Step5 的角色分析與流程分支是整個 action 的核心,但目前測試沒有覆蓋 main orchestrator:例如所有角色分析都失敗時應 exit 1、部分角色失敗時仍繼續、diff 為空時 exit 0、critical finding 最後應讓 workflow 失敗。這些行為沒有被驗證,等於 pipeline 成敗判斷還沒通過試煉。
建議:補上 main 流程層級測試,透過 mock getPRDiff、loadRoles、analyzeWithRole、postFindingsReview、process.exit 等相依,至少覆蓋:diff 空、全部分析失敗、部分分析失敗但繼續、產生 critical 後 exit 1、無 critical 後正常通過。
@@ -0,0 +129,4 @@newFindings.push(...findings);} catch (e) {warn(`[${role.name}] 分析失敗(跳過): ${e.message}`);}嚴重等級:🟡 警告
審查員:Rogue
問題:這裡把每個角色的 LLM 分析逐一 await,6 個角色就把總耗時堆成約 6 倍單次模型延遲;這些分析彼此獨立,CPU 沒偷到時間,反而把整條 pipeline 卡在序列網路/CLI 呼叫上。
建議:改用 Promise.allSettled 平行執行 roles.map(role => analyzeWithRole(role, diff)),再彙整 fulfilled 結果與 warning;保留 fulfilledAnalyses 的判斷即可。
@@ -0,0 +163,4 @@step('Step7', '排除規則與誤報過濾');if (reconcile.excludedFindings.length > 0) {// 以 repoDir 為主(即將提交回去的來源分支副本),WORKSPACE 為鏡像;// 順序須與下方 loadExclusions 一致,否則會讀到空的 WORKSPACE 而把既有排除規則覆蓋掉。嚴重等級:🟡 警告
審查員:Maya
問題:Step7 會把 reconcile.excludedFindings 追加到 exclusions,接著再載入並套用排除規則,但目前缺少整合測試驗證「誤報對話 → 寫入 exclusions → 後續 findings 被排除」這條關鍵路徑。若 append/load/apply 任一環節接錯 workspace 或 mirror,單元測試不一定會抓到。
建議:補一個接近流程層級的測試,mock reconcileConversations 回傳 excludedFindings,準備一筆會被排除的新 finding,驗證 appendExclusions 寫入的檔案被 loadExclusions 讀到,且最後 save/post 的 filtered findings 不含該誤報。
@@ -0,0 +218,4 @@result(false, `發現 ${criticalCount} 個嚴重問題,workflow 失敗(exit 1)`);section('Pipeline 結束');process.exit(1);}嚴重等級:🟡 警告
審查員:Mage
問題:若 Step6 的
cloneRepo()失敗,repoDir會是 undefined,但這裡仍呼叫commitAndPush(WORKSPACE, repoDir || WORKSPACE, ...)。最小重現:遠端 clone 因分支不存在或網路錯誤失敗後,流程降級繼續,最後 Step10 會在/workspace這個非 git repo 執行git config/status/commit,錯誤只被commitAndPush吞掉;findings/exclusions 已發布但不會被持久化到 PR 分支,下一輪會遺失記憶狀態。建議:Step10 應在
repoDir不存在時明確跳過 commit/push 並標記持久化失敗,或讓 clone 失敗成為會終止流程的錯誤;不要把 WORKSPACE 當成 repoDir fallback。@@ -0,0 +89,4 @@const finding = parseBotReviewComment(body);if (finding) g.botFinding = { ...finding, location: lineNum ? `${filePath}:${lineNum}` : filePath };}}嚴重等級:🟡 警告
審查員:Mage
問題:對話分組行號只讀
position或original_position,但新增留言發布時使用的是new_position。若 Gitea 回傳 review comment 只帶new_position,最小重現:同一檔案第 10 行與第 20 行兩則未解決 bot comment 都沒有position,兩者會被合併成file|0,只解析第一個 finding,後續 resolved/open/false_positive 判斷會套錯問題。建議:分組行號應納入
new_position,例如Number(c?.position) || Number(c?.new_position) || Number(c?.original_position) || 0,並針對缺行號的 comment 避免把同檔不同對話合併成同一組。@@ -0,0 +250,4 @@line: c.line,thread: c.thread,code: codeWindow(fileCache.get(c.path) || '', c.line),}));嚴重等級:🔴 嚴重
審查員:Assassin
問題:這裡會把 PR 上所有未解決的 review comment ID 全部送去 resolve,而不是只處理 AI Review bot 自己建立的 thread。攻擊者只要開 PR 觸發這個 action,就可能讓 bot 關閉人類審查者留下的安全疑慮或阻擋性對話,繞過人工審查流程。
建議:只 resolve 可證明由本 bot 建立且格式符合預期的 comment,例如檢查作者、固定 marker、review body 簽章或 botFinding 解析結果;人類留言與未知格式留言不得自動關閉。
@@ -0,0 +4,4 @@import yaml from 'js-yaml';import { warn } from './log.js';const ROLES_DIR = path.join(fileURLToPath(import.meta.url), '..', 'prompts', 'roles');嚴重等級:🔵 建議
審查員:Leo
問題:
ROLES_DIR用fileURLToPath(import.meta.url)直接接..來推目錄,雖然目前可運作,但語意上把檔案路徑當目錄路徑處理,未來搬檔或重構時不直覺。建議:先用
path.dirname(fileURLToPath(import.meta.url))取得目前模組目錄,再組prompts/roles,讓路徑意圖清楚且不依賴..抵銷檔名的技巧。🤖 AI Code Review 團隊
AI Code Review 統計
🤖 AI 助理使用量
本次審查(codex / gpt-5.4-mini,共 50 次呼叫)
剩餘可用
剩餘可用:無法計算百分比(未支援 codex 額度查詢)
@@ -0,0 +37,4 @@}/*** 取得 finding 等級的人類可讀字串(emoji + 中文標籤),已去除頭尾空白。嚴重等級:🟡 警告
審查員:Mage
問題:這裡把
file:0也視為有效行號;最小重現:只要上游傳進app/foo.js:0,parseLocation()會回傳 line=0,後續postPullReviewComment會帶著new_position: 0發到 Gitea,通常會被拒絕或定位失敗。也就是說,0 行號沒有被當成缺值處理。建議:把行號門檻改成
> 0,0與負數都應視為無效;同時讓需要行號的呼叫端把這種情況當作缺行號,重新定位或降級處理。@@ -0,0 +208,4 @@path: loc.file,body: reviewCommentBody(f),new_position: loc.line,};嚴重等級:🟡 警告
審查員:Maya
問題:
postFindingsReview()的主快樂路徑有測到,但它在批次 review 失敗後還有第二層降級邏輯:先重送 summary-only review,若 summary 也失敗才改走一般 comment,再逐筆補行內 comment。這條失敗鏈現在沒有被驗證,等於最重要的容錯行為只被程式碼描述,沒有被試煉。建議:補測
postReview({ body, comments })先丟錯、再讓postReview({ body, comments: [] })也丟錯的情境,斷言最後會呼叫postIssue(body),且原本的 comments 仍會逐筆走postInline。如果要更完整,也順便補postOldFindingsComment()與postNewNonCriticalComment()的篩選與空集合跳過案例。@@ -0,0 +28,4 @@// 取值優先序:有對應 `with:` 輸入的欄位一律 INPUT_* 優先(使用端明確傳入的值最權威,// 蓋過環境中剛好存在的 ambient env)→ 專用 env(相容舊 Docker 版與測試)→ 內建預設。// 無對應輸入的欄位(server url / repository / PR_*)則走專用 env → runner 內建 / 事件 payload。// 使用端 workflow 只需傳 `with: token`。嚴重等級:🟡 警告
審查員:Maya
問題:這裡是整個 action 讀取
INPUT_*、GITEA_*與事件 payload 的入口,但測試只覆蓋了getLLMConfig(),沒有把GITEA_TOKEN、GITEA_COMMENT_TOKEN、PR_NUMBER、PR_HEAD_SHA這些環境與 payload 的優先序鎖住。特別是 comment token 退回主 token、以及 event 檔讀不到時回到空值的情境,都是 CI 最容易因環境差異壞掉的地方。建議:新增 config 相關測試,分別用假
process.env和暫存 event payload 檔驗證:INPUT_*會蓋過 ambient env、GITEA_COMMENT_TOKEN缺值時會 fallback 到主 token、GITEA_EVENT_PATH/GITHUB_EVENT_PATH讀取失敗時不會拋錯且回傳預設值。@@ -0,0 +108,4 @@* @returns {string} 正規化後、以單一空白分隔的字串(可能為空字串)。* @remarks 用於 finding 與排除條目文字的雙向「包含」比對(applyExclusions、appendExclusions)。* 因為比對常對同一段文字重複呼叫(findings × exclusions 笛卡爾積),* 以模組層級 Map 對「字串輸入」做 memoization,避免重複跑 NFKC/正則替換。嚴重等級:🟡 警告
審查員:Leo
問題:
normalizeText與toKeyText兩套正規化規則不一致,前者會轉小寫,後者不會,而且註解還直接寫了「不確定」。這會讓排除、去重、比對在不同路徑出現微妙分歧,半年後很難追出到底是哪個標準才是正確來源。建議:把文字正規化抽成單一共用 helper,讓大小寫是否敏感變成明確參數或不同命名的意圖函式,並補上覆蓋兩種路徑的測試,避免未來兩套規則繼續漂移。
@@ -0,0 +379,4 @@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)}`;let located = null;for (let attempt = 1; attempt <= maxAttempts && located == null; attempt++) {嚴重等級:🔴 嚴重
審查員:Mage
問題:這個排除條件只要命中
location或role,就直接放行,不再檢查文字內容;最小重現:同一個app/a.js裡有一筆「誤報」排除後,app/a.js的其他不同 finding 也會一起被濾掉。結果是單一排除項目可以吞掉整個檔案的有效問題。建議:不要在有
location或role時跳過文字比對;至少要同時驗證檔案、角色與正規化後的問題文字都匹配才排除,或改成以更穩定的 finding 簽章精準比對。@@ -0,0 +410,4 @@return findings.map(({ level, role, location, problem, suggestion }) => ({ level, role, location, problem, suggestion }));}/**嚴重等級:🟡 警告
審查員:Rogue
問題:這裡對每一筆 finding 都用
exclusions.some(...)線性掃完整份排除清單,還在內層反覆做文字正規化,複雜度直接變成 O(findings × exclusions)。排除規則一多,這段會把 CPU 週期浪費在重複比對上。建議:先把 exclusions 依
filePath、role或正規化後的textKey建索引/分桶,再做比對;這樣可以把熱路徑從雙層掃描降到接近線性。@@ -0,0 +219,4 @@*/export async function commitAndPush(workspace, repoDir, _spawnSync = spawnSync, _sourceRoot = null, reviewOutcome = 'success') {const run = makeRunner(_spawnSync);const pushToken = GITEA_COMMENT_TOKEN || GITEA_TOKEN;嚴重等級:🟡 警告
審查員:Leo
問題:
commitAndPush把 repo 對齊、檔案複製、stage、commit、push、失敗降級全部塞在一起,還保留了一個目前沒用到的_sourceRoot參數。這種 API 會越長越像腳本,之後要改 staging 規則或推送策略時,維護者很難快速定位應該改哪一段。建議:把它拆成
syncRepo、stageReviewFiles、createCommit、pushCommit幾個步驟,再由一個很薄的 orchestrator 串起來;同時移除或真正使用_sourceRoot,避免留下誤導性的簽章。@@ -0,0 +83,4 @@/*** 取得指定分支 head commit 的訊息(先查 `GET /repos/{repo}/branches/{branch}` 取 SHA,再查該 commit)。* 失敗或 branch 為空時不拋例外,記錄警告並回傳空字串。嚴重等級:🔴 嚴重
審查員:Mage
問題:這裡只要
commit或branch訊息包含[ai-review-bot]就回傳 true,但沒有區分[success]與[failure];最小重現:上一輪 bot commit 是[ai-review-bot][failure],而getCommitMessageBySha讀取失敗時,流程會被當成「可跳過」直接結束,原本應該讓 workflow 失敗的訊號被吃掉。建議:讓這個函式回傳結構化結果,例如
success / failure / unknown,或至少在偵測到[failure]時明確回傳失敗狀態,交由main()先處理失敗再決定是否跳過。@@ -0,0 +85,4 @@* @param {string} fullPath 欲驗證的 JSON 檔案完整路徑。* @param {string} label 檔案標籤,用於日誌與提示訊息。* @param {(fullPath: string, label: string, rawText: string) => Promise<string>} [repairer=repairJSONArrayWithAI]* 可注入的修復函式,預設使用 repairJSONArrayWithAI;便於測試替換。嚴重等級:🟡 警告
審查員:Mage
問題:這裡只檢查
JSON.parse能不能過,沒有確認解析結果一定是陣列;最小重現:repairer 回傳{}或檔案本身就是{}時,函式仍會回報 valid 並寫回磁碟,但後續程式都把它當陣列讀取,最後會悄悄被當成空資料或造成形狀錯誤。建議:在驗證成功前先檢查
Array.isArray(parsed),只有真正的陣列才算通過;修復後也要同樣做陣列檢查,否則就丟錯並保留原檔。@@ -0,0 +18,4 @@'</system>','','<user>',userContent,嚴重等級:🟡 警告
審查員:Assassin
問題:這裡把未清洗的
userContent直接塞進模型提示詞,等於讓 PR 內容、留言內容或其他外部文字能反過來操控 LLM。攻擊者可以在 diff 裡埋入『忽略前述規則、回傳空陣列』這類指令,讓審查模型漏報真正的風險或把嚴重問題降級成誤報。建議:不要把不可信內容當成可執行指令使用。至少要把 diff/留言做更強的結構化封裝與逸出處理,並在輸出端加上嚴格的 JSON schema 驗證與 deterministic guardrail,避免 LLM 直接決定安全性結論。
@@ -0,0 +63,4 @@const stderr = String(e.stderr || '').trim();const stdout = String(e.stdout || '').trim();return extractMeaningfulError(stderr || stdout || e.message || String(e));}嚴重等級:🟡 警告
審查員:Maya
問題:
runAssistantCLI()目前只有成功與一般失敗的測試,沒有覆蓋 timeout、maxBuffer超限、以及opencode分支建立的暫存 prompt 檔在例外發生時是否確實清理。這些都是外部 CLI 整合最常出問題的失敗路徑,沒有測到就很難確定不會留下殘檔或把流程卡死。建議:補 fake CLI 測試,讓子程序超時、輸出超過
AI_ASSISTANT_MAX_BUFFER、以及opencode在spawn/close前後失敗,分別斷言會回傳對應錯誤,且暫存目錄與prompt.md會被清掉。@@ -0,0 +41,4 @@* - Step7 過濾:套用排除規則 + 防守方 AI 誤報裁決。* - Step8 發布:寫入 findings、組裝使用量,發布 Gitea Review(失敗則降級繼續)。* - Step9 JSON 驗證:驗證 findings/exclusions 檔,格式錯誤 exit 1,缺檔則建立空陣列檔。* - Step10 記憶區 commit/push:依是否有 critical 計算 reviewOutcome 後推回來源分支。嚴重等級:🔴 嚴重
審查員:Maya
問題:這個
main()是整個 action 的流程總管,但目前沒有任何整合測試或端到端測試去驗證 Step3~Step11 的分支切換與process.exit()行為。像是前置驗證失敗、偵測到 bot 自動提交、diff 為空、所有角色分析都失敗、JSON 驗證失敗、出現 critical finding、以及 commit/push 降級路徑,現在都只靠人工推演,實際接線後一旦流程順序或退出條件出錯,現有單元測試抓不到。建議:補一組
main.test.js,把各模組依賴都 mock 掉,分別覆蓋runPreflight=false、shouldSkipBotCommit=true、getPRDiff=''、分析全失敗、JSON 驗證拋錯、filtered 含 critical、push 失敗但流程不中斷等分支,並斷言對應的 exit code、呼叫順序與關鍵 log。@@ -0,0 +51,4 @@* 降級處理:Step4 對話收斂、Step5 角色介紹 comment 與個別角色分析、Step6 clone repo、* Step8 Review 發布等非致命步驟失敗時,僅 `warn` 後繼續執行。*/async function main() {嚴重等級:🟡 警告
審查員:Leo
問題:
main()已經變成整條 pipeline 的超級入口,11 個 step、exit 判斷、資料收集、排序/過濾與發布全部擠在同一個函式裡。未來只要某一步的前置條件改了,維護者就得在這個巨型函式裡追完整條狀態流,認知負擔很高。建議:把每個 step 拆成獨立函式並回傳明確的 context,讓
main()只負責流程編排與最終 exit 決策;這樣之後新增步驟或調整順序時,不會把整條 pipeline 綁死在同一個函式裡。@@ -0,0 +95,4 @@// Step5 角色分析:載入角色、取 diff,讓各角色平行產生 findingsstep('Step5', '角色分析產生 findings');const { provider, apiKeys, baseURL, model } = getLLMConfig();嚴重等級:🟡 警告
審查員:Rogue
問題:這裡把每個角色的 LLM 分析用
for...of + await串成一條龍,角色數一多就把總等待時間從「最慢那個角色」拉成「全部角色耗時相加」。每多一個角色,就白白多吃一輪模型呼叫延遲,熱路徑會被拖得很明顯。建議:改成平行發出各角色分析,例如先
Promise.all收集結果,再依原順序合併與排序;如果擔心單一失敗中斷,搭配Promise.allSettled保留容錯。@@ -0,0 +149,4 @@data = await resp.json();} catch (e) {return { ok: false, error: `codex 模型清單回應解析失敗: ${e.message}` };}嚴重等級:🟡 警告
審查員:Rogue
問題:前置驗證的 Gitea token、comment token、git remote、LLM 驗證彼此沒有相依,卻被拆成連續等待。每個步驟都可能卡網路與 30 秒級 timeout,最差會把啟動時間疊成多倍,白白浪費整段等待。
建議:把互不相依的檢查改成並行執行,至少讓 token / remote / LLM 這幾項同時跑,只保留必要的 env 檢查先行。
@@ -0,0 +180,4 @@/*** 對話收斂主流程:取得 PR 所有行內 review comment,* 先把**每一個未解決的 comment**(依 comment id 去重,含無 path/position 者)一律呼叫 Gitea resolve API 關閉* (findings.json 為唯一待辦來源,下次 review 依其重貼 comment);嚴重等級:🟡 警告
審查員:Leo
問題:
reconcileConversations同時負責收 comment、分組、關閉、讀檔、AI 裁決、結果分類與降級處理,職責太多而且彼此耦合。任何一個小規則變動,都得先看完整條流程,單元測試也很難只鎖定某一段行為。建議:拆成幾個可測的純函式與薄編排層,例如
groupConversations、closeOpenComments、buildJudgeItems、applyVerdicts分開處理,讓主流程只保留資料流轉與錯誤收斂。@@ -0,0 +210,4 @@// 要關閉的 comment:有 id 且尚未被 resolve(不依賴 path|line 分組,確保每個獨立 thread 都關到,含無 path/position 者)const unresolvedCommentIds = [...new Set((comments || []).filter(c => c?.id != null && !c?.resolver).map(c => c.id),)];嚴重等級:🟡 警告
審查員:Mage
問題:
findingSig只用檔案路徑加上suggestion來識別問題,忽略了problem、role,也沒有留下任何穩定的 thread 識別;最小重現:同一個a.js內有兩條都建議「加上 null 檢查」但其實是不同位置的 finding,先解掉其中一條後,另一條也會被當成同一筆而被dropResolvedFindings/addCarriedFindings誤合併或誤刪。建議:把識別鍵改成更穩定的組合,例如檔案路徑 + 正規化後的
problem+suggestion+role,或直接使用可追蹤的 thread/issue id;不要只靠suggestion斷言是不是同一個問題。🤖 AI Code Review 團隊
AI Code Review 統計
🤖 AI 助理使用量
本次審查(codex / gpt-5.4-mini,共 43 次呼叫)
剩餘可用
剩餘可用:無法計算百分比(未支援 codex 額度查詢)
@@ -0,0 +99,4 @@*/function reviewCommentBody(f) {return [`**嚴重等級**:${levelText(f)}`,嚴重等級:🔵 建議
審查員:Rogue
問題:統計表與單行摘要各欄位都用
filter(...).length重掃多次,同一批 findings 會被走 4 到 8 次。資料量一大,連 log 文字本身都開始吃不必要的掃描成本。建議:改成單次迴圈同時累加 critical / warning / info / 未分類計數,再把結果組成表格和摘要;一次走完就好,別讓統計自己變熱點。
@@ -0,0 +230,4 @@const comments = sortedComments.filter(f => f.is_new !== false).map(toReviewComment).filter(Boolean);const body = buildReviewSummary(summaryFindings, usageSection);try {await postReview({ body, comments });嚴重等級:🟡 警告
審查員:Maya
問題:
postFindingsReview的降級流程有兩層:先嘗試批次 review,再失敗時改成 summary-only,最後 summary-only 也失敗才退回一般 comment。現在的測試只驗到第一層失敗後、第二層成功的情境,沒有驗證 summary-only 也失敗時是否真的會呼叫postIssue(body),這是最脆弱的 fallback 路徑之一。建議:新增一個測試讓第一次
postReview({comments})失敗、第二次postReview({comments: []})也失敗,然後斷言postIssue(body)有被呼叫,且 inline comments 仍會逐筆嘗試送出。@@ -0,0 +263,4 @@for (const targetDir of targets) {const fullPath = path.join(targetDir, FINDINGS_PATH);fs.mkdirSync(path.dirname(fullPath), { recursive: true });fs.writeFileSync(fullPath, JSON.stringify(findings, null, 2) + '\n', 'utf8');嚴重等級:🔵 建議
審查員:Maya
問題:
postOldFindingsComment與緊接著的postNewNonCriticalComment都是這次新加的對外 comment 發布行為,但目前沒有專門測試它們的空陣列早退、標題文字與表格內容。這會讓 comment 分流邏輯只靠間接測試支撐,回歸時很容易漏掉。建議:補這兩個函式的單元測試:至少驗證空陣列時不會送 comment、非空時 body 內容包含正確標題與表格,且
postOldFindingsComment只收舊問題、postNewNonCriticalComment只收新非 critical 問題。@@ -0,0 +24,4 @@const EVENT = readEventPayload();const PR = EVENT.pull_request || {};嚴重等級:🟡 警告
審查員:Bard
問題:這段註解已經跟著介面走音了。它宣稱使用端「只需傳
with: token」,但這次 action 其實已新增comment_token與model等輸入,註解仍停留在舊旋律,容易讓讀者誤判介面現況。建議:把這組說明改成與目前 inputs 一致,明確列出
token、comment_token、model的優先序與用途;如果無法精簡,就直接移到 README 或設計文件,避免在程式中留下過時註記。@@ -0,0 +224,4 @@if (group.samples.length < 2 && exclusion.text) group.samples.push(exclusion.text);}return [...groups.values()]嚴重等級:🟡 警告
審查員:Mage
問題:這個抽取器一旦命中目標檔案,就一路把後面的 diff 全部帶進去,沒有在下一個
diff --git區塊時停下來。最小重現:diff 同時有a.js和b.js,要補a.js的行號時,送給 LLM 的內容會混進b.js的 hunks,結果很容易定位到錯的行,或讓模型把別檔的內容誤認成目標檔上下文。建議:在開始捕捉後,遇到下一個
diff --git就應該停止,只回傳目前檔案那一段;找不到目標檔時再退回整份 diff。@@ -0,0 +274,4 @@if (group.paths.length > 0) parts.push(`paths=${group.paths.join(', ')}`);if (group.roles.length > 0) parts.push(`roles=${group.roles.join(', ')}`);if (group.samples.length > 0) parts.push(`samples=${group.samples.join(' | ')}`);return `- ${parts.join(' ; ')}`;嚴重等級:🟡 警告
審查員:Maya
問題:
deduplicateWithAI是新的核心語意去重流程,但目前完全沒有直接測試它的成功與失敗分支。尤其是 LLM 回傳排序不同、夾雜幻覺項目、回傳空陣列或超量結果時,程式會改走保守 fallback,這些都是很容易壞掉但現在沒被驗證的邊界。建議:替
deduplicateWithAI補測兩類情境:一是 stubchatJSON回傳重排後的重複項與一筆幻覺項,確認只保留能對應回原始 findings 的資料;二是回傳空陣列或不合法結果時,確認會保守回傳原始 findings。@@ -0,0 +290,4 @@};}/**嚴重等級:🟡 警告
審查員:Rogue
問題:每筆缺行號的 finding 都是單獨處理,還在每筆裡串行重試最多 3 次 LLM 查詢。N 筆問題就會膨脹成 N×3 次遠端呼叫與等待,幾十筆時延遲會直接被放大好幾倍。
建議:把行號定位改成可平行的批次流程,例如同檔或同角色一起送出後用
Promise.allSettled收結果;重試只保留在單筆失敗時,別讓每個 finding 都自己拖慢整條管線。@@ -0,0 +349,4 @@const m = /^(.+?):(\d+)(?:-\d+)?$/.exec(s);return m ? Number(m[2]) : null;}嚴重等級:🔴 嚴重
審查員:Assassin
問題:這裡會直接讀取 PR 工作樹中的
.gitea/ai-review/exclusions.json當成可信排除來源。攻擊者可以先在分支裡放一份藏在.gitea/下的 exclusions 檔,利用被忽略的路徑把自己的問題先排除掉,讓後續的 findings 被靜默吃掉。建議:把 exclusions 視為 bot 自己管理的狀態,不要從 PR head 的工作樹直接信任既有內容;應該改成只讀受保護來源,或在載入前驗證檔案確實由 bot 生成且未被 PR 作者預先植入。
@@ -0,0 +390,4 @@}if (located != null) {f.location = `${file}:${located}`;resolved += 1;嚴重等級:🔵 建議
審查員:Rogue
問題:前面已經把 exclusions 正規化、去重過一次了,這裡為了 log 又再丟進
buildExclusionContext重做 normalize / dedupe / group。等於同一批資料在同一輪流程裡被重算兩次,白白多吃一輪 O(n) 到 O(n log n) 的 CPU。建議:把第一次處理的摘要一起回傳或快取下來,後面的 log 直接重用同一份結果,不要再對同一批 exclusions 重跑分組。
@@ -0,0 +439,4 @@/*** 讀取排除問題檔案(從來源分支的 cloned repoDir 中的 EXCLUSIONS_PATH)*/嚴重等級:🟡 警告
審查員:Rogue
問題:這裡每一筆 finding 都要跟整包 exclusions 做一次
.some(),而且內層還反覆跑normalizeText和字串包含比對,資料一多就直接變成 O(F×E) 的熱點。像 300 筆 finding 配 500 筆 exclusions,會吃掉 15 萬次以上的比對與正規化,CPU 和字串配置都在浪費。建議:先把 exclusions 在載入時一次正規化並依
filePath / role / textKey建索引,讓過濾改成近似 O(F);至少把normalizeText移到內層迴圈外,避免同一段字串被重算成百上千次。嚴重等級:🟡 警告
審查員:Rogue
問題:這裡每一筆 finding 都要跟整包 exclusions 做一次
.some(),而且內層還反覆跑normalizeText和字串包含比對,資料一多就直接變成 O(F×E) 的熱點。像 300 筆 finding 配 500 筆 exclusions,會吃掉 15 萬次以上的比對與正規化,CPU 和字串配置都在浪費。建議:先把 exclusions 在載入時一次正規化並依
filePath / role / textKey建索引,讓過濾改成近似 O(F);至少把normalizeText移到內層迴圈外,避免同一段字串被重算成百上千次。@@ -0,0 +63,4 @@return String(text || '').split('\n').map(l => l.trim()).filter(l => l && !l.startsWith('#'));嚴重等級:🔴 嚴重
審查員:Assassin
問題:這裡直接從 PR head 讀取
.reviewignore,再拿它當成排除規則。攻擊者可以在自己的分支塞入排除條目,讓 bot 故意跳過包含惡意變更的檔案或整個目錄,等於自己決定哪些地方不被審查。建議:不要信任 PR head 裡的
.reviewignore來決定安全掃描範圍;改從受保護的 base branch 或 maintainer 管控的位置讀取,且要與固定的預設排除清單合併,而不是讓它覆蓋預設規則。@@ -0,0 +66,4 @@process.exit(1);}// Step3 自動提交檢查:判斷本次 PR head 是否為 bot 自動提交嚴重等級:🟡 警告
審查員:Mage
問題:這裡先檢查 head SHA 對應的訊息是否為 failure,但如果 SHA 查詢失敗或是空值,後面的
shouldSkipBotCommit()仍可能只看到分支 head 上的[ai-review-bot]標記就直接跳過。最小重現:getCommitMessageBySha()因 Gitea API 暫時失敗回空字串,而分支 head 正好是[ai-review-bot][failure],流程就會 exit 0,等於把本來應該失敗的 bot commit 當成可跳過的自動提交。建議:把「是否跳過」和「是否 failure」分開判斷,或讓 helper 回傳解析出的 outcome;只允許 success 標記走 skip,failure 標記不論 SHA/branch 來源都應優先讓流程失敗。
嚴重等級:🟡 警告
審查員:Mage
問題:這裡先檢查 head SHA 對應的訊息是否為 failure,但如果 SHA 查詢失敗或是空值,後面的
shouldSkipBotCommit()仍可能只看到分支 head 上的[ai-review-bot]標記就直接跳過。最小重現:getCommitMessageBySha()因 Gitea API 暫時失敗回空字串,而分支 head 正好是[ai-review-bot][failure],流程就會 exit 0,等於把本來應該失敗的 bot commit 當成可跳過的自動提交。建議:把「是否跳過」和「是否 failure」分開判斷,或讓 helper 回傳解析出的 outcome;只允許 success 標記走 skip,failure 標記不論 SHA/branch 來源都應優先讓流程失敗。
@@ -0,0 +85,4 @@const body = typeof c?.body === 'string' ? c.body : '';if (body) g.bodies.push(body);if (c?.resolver) g.resolved = true;if (!g.botFinding) {嚴重等級:🔵 建議
審查員:Rogue
問題:這個
codeWindow每遇到一筆 open conversation 就對整份檔案內容再split('\n')一次。若同一個檔案有多條 thread,O(L) 的切割和陣列配置會被重複吃掉,明明同一份內容卻一直重複解剖。建議:先把檔案內容預先切成行陣列並快取,或讓
codeWindow直接吃已分割好的 lines;這樣同檔多條對話就不用重複掃描整份內容。🤖 AI Code Review 團隊
AI Code Review 統計
🤖 AI 助理使用量
本次審查(codex / gpt-5.4-mini,共 50 次呼叫)
剩餘可用
剩餘可用:無法計算百分比(未支援 codex 額度查詢)
@@ -0,0 +168,4 @@* @returns {string} 形如 `新: 嚴重1 / 警告0 / 建議2 / 無法標示0;舊: ...` 的單行字串。* @remarks 供 {@link postFindingsReview} 在 log 輸出統計時呼叫。內容與 {@link formatFindingsStats} 一致,僅格式為單行純文字。*/export function formatFindingsStatsLine(findings) {嚴重等級:🟡 警告
審查員:Maya
問題:
postFindingsReview的救援路徑只測到「批次 review 失敗後,改發逐筆 inline comment」這一段,卻沒有驗證第二次postReview({ comments: [] })也失敗時,會正確降級到postIssue(body)。這條路徑是 Gitea review API 整個故障時保住摘要的最後保險絲,沒測到的話,真正出事時很容易靜默漏報。建議:新增一個雙重失敗測試:第一次
postReview因 comments 拋錯、第二次postReview也拋錯,最後斷言有呼叫postIssue,而且 inline comments 仍會依序嘗試發布。@@ -0,0 +211,4 @@};}/**嚴重等級:🟡 警告
審查員:Maya
問題:這裡新增了
postOldFindingsComment與postNewNonCriticalComment兩條公開的留言分流路徑,但現有測試只驗證了postNewCriticalComments與postFindingsReview,完全沒有案例確認這兩個函式的篩選條件、空陣列時是否跳過、以及輸出的 Markdown 內容是否真的只包含對應的 findings。這種分流邏輯一旦算錯,就會發生該發的沒發、或不該公告的問題被貼出去。建議:補上這兩個函式的單元測試:空陣列時不呼叫
postComment;postOldFindingsComment只送出is_new === false的項目;postNewNonCriticalComment只送出is_new且level !== 'critical'的項目;再斷言 comment 標題與表格列數都符合預期。@@ -0,0 +1,588 @@import fs from 'fs';嚴重等級:🟡 警告
審查員:Leo
問題:這個模組同時處理舊 findings 載入、合併去重、缺行號補齊、排除規則正規化、誤報過濾、AI 去重、以及 exclusions 的讀寫,職責已經混成一包。更麻煩的是
loadExclusions、appendExclusions、applyExclusions各自都有一套相近但不完全一致的比對邏輯,未來只要規則改一處,另一處沒同步就會開始出現不可預期的行為差異。建議:把 exclusions 的正規化與比對規則抽成唯一來源,例如
normalizeExclusionEntry+matchesExclusion之類的共用 helper,並把 AI 去重、行號補齊、檔案持久化拆到不同模組,減少這個檔案的責任面。@@ -0,0 +319,4 @@return true;});const merged = [...oldFindings, ...deduped];ok(`合併結果: 舊=${oldFindings.length} 新(去重後)=${deduped.length} 總計=${merged.length}`);嚴重等級:🟡 警告
審查員:Rogue
問題:每一筆缺行號的 finding 都重新呼叫
extractFileDiff(diff, file)掃完整份 diff,若同一檔案有 k 筆問題,就會重複做 k 次整份 diff 解析,浪費量是 O(k × diff長度)。建議:先把 diff 依檔名切成快取 Map,一次掃描建立好
file -> fileDiff,後續同檔 finding 直接共用已切好的片段。@@ -0,0 +110,4 @@const normalized = repaired.endsWith('\n') ? repaired : `${repaired}\n`;// 先驗證修復結果是否為合法 JSON;無效就在寫檔前丟出,避免用毀損內容覆寫原檔。JSON.parse(normalized);fs.writeFileSync(fullPath, normalized, 'utf8');嚴重等級:🟡 警告
審查員:Mage
問題:這裡只檢查
JSON.parse(normalized)能不能成功,沒有確認修復後的內容真的是陣列。最小重現是 AI 把findings.json修成{ "a": 1 },函式會照樣寫回檔案並回報成功,但下一輪讀取時readJSONArray會把它當成非陣列而視為空值,等於把資料靜默吃掉。建議:在寫檔前先
const parsed = JSON.parse(normalized),再加上Array.isArray(parsed)檢查;不是陣列就直接丟錯,不要覆寫原檔。@@ -0,0 +53,4 @@'<user>',userContent,'</user>',].join('\n');嚴重等級:🟡 警告
審查員:Bard
問題:
cliArgs把不同提供者的參數拼湊在同一個分支裡,還讓opencode走了另一套文字輸入路線,整個 helper 的節奏忽然一分為二。讀起來像兩個介面硬塞進同一支笛子。建議:拆成各提供者各自的 argv builder,或至少把 prompt 輸入方式抽成獨立 helper,讓每個分支只處理一種責任,結構會更俐落。
@@ -0,0 +14,4 @@const WORKSPACE = process.env.GITHUB_WORKSPACE || '/workspace';/**嚴重等級:🟡 警告
審查員:Maya
問題:
main()整個流程目前沒有任何直接測試,只能靠零散的子函式單測推測結果;但這裡包含多個關鍵分支與process.exit行為,例如 preflight 失敗、bot 自動提交跳過、空 diff 提早結束、JSON 驗證失敗、以及偵測到 critical 後結束失敗。只要接線順序或退出碼改壞,現有測試不會第一時間抓到。建議:補一組整合測試,把
runPreflight、getPRDiff、reconcileConversations、validateJSONArrayFile、commitAndPush以 stub 注入,逐一覆蓋 Step3/5/9/11 的 exit 0/1 分支,至少驗證process.exit與主要副作用被正確觸發。@@ -0,0 +155,4 @@warn(`clone repo 失敗(繼續執行): ${e.message}`);}const repoState = repoDir ? getRepoState(repoDir) : null;if (repoState) line(`repo: branch=${repoState.branch || 'detached'} commit=${repoState.shortSha || 'unknown'}`);嚴重等級:🔴 嚴重
審查員:Assassin
問題:這裡直接載入 PR 工作樹中的
.gitea/ai-review/exclusions.json當成既有排除規則。攻擊者可以先在 PR 內預埋一份排除清單,因為.gitea/又被預設排除於 diff 之外,這些惡意排除不會被審查到,卻會被流程直接拿來吞掉真正的 findings,形成靜默的審查繞過。建議:不要從 PR head 讀取可由提交者任意修改的 exclusions;只接受由受信任 bot、受保護分支或外部持久化儲存產生的排除資料,並驗證來源身分與 commit marker,避免使用者自行預埋排除規則。
@@ -0,0 +71,4 @@* 對話只要任一則 comment 帶有 resolver 即視為已解決;同時嘗試解析出該對話對應的 bot finding。*/export function groupConversations(comments) {const groups = new Map();嚴重等級:🟡 警告
審查員:Mage
問題:這裡用
path + line當唯一群組鍵,且只保留第一筆botFinding。最小重現是同一個檔案同一行同時被兩個角色指出不同問題,groupConversations會把它們合成同一組,後來的那筆 finding 會被吞掉,導致後續關閉、回寫或保留時少掉一個問題。建議:不要只用
path + line折疊所有 comment;至少要保留同一組內的所有 botFinding,或改成以 comment id / finding 本身為單位處理,再在最後階段做去重。@@ -0,0 +198,4 @@let comments;try {comments = await listComments();} catch (e) {嚴重等級:🔵 建議
審查員:Leo
問題:
reconcileConversations()同時在做 comment 分組、關閉遠端 review、讀檔、抽 code window、AI 裁決、再把結果拆成 resolved / excluded / carried 三條路徑,流程很完整,但也很難局部理解或替換。未來任何一段判斷要調整,都得先吞下整個函式的心智負擔,維護門檻偏高。建議:把它拆成幾個可單獨測試的步驟,例如
collectOpenConversations()、loadConversationCode()、judgeConversationVerdicts()、mapVerdictsToFindings(),讓主流程只保留編排,不要把資料轉換與外部副作用全塞在一起。@@ -0,0 +209,4 @@export async function fetchAccountQuota(provider, config = {}, deps = {}) {const get = deps.get || axios.get;const strategy = QUOTA_STRATEGIES[provider];if (!strategy) return { available: false, reason: `未支援 ${provider} 額度查詢` };嚴重等級:🔵 建議
審查員:Mage
問題:這個百分比計算只擋了
limit <= 0,沒有擋remaining < 0。最小重現是resolveRemainingPercent({ available: true, used: 150, limit: 100 }, null)或remaining = -1,會算出負百分比,讓使用量摘要出現不合理的-50%之類結果,和函式註解宣告的「負數視為無法計算」不一致。建議:把
remaining < 0也納入無效值判斷,直接回null;若你想保守顯示,可另外在輸出層把負值夾到 0,但不要讓計算層回傳負百分比。🤖 AI Code Review 團隊
AI Code Review 統計
🤖 AI 助理使用量
本次審查(codex / gpt-5.4-mini,共 69 次呼叫)
剩餘可用
剩餘可用:無法計算百分比(未支援 codex 額度查詢)
@@ -0,0 +18,4 @@* `role`、`location`、`suggestion` 直接內嵌字串(未定義時會輸出 undefined 字樣)。傳入 null/undefined 時回傳空字串(已防呆,不會拋例外)。* @returns {string} 形如 `| 🔴 嚴重 | role | location | suggestion |` 的表格列字串;`f` 為空值時回傳空字串。* @remarks 內部輔助函式,供 {@link buildTable} 逐列組裝表格使用,本身不含換行。*/嚴重等級:🔵 建議
審查員:Leo
問題:Markdown 表格列直接把
role、location、suggestion原樣插進去,沒有處理|、換行或其他會破壞表格結構的字元。現在看起來能跑,但只要 LLM 產出一個含管線符號的建議,表格格式就會裂掉,後續維護者會一直在修奇怪的留言排版。建議:在組表格列前先對欄位做 Markdown escape,至少處理
|與換行;如果內容本來就可能很長,改成每列用 code block 或純段落格式會更穩。@@ -0,0 +72,4 @@return { file: match[1], line: Number(match[2]) };}/** 行內 comment 內容:等級/審查員/建議 */嚴重等級:🟡 警告
審查員:Mage
問題:這裡把
file:0當成有效行號回傳。後續postFindingsReview和行內 critical comment 會把它送進 Gitea,但new_position = 0並不是有效 diff 行號,結果不是 API 拒絕,就是整筆 comment 被降級/略過。最小重現:LLM 回app/a.js:0,流程仍會嘗試建立行內註解。建議:在
parseLocation只接受line > 0,0或負數都應視為無法定位,回傳null讓流程走補行號或一般 comment 的降級路徑。@@ -0,0 +150,4 @@export function formatFindingsStats(findings) {const oldFindings = findings.filter(f => f.is_new === false);const newFindings = newFindingsOnly(findings);const row = (label, items) => `| ${label} | ${countBy(items, f => f.level === 'critical')} 筆 | ${countBy(items, f => f.level === 'warning')} 筆 | ${countBy(items, f => f.level === 'info')} 筆 | ${countBy(items, isUnclassified)} 筆 |`;嚴重等級:🟡 警告
審查員:Leo
問題:這裡開始對
is_new的解讀就和前面的統計邏輯不一致了:!f.is_new會把undefined當成舊問題,但同檔前面的newFindingsOnly()又把undefined當新問題。之後formatFindingsStats、舊問題留言、新問題留言會各自走不同分類,未來只要上游少填一個欄位,結果就會悄悄分岔,很難追。建議:抽出單一的
isNewFinding(f)/isOldFinding(f)判斷 helper,所有統計、留言、持久化都只用同一套規則;同時把undefined到底算新還算舊明確定義並寫進註解與測試。@@ -0,0 +183,4 @@* @returns {string} review 本文(Markdown)。* @remarks 內部輔助函式,供 {@link postFindingsReview} 產生整批 review 的 body。*/function buildReviewSummary(findings, usageSection = '') {嚴重等級:🔵 建議
審查員:Rogue
問題:這裡先把
commentFindings全部排序,再過濾掉is_new === false的舊問題。等於對一批最後根本不會送出的資料先付一次 O(n log n) 排序成本,舊 finding 越多,這筆白工越大。建議:先過濾出真正要發布的 new findings,再對那個子集合排序;不要讓舊問題一起吃排序成本。
@@ -0,0 +264,4 @@const fullPath = path.join(targetDir, FINDINGS_PATH);fs.mkdirSync(path.dirname(fullPath), { recursive: true });fs.writeFileSync(fullPath, JSON.stringify(findings, null, 2) + '\n', 'utf8');ok(`findings 寫入: ${fullPath} (${findings.length} 筆)`);嚴重等級:🟡 警告
審查員:Maya
問題:
postOldFindingsComment和緊接著的postNewNonCriticalComment都是新公開行為,但測試只覆蓋postNewCriticalComments與postFindingsReview,沒有直接驗證這兩個 comment helper 的內容格式、空陣列跳過、以及is_new/level過濾是否正確。這會讓留言分流一旦退化,測試完全抓不到。建議:為
postOldFindingsComment補一個「有舊問題時會送出一則含表格的 comment、沒舊問題時完全不送」的測試,再為postNewNonCriticalComment補一個「只包含新且非 critical 的 finding、critical 不會混進去」的案例,並確認標題與表格列內容。@@ -0,0 +13,4 @@*/export async function analyzeWithRole(role, diff) {line(`[${role.name}] 開始分析`);const findings = await chatJSON(buildAnalysisPrompt(role), `以下是 Git Diff 內容:\n\n${diff}`);嚴重等級:🟡 警告
審查員:Assassin
問題:這裡把整份 diff 直接塞進 LLM 輸入,沒有做結構化封裝或輸出約束。惡意提交者可以在程式碼註解、字串或檔案內容裡埋 prompt injection,誘導模型少報、漏報,甚至捏造不該存在的問題,讓後續去重與發布流程建立在被污染的判斷上。
建議:把 diff 以嚴格結構化資料傳給模型,並對內容做明確分隔與 escape;同時把 LLM 輸出視為不可信建議,加入 deterministic 驗證與白名單檢查,不要讓單次回應直接驅動刪除或封存問題。
@@ -0,0 +88,4 @@function formatFileTime(mtimeMs) {if (!Number.isFinite(mtimeMs)) return 'unknown';return new Date(mtimeMs).toISOString();}嚴重等級:🟡 警告
審查員:Leo
問題:這裡的文字正規化規則和
normalizeText()不一致,還在註解裡直接寫了「不確定是否預期」。再往下又有mergeFindings、appendExclusions、applyExclusions各自用不同簽章做去重/比對,等於同一份排除資料在不同流程可能被視為不同東西。這種規則分裂最容易在半年後變成『怎麼這筆有時候去重,有時候又新增』的維運災難。建議:把『同一條排除/去重規則』抽成單一 canonical helper,包含大小寫、標點、空白、簽章欄位的定義都集中在一處,所有 load / append / filter / dedupe 共同使用;如果差異是刻意的,也要把原因寫死在命名和測試裡。
@@ -0,0 +199,4 @@* 將排除條目依 textKey 分組統計,產生供 AI prompt 使用的群組摘要(含出現次數、涉及路徑與角色、樣本)。** @param {Array<object>} exclusions - 已正規化(含 textKey、filePath、role、text、fingerprint)的排除條目。* @returns {Array<{text: string, count: number, paths: string[], roles: string[], samples: string[]}>}嚴重等級:🟡 警告
審查員:Mage
問題:這個去重 key 把
suggestion截成前 50 個字元。只要兩筆 finding 在同檔、同角色、同位置,且建議文字前 50 字相同,後面的差異就會被吃掉。最小重現:兩個不同問題的 suggestion 都以相同開頭描述,第二筆會被當成重複直接丟失。建議:不要用截斷字串當唯一鍵。改成完整
suggestion,或更穩定的 fingerprint(例如role + location + suggestion hash)。如果真的要縮短,截斷值只能拿來顯示,不能拿來判重。@@ -0,0 +246,4 @@function buildExclusionContext(exclusions) {if (exclusions.length === 0) {return {rawCount: 0,嚴重等級:🟡 警告
審查員:Maya
問題:
mergeFindings與sortByLevel是 Step6 的核心邏輯,但目前沒有直接測到去重 key 的行為,也沒有測到合併後的排序是否真的維持 critical > warning > info。只靠上層流程的間接測試,對這種資料整理規則不夠穩。建議:新增針對
mergeFindings的單元測試,至少覆蓋:舊新 findings 以role + location + suggestion 前 50 字去重、相同 key 只保留一筆、不同 key 不會誤合併;再補sortByLevel的排序測試,確認未知等級會被排到最後。@@ -0,0 +300,4 @@const stat = fs.statSync(fullPath);line(`讀取舊 findings 檔案: ${fullPath}`);line(`舊 findings 檔案資訊: bytes=${stat.size} mtime=${formatFileTime(stat.mtimeMs)} path=${path.relative(workspace, fullPath) || fullPath}`);} else {嚴重等級:🟡 警告
審查員:Mage
問題:這裡回填 AI 去重結果時,也用同一個
location + suggestion 前 50 字當對照鍵。只要兩筆原始 finding 的 key 撞到,origMap會只留最後一筆,AI 回傳的結果就可能對到錯的原始 finding,或直接被filter(Boolean)吃掉。最小重現:同檔同位置兩筆 suggestion 前 50 字相同,去重後會錯配或少一筆。建議:讓 AI 回傳可追蹤的穩定 id 或 fingerprint,回填時用 id 對照原始資料,不要再依賴截斷 suggestion。若短 key 只是為了節省 token,至少也要另外保留完整對照表。
@@ -0,0 +403,4 @@/*** 將 findings 精簡為僅含 level、role、location、problem、suggestion 的物件,移除多餘欄位以節省 token。** @param {Array<object>} findings - 完整 findings 陣列。嚴重等級:🟡 警告
審查員:Rogue
問題:這裡是典型的
findings × exclusions雙層掃描,而且內層每次還要做字串正規化與比對。排除規則一多就變成 O(F*E) 熱點,幾百筆 finding 配幾百條 exclusions 時,白白重複掃描與正規化的成本會很明顯。建議:先把 exclusions 預編成索引,例如依
filePath、role、正規化文字建Map/Set,讓每個 finding 只檢查少量候選,別每次都把整包 exclusions 全掃過一遍。@@ -0,0 +423,4 @@const result = await chatJSON(systemPrompt, JSON.stringify(toAIPayload(findings)));// 去重結果數量不得超過輸入(避免 LLM 無中生有),且每筆都必須能對應回原始 finding。if (Array.isArray(result) && result.length > 0 && result.length <= findings.length) {const keyOf = f => `${f.location}|${String(f.suggestion).slice(0, 50)}`;嚴重等級:🟡 警告
審查員:Mage
問題:這裡只要
exPath或ex.role存在,就直接把textMatches跳過。實際結果是:同一個檔案、同一個角色的任何其他 finding,只要碰上這筆排除規則就會被整包濾掉,哪怕問題本質完全不同。最小重現:先把app/a.js某個誤報加入 exclusions,之後同檔同角色的另一個真問題也會一起消失。建議:把排除條件改成「路徑、角色、文字」的明確交集,不要在有
exPath時就略過文字比對。若要容許寬鬆排除,至少也要把正規化後的原文或穩定指紋納進判斷,避免同檔不同問題被誤殺。@@ -0,0 +68,4 @@/*** 從被審 PR 的 head ref 取得 `.reviewignore` 並解析為排除清單。* 檔案不存在或為空時退回 {@link DEFAULT_REVIEW_IGNORE}。嚴重等級:🟡 警告
審查員:Assassin
問題:
.reviewignore是從被審查的 PR head 直接讀回來的,提交者自己就能在同一個 PR 裡新增排除規則,把惡意檔案或關鍵目錄整批從 diff 中消失。攻擊者只要加幾條前綴,就能讓這個審查流程根本看不到真正危險的變更。建議:不要信任 PR 內容裡的
.reviewignore;改從受保護的 base branch、獨立設定檔或固定白名單載入,並禁止同一次 PR 修改忽略規則時自動生效,改為人工覆核。@@ -0,0 +115,4 @@/*** 取得指定分支 head commit 的訊息(先查 `GET /repos/{repo}/branches/{branch}` 取 SHA,再查該 commit)。* 失敗或 branch 為空時不拋例外,記錄警告並回傳空字串。嚴重等級:🔴 嚴重
審查員:Assassin
問題:這裡只靠 commit 訊息是否包含
[ai-review-bot]來判斷要不要跳過審查,等於把信任建立在可由任何提交者自行偽造的字串上。攻擊者只要把自己的 PR head commit 訊息改成這個標記,整個審查流程就會被直接略過。建議:不要用可偽造的 commit message 當跳過依據;改查 Gitea 的提交作者、簽章或機器人帳號身分,或使用只有 bot 自己能產生的不可偽造狀態標記。
@@ -0,0 +14,4 @@/*** 對 items 並行執行 async fn(保序回傳),加速多個獨立的 LLM 子行程呼叫。** limit 為同時執行上限;`limit <= 0`、非數字或大於項目數時「不限制」(全部並行)。嚴重等級:🟡 警告
審查員:Rogue
問題:這裡把
limit <= 0解讀成「不限制」,直接開到items.length個 worker。只要 finding 或對話一多,就會同時 spawn 一整排 LLM 子行程,CPU、記憶體、檔案描述元一起被打爆,熱路徑很容易從平行加速變成資源風暴。建議:預設改成固定上限或依 CPU 核心數設合理值,例如 2~4 或
os.cpus().length,把「完全不限制」改成明確 opt-in,避免大 PR 直接全開。@@ -0,0 +28,4 @@* @returns {Promise<void>} 流程正常走完(無嚴重問題)時 resolve;多數結束路徑會直接* 呼叫 `process.exit()` 結束程序,函式不會以回傳值回報審查結果。* @throws {Error} 內部未被個別 try/catch 攔截的未預期例外會向上拋出,* 由頂層 `main().catch(...)` 接住並以 `process.exit(1)` 結束。嚴重等級:🟡 警告
審查員:Maya
問題:這個 orchestrator 是整條 pipeline 的入口,但目前測試都停在零件層,沒有直接驗證
main()的關鍵分支:前置驗證失敗、bot 自動提交直接退出、diff 為空直接退出、JSON 驗證失敗退出、以及發現 critical 時的 exit 1。這些流程一旦接線錯了,單元測試還是可能全綠。建議:補一組
main()的整合測試,透過依賴注入或 mock 把runPreflight、getPRDiff、cloneRepo、postFindingsReview、validateJSONArrayFile、commitAndPush與process.exit全部接起來,逐一斷言上述 exit/降級路徑與 side effect 都有被觸發。@@ -0,0 +123,4 @@let auth;try {auth = JSON.parse(fs.readFileSync(authPath, 'utf8'));} catch (e) {嚴重等級:🔵 建議
審查員:Leo
問題:
clientVersion被硬編成0.142.5,這種版本字串沒有單一來源,時間一久幾乎一定會過期。到時候 preflight 會因為一個靜態常數失效,維護者還得回頭搜尋到底是哪裡卡住,排查成本很高。建議:把 client version 提成共用常數或環境變數,或改成由 CLI/設定檔提供;至少在集中位置定義一次,避免多處散落的魔術字串。
@@ -0,0 +51,4 @@export function parseBotReviewComment(body) {if (typeof body !== 'string' || !body.includes('**')) return null;const normalized = body.replace(/\r\n/g, '\n');const levelRaw = fieldValue(normalized, '嚴重等級') || fieldValue(normalized, '等級');嚴重等級:🔵 建議
審查員:Maya
問題:
groupConversations目前有測position與original_position,但沒有測new_position這個常見的 Gitea review comment 欄位。這代表如果 API 回來的是new_position,對話分組與 bot finding 對位是否正確,現在沒有被試煉過。建議:補一個只帶
new_position、不帶position的 comment fixture,驗證它能正確分組、line會用new_position,而且解析出的botFinding.location仍然是file:line。@@ -0,0 +88,4 @@if (!g.botFinding) {const finding = parseBotReviewComment(body);if (finding) g.botFinding = { ...finding, location: lineNum ? `${filePath}:${lineNum}` : filePath };}嚴重等級:🟡 警告
審查員:Mage
問題:同一個
path|line的多筆 bot comment 只會保留第一筆botFinding,後面的 finding 會被靜默丟掉。最小重現:同一行上有兩個不同角色或不同建議的 bot comment,reconcile 時只會帶回第一筆,另一筆不會進入 resolved/excluded/carried 清單。建議:不要把同一個
path|line的 thread 壓成單一 finding。至少要把同群組內所有 bot finding 都保留下來,或再加上穩定 fingerprint / comment id 做細分,避免同線多筆問題互相覆蓋。@@ -0,0 +166,4 @@}/*** 安全守衛:判定路徑是否為 repo 內的相對路徑(拒絕絕對路徑、Windows 磁碟機前綴與含 `..` 的路徑穿越)。嚴重等級:🟡 警告
審查員:Rogue
問題:這段為了產生約 41 行的
codeWindow,先把整個檔案內容從 Gitea 抓回來。大檔案時等於每個 open conversation 都在付整份檔案的網路與記憶體成本,實際只用到一小段片段,浪費量會跟檔案大小線性成長。建議:改成只取需要的區間或 hunk 內容,或至少在可用本地 repo 時改讀本機檔案再切窗;不要為了少量上下文把整份檔案搬進來。
@@ -0,0 +225,4 @@if (open.length === 0) {ok(`對話收斂完成: 關閉 comment=${closedCount} 已修復=0 誤報=0 仍成立=0`);return { ...EMPTY, closedCount };}嚴重等級:🟡 警告
審查員:Rogue
問題:這個
extractFileDiff一旦開始抓到目標檔案,就一路把後面的所有 diff 都塞進去,根本沒有在下一個diff --git停下來。結果本來只想餵單一檔案的提示,可能膨脹成接近整份 PR diff,每次補行號都在多燒 token 與傳輸時間。建議:在下一個
diff --git區塊出現時立刻結束,或直接只擷取目標檔案的 hunk 區段;不要把目標檔後面的 diff 全部一併帶進 prompt。@@ -0,0 +38,4 @@* 結果快取於模組層級(`cachedRoles`),同一程序生命週期內只讀檔一次;之後即使* 角色檔有變動也不會重新載入,需重啟程序才會生效。單一檔案解析失敗(壞 YAML、* 缺 frontmatter 等)只記錄警告並略過,不會中斷其他角色的載入。*嚴重等級:🟡 警告
審查員:Assassin
問題:角色 prompt 直接從目前 checkout 的
src/prompts/roles/*.md載入,等於把 system prompt 放在可被 PR 修改的位置。攻擊者只要改這些 markdown,就能改寫審查角色的指令,命令模型忽略漏洞、輸出空陣列,或把所有問題打成誤報。建議:把角色 prompt 移出可被審查分支影響的路徑,例如打包進發佈產物或從受保護分支、簽章校驗後載入;至少要在審查目標分支變更這些檔案時拒絕自動採用。
@@ -0,0 +178,4 @@'',role.body,]: ['你是 🛡️ Paladin(聖騎士),公正的裁判。不冤枉無辜的程式碼,也不放水。'];嚴重等級:🔵 建議
審查員:Maya
問題:
buildVerdictPrompt是防守方裁決流程的 prompt 契約,但目前沒有任何測試確認它會帶入預設 Paladin 人設、exclusionHint、以及confirmed / false_positive的輸出格式。這類 prompt 一旦格式偏掉,後面的對話收斂會很難診斷。建議:補一個針對
buildVerdictPrompt的測試,分別驗證:未傳 role 時會使用 Paladin 預設敘述、傳入防守角色時會套用角色 body、傳入exclusionHint時會原樣出現在 prompt 中,且回傳規格仍要求confirmed | false_positive。🤖 AI Code Review 團隊
AI Code Review 統計
🤖 AI 助理使用量
本次審查(codex / gpt-5.4-mini,共 11 次呼叫)
剩餘可用
剩餘可用:無法計算百分比(未支援 codex 額度查詢)