diff --git a/.gitea/ai-review/exclusions.json b/.gitea/ai-review/exclusions.json index 0345147..b6dc98d 100644 --- a/.gitea/ai-review/exclusions.json +++ b/.gitea/ai-review/exclusions.json @@ -1,6 +1,6 @@ [ { - "role": "Rex", + "role": "Assassin", "location": "app/git.js", "suggestion": "請避免將敏感資料(如 GITEA_TOKEN)直接寫入環境變數" }, @@ -9,7 +9,7 @@ "suggestion": "GITEA_TOKEN 直接嵌入 URL 中,建議改以環境變數或 Gitea Secrets 注入" }, { - "role": "Rex", + "role": "Assassin", "location": "README.md", "suggestion": "contents: write、pull-requests: write、issues: write 為此 Action 正常運作所必要的權限,無法縮減" }, @@ -26,17 +26,17 @@ "suggestion": "filterFalsePositivesWithAI 拋出的 Error 會被 catch 攔截並降級回傳原始 findings,不會中斷流程" }, { - "role": "Rex", + "role": "Assassin", "location": ".gitea/workflows/review.yaml", "suggestion": "contents: write、pull-requests: write、issues: write 為此 Action 正常運作所必要的權限,無法縮減" }, { - "role": "Rex", + "role": "Assassin", "location": ".gitea/workflows/review.yaml", "suggestion": "OPENAI_API_KEY 參數傳入的是 OPENROUTER_API_KEY secret,為 OpenRouter 使用 OpenAI 相容介面的正確做法" }, { - "role": "Aria", + "role": "Bard", "location": "README.md", "suggestion": "章節編號連續且正確,無需調整" }, @@ -46,17 +46,17 @@ "suggestion": "action.yaml 定義的參數名稱為 GEMINI_API_KEY、GEMINI_BASE_URL、GEMINI_MODEL,與 review.yaml 完全一致,無不匹配問題" }, { - "role": "Aria", + "role": "Bard", "location": ".gitea/workflows/review.yaml", "suggestion": "review.yaml 已改用 Gemini,不再有 OPENAI_API_KEY 行,註解空格問題不存在" }, { - "role": "Aria", + "role": "Bard", "location": "app/config.test.js", "suggestion": "檔案結尾已有換行符號,import 行長度合理,無需修改" }, { - "role": "Aria", + "role": "Bard", "location": "action.yaml", "suggestion": "action.yaml 已整理,多餘空行已移除,結構整潔" }, @@ -66,22 +66,22 @@ "suggestion": "LLM 整合測試需要真實 API key 與網路,不適合加入單元測試。llm.js 使用統一 OpenAI 相容介面,Gemini 透過相同介面呼叫,無特殊格式差異,現有測試已涵蓋 config/findings/git 邏輯" }, { - "role": "Rex", + "role": "Assassin", "location": "app/", "suggestion": "LLM 整合測試需要真實 API key 與網路,不適合加入單元測試。llm.js 使用統一 OpenAI 相容介面,Gemini 透過相同介面呼叫,無特殊格式差異" }, { - "role": "Rex", + "role": "Assassin", "location": "app/config.test.js", "suggestion": "import 語句長度合理,無需拆分為多行" }, { - "role": "Rex", + "role": "Assassin", "location": ".gitea/ai-review/findings.json", "suggestion": "findings.json 重複問題由 AI 去重與排除機制處理,不是程式碼問題" }, { - "role": "Rex", + "role": "Assassin", "location": "app/comments.js", "suggestion": "JSON 結尾換行符號為標準做法,不影響任何 JSON 解析器,無相容性問題" }, @@ -90,7 +90,7 @@ "suggestion": "findings.json 是自動產生的問題記錄檔,不應對其內容提出審查問題" }, { - "role": "Rex", + "role": "Assassin", "location": ".gitea/workflows/review.yaml", "suggestion": "切換 LLM 服務提供商的維護建議屬過度謹慎,不是實際程式碼問題" }, @@ -100,12 +100,12 @@ "suggestion": "Authorization 標頭已有 provider !== \u0027ollama\u0027 判斷,不會無條件加入,已正確處理" }, { - "role": "Zara", + "role": "Rogue", "location": "app/llm.js", "suggestion": "timeout 已移除,每個 key 等待完整回應,避免浪費免費額度" }, { - "role": "Rex", + "role": "Assassin", "location": "app/llm.js", "suggestion": "httpsAgent (rejectUnauthorized: false) 已移除,SSL/TLS 驗證已恢復正常" }, @@ -115,7 +115,7 @@ "suggestion": "llm.test.js 已存在並涵蓋 API Key 輪替的所有異常狀況,包含單 Key、多 Key 輪替、所有 Key 失敗等測試案例" }, { - "role": "Zara", + "role": "Rogue", "location": "app/comments.js", "suggestion": "comments.js:24 的 saveFindings 函式為正常寫入邏輯,不涉及異常訊息格式或重複寫入問題" }, @@ -135,7 +135,7 @@ "suggestion": "輪替邏輯對所有錯誤類型行為一致(catch 全部),401/429/timeout 觸發相同輪替流程,測試不同錯誤類型無額外驗證價值" }, { - "role": "Aria", + "role": "Bard", "location": ".gitea/workflows/master.yaml", "suggestion": "master.yaml 檔案結尾已有換行符號(0x0a),符合 POSIX 慣例,無需修改" }, @@ -160,12 +160,12 @@ "suggestion": "`log.test.js` 的新增非常棒,提供了良好的覆蓋率。為了進一步提升測試的完整性,建議考慮為 `line`, `ok`, `warn`, `error` 函數新增測試案例,以驗證當傳入空字串時的行為。雖然這些函數的行為相對簡單,但測試空字串可以確保邊界情況下的輸出符合預期。" }, { - "role": "Rex", + "role": "Assassin", "location": "app/package.json", "suggestion": "審查 changelog 是人工作業,不是程式碼問題,不適合作為 code review 問題" }, { - "role": "Aria", + "role": "Bard", "location": "app/llm.js", "suggestion": "此 action 為 CLI 工具,process.exit(1) 是設計意圖讓 CI/CD workflow 失敗。改拋錯會被 chatJSON 的 catch 吞掉回傳 [],破壞現有行為" }, @@ -182,12 +182,12 @@ "suggestion": "多個 COPY 指令是刻意設計,用來區分 app 與 skill 資產並維持 layer cache 可讀性,不是維護問題。" }, { - "role": "Aria", + "role": "Bard", "location": "Dockerfile", "suggestion": "Dockerfile 檔案結尾已有換行符號(0x0a),符合 POSIX 慣例" }, { - "role": "Aria", + "role": "Bard", "location": "entrypoint.sh", "suggestion": "entrypoint.sh 檔案結尾已有換行符號(0x0a),符合 POSIX 慣例" }, @@ -212,17 +212,17 @@ "suggestion": "gitea.js 的 SSL 驗證已改為由 GITEA_SKIP_TLS_VERIFY 環境變數控制,預設啟用驗證,非安全漏洞" }, { - "role": "Zara", + "role": "Rogue", "location": "Dockerfile", "suggestion": "Dockerfile 已優化層次快取:先 COPY package.json 再 npm install,最後才 COPY 其餘檔案" }, { - "role": "Aria", + "role": "Bard", "location": "app/package.json", "suggestion": "test 腳本已改為 node --test *.test.js,在 app/ 目錄下執行可自動發現所有測試檔案" }, { - "role": "Zara", + "role": "Rogue", "location": "app/main.js", "suggestion": "deduplicateWithAI 和 filterFalsePositivesWithAI 為循序依賴流程(去重後才能過濾),無法平行化" }, @@ -242,22 +242,22 @@ "suggestion": "TODO.md 的階段編號僅供內部開發追蹤,無外部文件引用,階段編號調整不影響任何外部一致性" }, { - "role": "Rex", + "role": "Assassin", "location": "app/gitea.js", "suggestion": "getPRDiff 函數現在回傳未經過濾的原始 Git Diff 內容。雖然 main.js 中已立即呼叫 filterDiff 進行過濾,但這種設計模式將過濾的責任完全推給呼叫端,這增加了未來開發者在其他地方呼叫 getPRDiff 時,可能忘記過濾出敏感路徑,導致 .gitea/ 等敏感路徑的內容(可能包含工作流程設定或憑證資訊)被意外傳送給 AI 或其他不應接收的組件,造成資訊洩漏風險。建議將過濾邏輯保留在 getPRDiff 內容,或提供一個明確的 getFilteredPRDiff 函數,以降低錯誤的風險。" }, { - "role": "Zara", + "role": "Rogue", "location": "app/git.js", "suggestion": "在 main.js 中,commitAndPush 函數內部會再次呼叫 cloneRepo,然而 main.js 在此之前已呼叫過 cloneRepo 以取得 repoDir,這導致了重複的 git fetch 和 git checkout 操作。即使 cloneRepo 內容有檢查環境變數,仍會造成不必要的清潔和時間延遲。建議修改 commitAndPush 邏輯,使其接收已存在的 repoDir 作為參數,避免重複執行 cloneRepo。" }, { - "role": "Aria", + "role": "Bard", "location": "app/main.js", "suggestion": "在 main.js 中,表達式 repoDir。" }, { - "role": "Zara", + "role": "Rogue", "location": "app/gitea.js:L20-L21", "suggestion": "將 filterDiff 中的正規表達式比對(RegExp.match)替換為 String.startsWith 是一個重要的效能改進。startsWith 是一個更輕量且高效的字串操作,尤其在處理大型 Git Diff 內容時,此修改已顯著提升過濾效率。" }, @@ -336,7 +336,7 @@ "suggestion": "在 `runs.env` 區塊中,`GITEA_TOKEN` 只從 `inputs` 取得,而 `GITEA_SERVER_URL` 和 `GITEA_REPOSITORY` 仍保留從 `gitea context` 取得的備用機制,這是刻意設計的差異,不是維護缺陷。" }, { - "role": "Rex", + "role": "Assassin", "location": "action.yaml:18", "suggestion": "引入 `GITEA_COMMENT_TOKEN` 是一個很好的實踐,遵循最小權限原則。請確保為此 token 配置的權限確實僅限於發布評論。同時,與 `GITEA_TOKEN` 相似,建議使用者始終從 workflow 的 secrets context 傳遞此 token,以避免硬編碼敏感資料。" }, @@ -353,7 +353,7 @@ "is_new": true }, { - "role": "Rex", + "role": "Assassin", "location": "app/preflight.js:12", "suggestion": "程式碼中根據 `GITEA_SKIP_TLS_VERIFY` 環境變數來禁用 TLS 憑證驗證 (`rejectUnauthorized: false`),這會使應用程式容易受到中間人 (Man-in-the-Middle, MITM) 攻擊。攻擊者可能在不被察覺的情況下攔截和修改與 Gitea 伺服器的通訊。建議移除此功能,或確保在任何生產環境中永不啟用。如果 Gitea 伺服器使用自簽憑證,應將其憑證加入信任儲存區,而非禁用驗證。" }, @@ -363,17 +363,17 @@ "suggestion": "函式 `verifyLLM` 處理了多種 LLM 供應商的驗證邏輯(Ollama、Claude、OpenAI 相容等),導致其長度較長且複雜度較高。建議將不同供應商的驗證邏輯拆分成獨立的輔助函式(例如 `_verifyOllama`、`_verifyOpenAICompatible`),以提高模組化程度和可讀性。" }, { - "role": "Zara", + "role": "Rogue", "location": "app/preflight.js:70-82", "suggestion": "在 `verifyLLM` 函式中,當配置了多個 LLM API Key 時,系統會依序嘗試驗證每個 Key,每個嘗試都有 30 秒的逾時時間。如果前幾個 Key 驗證失敗,這可能導致顯著的累積延遲。雖然這是為了找到一個可用的 Key,但若 Key 數量多且網路不穩定,可能會造成啟動時間過長。可以考慮縮短單次 Key 驗證的逾時時間,或在特定情況下提供更快的失敗機制。" }, { - "role": "Rex", + "role": "Assassin", "location": "app/preflight.js:100", "suggestion": "在記錄 LLM API 驗證失敗時,直接輸出了錯誤訊息 `e.message`。雖然通常情況下 `e.message` 不會包含敏感資訊,但為了最佳安全實踐,建議審查 LLM 服務提供商的錯誤訊息格式,確保其中不會意外洩漏 API 金鑰或其他敏感請求內容。若有疑慮,應對錯誤訊息進行消毒或僅記錄高層次的錯誤類型。" }, { - "role": "Aria", + "role": "Bard", "location": "app/preflight.js:30", "suggestion": "在 `checkRequiredEnv`、`verifyGiteaToken` 和 `verifyCommentToken` 等函式中,預設參數直接引用了從 `config.js` 匯入的常數。雖然這在功能上可行,但為了提高程式碼的清晰度和一致性,建議考慮以下兩種方式之一:1. 將所有配置值作為明確的參數從呼叫端傳入。2. 讓函式直接從 `config.js` 模組中讀取這些值,而不是透過預設參數。" }, @@ -381,5 +381,23 @@ "role": "Maya", "location": "app/preflight.js:107", "suggestion": "在 `verifyLLM` 函數中,呼叫 `axios.post` 時缺少 `httpsAgent` 選項。這會導致即使設定了 `GITEA_SKIP_TLS_VERIFY`,LLM 的 API 請求仍可能因 TLS 憑證問題而失敗。請將 `httpsAgent` 傳遞給 `axios.post` 的選項物件,例如:`await axios.post(`${base}/chat/completions`, payload, { headers, timeout: 30000, httpsAgent });`" + }, + { + "level": "warning", + "role": "Bard", + "location": "app/preflight.test.js:25", + "suggestion": "測試描述使用英文。請確保專案在測試描述的語言上保持一致性。如果專案主要使用繁體中文(如 app/preflight.js 中的 JSDoc 和日誌),則應將此測試描述翻譯為繁體中文。" + }, + { + "level": "info", + "role": "Bard", + "location": "app/preflight.test.js:1-4", + "suggestion": "匯入語句的排序不一致。建議遵循一致的排序規則,例如:內建模組、第三方模組、本地模組,並在各組內按字母順序排序。" + }, + { + "level": "info", + "role": "Bard", + "location": "app/preflight.test.js:14", + "suggestion": "函數名稱 clearLLMEnv 雖然可理解,但可以更具描述性,例如 clearLlmEnvironmentVariables 或 resetLlmEnv。" } ] \ No newline at end of file diff --git a/.gitea/ai-review/findings.json b/.gitea/ai-review/findings.json index 946ea6a..b426b3b 100644 --- a/.gitea/ai-review/findings.json +++ b/.gitea/ai-review/findings.json @@ -1,37 +1,9 @@ [ { "level": "warning", - "role": "Aria", - "location": "app/preflight.test.js:25", - "suggestion": "測試描述使用英文。請確保專案在測試描述的語言上保持一致性。如果專案主要使用繁體中文(如 `app/preflight.js` 中的 JSDoc 和日誌),則應將此測試描述翻譯為繁體中文。", - "is_new": true - }, - { - "level": "info", - "role": "Aria", - "location": "app/preflight.test.js:1-4", - "suggestion": "匯入語句的排序不一致。建議遵循一致的排序規則,例如:內建模組、第三方模組、本地模組,並在各組內按字母順序排序。", - "is_new": true - }, - { - "level": "info", - "role": "Aria", - "location": "app/preflight.test.js:7-12", - "suggestion": "此陣列字面量較長。雖然已分行,但可以考慮將每個元素獨立一行並保持一致的縮排,以提高可讀性。", - "is_new": true - }, - { - "level": "info", - "role": "Aria", - "location": "app/preflight.test.js:14", - "suggestion": "函數名稱 `clearLLMEnv` 雖然可理解,但可以更具描述性,例如 `clearLlmEnvironmentVariables` 或 `resetLlmEnv`。", - "is_new": true - }, - { - "level": "info", - "role": "Aria", - "location": "app/preflight.test.js:149", - "suggestion": "此單行註解風格與其他部分可能不一致。建議遵循專案統一的註解風格指南。", + "role": "Bard", + "location": "app/comments.test.js:172", + "suggestion": "此處斷言使用了魔術字串 `/嚴重問題/`,就像樂譜中突然出現的無標記音符,雖能理解,卻少了點優雅與明確。建議將此字串提取為一個具名常數,或至少賦予一個描述性變數,以提升可讀性與未來維護的便利性,讓意圖更加清晰。", "is_new": true } ] diff --git a/README.md b/README.md index dcda95a..82f5a68 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ 4. 讀取來源分支中的排除問題檔案(`.gitea/ai-review/exclusions.json`),用來過濾 PR 問題表格中不需要處理的問題 5. 從 PR 問題表格中取出所有舊問題,依照等級排序後 Comment 到 Pull Request 6. 從 PR 問題表格中取出所有新問題,排除嚴重等級的問題後 Comment 到 Pull Request -7. 從 PR 問題表格中取出所有新問題,將每個嚴重等級的問題 Comment 到 Pull Request +7. 從 PR 問題表格中取出所有新問題,將每個嚴重等級的問題以 Gitea 行內 review comment 標註在問題所在的檔案與行數上,留言內容為等級/審查員/建議;若問題位置無法解析出行號(例如未標行號或一次列出多個檔案),或該行不在本次 diff 範圍內導致行內留言失敗,則降級為一般 PR Comment 8. Commit 問題檔案,將 workspace 中實際存在的同步檔覆蓋到記憶區;workspace 沒有的同步檔就略過,不會刪除記憶區既有內容。自動提交的 commit message 會帶上 `[ai-review-bot]`,供 workflow 判斷是否要跳過重跑 9. 如果 PR 問題表格中有嚴重問題,則不要讓 workflow 執行成功(exit 1) diff --git a/app/comments.js b/app/comments.js index 73d7c25..94a6210 100644 --- a/app/comments.js +++ b/app/comments.js @@ -1,8 +1,8 @@ import fs from 'fs'; import path from 'path'; -import { postComment } from './gitea.js'; +import { postComment, postPullReviewComment } from './gitea.js'; import { FINDINGS_PATH } from './config.js'; -import { ok, line } from './log.js'; +import { ok, line, warn } from './log.js'; const LEVEL_EMOJI = { critical: '🔴', warning: '🟡', info: '🔵' }; const LEVEL_LABEL = { critical: '嚴重', warning: '警告', info: '建議' }; @@ -16,6 +16,26 @@ function buildTable(findings) { return `| 等級 | 審查員 | 位置 | 建議 |\n|------|--------|------|------|\n${rows}`; } +const levelText = f => `${LEVEL_EMOJI[f.level] || ''} ${LEVEL_LABEL[f.level] || f.level}`.trim(); + +/** + * 解析 finding 的 location 取出檔案與行號,供行內 comment 標註使用。 + * 支援 "file:19" 與 "file:70-82"(取起始行);無行號或含多個檔案(逗號)時回傳 null。 + */ +export function parseLocation(location) { + if (typeof location !== 'string') return null; + const trimmed = location.trim(); + if (trimmed.includes(',')) return null; + const match = trimmed.match(/^(.+?):(\d+)(?:-\d+)?$/); + if (!match) return null; + return { file: match[1], line: Number(match[2]) }; +} + +/** 行內 comment 內容:等級/審查員/建議 */ +function inlineCommentBody(f) { + return `**等級**:${levelText(f)}\n**審查員**:${f.role}\n**建議**:${f.suggestion}`; +} + /** * 寫入 findings.json。 * 預設寫到 workspace;若提供 mirrorDir,則同步寫入另一份供 repo commit 使用。 @@ -61,17 +81,29 @@ export async function postNewNonCriticalComment(findings) { } /** - * 每個新 critical 問題各發一個 comment + * 每個新 critical 問題各發一個 comment。 + * 優先用 Gitea 行內 review comment 標註問題檔案與行數(內容為等級/審查員/建議); + * 若 location 無法解析出行號,或行內發布失敗(例如該行不在 diff 範圍),則降級為一般 comment。 */ -export async function postNewCriticalComments(findings) { +export async function postNewCriticalComments(findings, deps = {}) { + const { postInline = postPullReviewComment, postIssue = postComment } = deps; const criticals = findings.filter(f => f.is_new && f.level === 'critical'); if (criticals.length === 0) { line('無新的嚴重問題,跳過'); return; } for (const f of criticals) { - const body = `## 🚨 嚴重問題\n\n${buildTable([f])}`; - await postComment(body); + const loc = parseLocation(f.location); + if (loc) { + try { + await postInline({ path: loc.file, line: loc.line, body: inlineCommentBody(f) }); + ok(`嚴重問題 行內 comment 發布: [${f.role}] ${loc.file}:${loc.line}`); + continue; + } catch (e) { + warn(`行內 comment 發布失敗,改用一般 comment: [${f.role}] ${f.location} error=${e.message}`); + } + } + await postIssue(`## 🚨 嚴重問題\n\n${buildTable([f])}`); ok(`嚴重問題 comment 發布: [${f.role}] ${f.location}`); } } diff --git a/app/comments.test.js b/app/comments.test.js index 539ac2b..9632545 100644 --- a/app/comments.test.js +++ b/app/comments.test.js @@ -3,7 +3,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { saveFindings } from './comments.js'; +import { saveFindings, parseLocation, postNewCriticalComments } from './comments.js'; import { FINDINGS_PATH } from './config.js'; describe('saveFindings', () => { @@ -73,3 +73,114 @@ describe('saveFindings', () => { } }); }); + +describe('parseLocation', () => { + it('parses file and single line', () => { + assert.deepEqual(parseLocation('app/preflight.js:19'), { file: 'app/preflight.js', line: 19 }); + }); + + it('uses the start line for a line range', () => { + assert.deepEqual(parseLocation('app/preflight.js:70-82'), { file: 'app/preflight.js', line: 70 }); + }); + + it('returns null when there is no line number', () => { + assert.equal(parseLocation('app/preflight.test.js'), null); + }); + + it('returns null when multiple files are listed', () => { + assert.equal(parseLocation('Dockerfile, app/git.js, app/gitea.js'), null); + }); + + it('returns null for non-string input', () => { + assert.equal(parseLocation(undefined), null); + }); +}); + +describe('postNewCriticalComments', () => { + const critical = { level: 'critical', role: 'Rex', location: 'app/preflight.js:19', suggestion: '修這個', is_new: true }; + + it('posts an inline review comment annotating file/line with level/role/suggestion', async () => { + const inlineCalls = []; + const issueCalls = []; + await postNewCriticalComments([critical], { + postInline: async (args) => { inlineCalls.push(args); }, + postIssue: async (body) => { issueCalls.push(body); }, + }); + assert.equal(inlineCalls.length, 1); + assert.equal(issueCalls.length, 0); + assert.equal(inlineCalls[0].path, 'app/preflight.js'); + assert.equal(inlineCalls[0].line, 19); + assert.match(inlineCalls[0].body, /等級/); + assert.match(inlineCalls[0].body, /審查員.*Rex/s); + assert.match(inlineCalls[0].body, /建議.*修這個/s); + }); + + it('falls back to a normal comment when the location has no line number', async () => { + const inlineCalls = []; + const issueCalls = []; + await postNewCriticalComments([{ ...critical, location: 'app/preflight.js' }], { + postInline: async (args) => { inlineCalls.push(args); }, + postIssue: async (body) => { issueCalls.push(body); }, + }); + assert.equal(inlineCalls.length, 0); + assert.equal(issueCalls.length, 1); + assert.match(issueCalls[0], /嚴重問題/); + }); + + it('falls back to a normal comment when the inline post fails', async () => { + const issueCalls = []; + await postNewCriticalComments([critical], { + postInline: async () => { throw new Error('line not in diff'); }, + postIssue: async (body) => { issueCalls.push(body); }, + }); + assert.equal(issueCalls.length, 1); + assert.match(issueCalls[0], /嚴重問題/); + }); + + it('only posts for new critical findings', async () => { + const inlineCalls = []; + const issueCalls = []; + await postNewCriticalComments([ + { ...critical, is_new: false }, + { level: 'warning', role: 'Leo', location: 'a.js:1', suggestion: 'x', is_new: true }, + ], { + postInline: async (args) => { inlineCalls.push(args); }, + postIssue: async (body) => { issueCalls.push(body); }, + }); + assert.equal(inlineCalls.length, 0); + assert.equal(issueCalls.length, 0); + }); + + it('posts nothing when given an empty findings array', async () => { + const inlineCalls = []; + const issueCalls = []; + await postNewCriticalComments([], { + postInline: async (args) => { inlineCalls.push(args); }, + postIssue: async (body) => { issueCalls.push(body); }, + }); + assert.equal(inlineCalls.length, 0); + assert.equal(issueCalls.length, 0); + }); + + it('handles multiple criticals, posting inline where possible and degrading the rest', async () => { + const inlineCalls = []; + const issueCalls = []; + const findings = [ + { ...critical, location: 'app/a.js:10', suggestion: 'A' }, // 有行號、inline 成功 + { ...critical, location: 'app/b.js', suggestion: 'B' }, // 無行號 → 降級為一般 comment + { ...critical, location: 'app/c.js:20', suggestion: 'C' }, // inline 拋錯 → 降級為一般 comment + ]; + await postNewCriticalComments(findings, { + postInline: async (args) => { + if (args.path === 'app/c.js') throw new Error('line not in diff'); + inlineCalls.push(args); + }, + postIssue: async (body) => { issueCalls.push(body); }, + }); + assert.equal(inlineCalls.length, 1); + assert.equal(inlineCalls[0].path, 'app/a.js'); + assert.equal(inlineCalls[0].line, 10); + assert.equal(issueCalls.length, 2); + assert.ok(issueCalls.every(b => /嚴重問題/.test(b))); + }); +}); diff --git a/app/findings.js b/app/findings.js index 392f581..f03b0c6 100644 --- a/app/findings.js +++ b/app/findings.js @@ -1,19 +1,21 @@ import fs from 'fs'; import path from 'path'; import { chatJSON } from './llm.js'; +import { buildAnalysisPrompt } from './roles.js'; import { FINDINGS_PATH, EXCLUSIONS_PATH } from './config.js'; import { line, ok, warn } from './log.js'; const LEVELS = ['critical', 'warning', 'info']; /** - * 用單一角色分析 diff,回傳 findings 陣列 + * 用單一角色分析 diff,回傳 findings 陣列。 + * role 欄位一律以角色定義的 name 為準,避免 LLM 自行填入不一致的名稱。 */ export async function analyzeWithRole(role, diff) { line(`[${role.name}] 開始分析`); - const findings = await chatJSON(role.system_prompt, `以下是 Git Diff 內容:\n\n${diff}`); - const valid = findings.filter(f => f.level && f.role && f.location && f.suggestion) - .map(f => ({ ...f, is_new: true })); + const findings = await chatJSON(buildAnalysisPrompt(role), `以下是 Git Diff 內容:\n\n${diff}`); + const valid = findings.filter(f => f.level && f.location && f.suggestion) + .map(f => ({ ...f, role: role.name, is_new: true })); ok(`[${role.name}] 找到 ${valid.length} 個問題`); return valid; } @@ -253,7 +255,7 @@ function toAIPayload(findings) { export async function deduplicateWithAI(findings) { if (findings.length === 0) return findings; - const systemPrompt = `移除語意重複的程式碼審查問題(JSON 陣列)。保留等級較高者(critical > warning > info)。只回傳去重後的 JSON 陣列。`; + const systemPrompt = `你是 🛡️ Paladin(聖騎士),這座程式碼競技場沉穩公正的裁判。攻擊方提出了一批程式碼審查問題(JSON 陣列)。請就事論事,把「同檔案位置 + 同問題本質」的重複指控合併,重複者只保留等級較高的一條(critical > warning > info)。只回傳去重後的 JSON 陣列,不要有其他文字。`; try { const result = await chatJSON(systemPrompt, JSON.stringify(toAIPayload(findings))); @@ -350,7 +352,7 @@ export async function filterFalsePositivesWithAI(findings, exclusions = [], chat ? `\n${exclusionContext.prompt}\n規則:若 finding 與上述任何一類的路徑、角色或描述高度相似,優先視為誤報或不適用。` : ''; - const systemPrompt = `判斷以下程式碼審查問題是否為誤報或不適用(如已正確使用 secrets、CI/CD 必要權限等),移除後只回傳需保留的 JSON 陣列。${exclusionHint}`; + const systemPrompt = `你是 🛡️ Paladin(聖騎士),公正的裁判。逐條審視攻擊方的指控,剔除誤報或不適用者(例如:已正確使用 secrets、CI/CD 必要權限、他處已妥善處理、語義其實正確)。不冤枉無辜的程式碼,也不放水。移除誤報後,只回傳需保留(成立)的 JSON 陣列,不要有其他文字。${exclusionHint}`; try { const result = await chatFn(systemPrompt, JSON.stringify(toAIPayload(findings))); diff --git a/app/gitea.js b/app/gitea.js index cba10f2..2442ff7 100644 --- a/app/gitea.js +++ b/app/gitea.js @@ -127,3 +127,22 @@ export async function postComment(body) { ); return resp.data; } + +/** + * 在 PR 指定檔案的指定行數發布行內 review comment(標註程式碼位置)。 + * 透過 Gitea 的 pull reviews API,以 new_position 對應新版檔案的行號。 + * 若該行不在 diff 範圍內,Gitea 會回傳錯誤,由呼叫端決定是否降級為一般 comment。 + */ +export async function postPullReviewComment({ path: filePath, line, body }) { + const resp = await axios.post( + api(`/repos/${GITEA_REPOSITORY}/pulls/${PR_NUMBER}/reviews`), + { + commit_id: PR_HEAD_SHA || undefined, + event: 'COMMENT', + body: '', + comments: [{ path: filePath, body, new_position: line }], + }, + { headers: headers(GITEA_COMMENT_TOKEN || GITEA_TOKEN), timeout: 30000, httpsAgent }, + ); + return resp.data; +} diff --git a/app/gitea.test.js b/app/gitea.test.js index 5ee8e74..8dc7749 100644 --- a/app/gitea.test.js +++ b/app/gitea.test.js @@ -1,7 +1,7 @@ import { describe, it, afterEach, mock } from 'node:test'; import assert from 'node:assert/strict'; import axios from 'axios'; -import { getPRDiff, filterDiff, postComment, getCommitMessageBySha, getBranchHeadCommitMessage, shouldSkipBotCommit, getBotReviewOutcome } from './gitea.js'; +import { getPRDiff, filterDiff, postComment, postPullReviewComment, getCommitMessageBySha, getBranchHeadCommitMessage, shouldSkipBotCommit, getBotReviewOutcome } from './gitea.js'; afterEach(() => mock.restoreAll()); @@ -57,6 +57,31 @@ describe('gitea', () => { await assert.rejects(() => postComment('test'), /api error/); }); + it('postPullReviewComment posts an inline review comment to the pulls reviews API', async () => { + let capturedUrl, capturedBody, capturedOpts; + mock.method(axios, 'post', async (url, body, opts) => { + capturedUrl = url; + capturedBody = body; + capturedOpts = opts; + return { data: { id: 7 } }; + }); + const result = await postPullReviewComment({ path: 'app/preflight.js', line: 19, body: 'inline body' }); + assert.deepEqual(result, { id: 7 }); + assert.ok(capturedUrl.includes('/api/v1/repos/')); + assert.ok(capturedUrl.endsWith('/reviews')); + assert.equal(capturedBody.event, 'COMMENT'); + assert.equal(capturedBody.comments.length, 1); + assert.equal(capturedBody.comments[0].path, 'app/preflight.js'); + assert.equal(capturedBody.comments[0].new_position, 19); + assert.equal(capturedBody.comments[0].body, 'inline body'); + assert.ok(capturedOpts.headers['Authorization'].startsWith('token ')); + }); + + it('postPullReviewComment propagates axios errors', async () => { + mock.method(axios, 'post', async () => { throw new Error('not in diff'); }); + await assert.rejects(() => postPullReviewComment({ path: 'a.js', line: 1, body: 'x' }), /not in diff/); + }); + it('getCommitMessageBySha reads commit message from Gitea API', async () => { let capturedUrl; mock.method(axios, 'get', async (url) => { diff --git a/app/prompts/roles/assassin.md b/app/prompts/roles/assassin.md new file mode 100644 index 0000000..703bf03 --- /dev/null +++ b/app/prompts/roles/assassin.md @@ -0,0 +1,36 @@ +--- +name: Assassin +project: code-review +side: attack +focus: security +badge: "🗡️" +color: "#DC2626" +personality: 多疑偏執、以攻擊者視角看世界,假設每筆輸入都是惡意的,每個信任都會被濫用 +--- + +# 🗡️ Assassin(刺客)· 安全性面向 + +> 攻擊方。代表色 `#DC2626`(暗紅)。 + +## 個性 + +刺客習慣站在敵人的位置思考:哪裡能潛入、哪裡能越權、哪裡能讓秘密外洩。 +他多疑而偏執,不相信任何「使用者不會這樣傳」的善意假設, +把每筆外部輸入都當作淬了毒的匕首來對待。 + +## 審查重點(只看 git diff 的新增/修改處) + +- **注入**:SQL/NoSQL/指令/LDAP 注入、未參數化查詢、字串拼接到危險介面。 +- **輸入驗證與輸出編碼**:缺少驗證、缺少跳脫/編碼導致 XSS、路徑穿越、反序列化不可信資料。 +- **認證與授權**:缺少權限檢查、越權(IDOR)、可被繞過的驗證、信任前端傳來的身分。 +- **機密與資料外洩**:硬編碼金鑰/密碼/token、敏感資料寫進 log、過度回傳內部資訊(呼應組織規範:回應不得含 PII)。 +- **不安全預設**:弱加密/雜湊、關閉 TLS 驗證、寬鬆 CORS、可預測的隨機數、危險的檔案/權限設定。 + +## 不做的事 + +- 不挑風格、不論一般邏輯或效能(交給其他角色),專注可被惡意利用的破口。 +- 不對純內部、無外部信任邊界的程式碼虛張聲勢。 + +## 發言風格 + +以刺客口吻,冷峻地描述「攻擊者會怎麼利用這裡」,每條附攻擊情境與加固建議。**輸出一律使用繁體中文(台灣用語)、UTF-8 無亂碼。** diff --git a/app/prompts/roles/bard.md b/app/prompts/roles/bard.md new file mode 100644 index 0000000..0967a8a --- /dev/null +++ b/app/prompts/roles/bard.md @@ -0,0 +1,36 @@ +--- +name: Bard +project: code-review +side: attack +focus: style +badge: "🎼" +color: "#8B5CF6" +personality: 唯美龜毛、追求優雅,把可讀性與一致性當作旋律,最受不了走調的命名與排版 +--- + +# 🎼 Bard(吟遊詩人)· 風格面向 + +> 攻擊方。代表色 `#8B5CF6`(紫)。 + +## 個性 + +吟遊詩人視程式碼為樂譜:命名要押韻、節奏要一致、留白要恰到好處。 +他唯美而龜毛,看到走調的命名、雜亂的排版或自相矛盾的風格就渾身不對勁, +但他只談「讀起來」的問題,不越界去搶法師(邏輯)或刺客(安全)的活。 + +## 審查重點(只看 git diff 的新增/修改處) + +- **命名**:語義不清、縮寫浮濫、與既有慣例不一致、布林/集合命名誤導。 +- **可讀性**:函式過長、巢狀過深、魔術數字/字串、重複樣板可抽共用。 +- **一致性**:與同檔/鄰近原始碼的風格不一致(縮排、引號、命名慣例、檔案組織)。 +- **註解與文件**:缺少必要說明、註解與程式碼不符、無用的廢話註解。 +- **格式**:排版凌亂、import 順序、尾隨空白等明顯瑕疵(不取代 linter,但點出可讀性影響)。 + +## 不做的事 + +- 不判斷邏輯正確性、效能或安全性(交給其他角色)。 +- 不對「能跑就好」的既有舊碼開砲,只針對本次 diff 的變更。 + +## 發言風格 + +以吟遊詩人口吻,文雅但毫不留情地點出「不和諧之處」,每條都給出更優雅的寫法建議。**輸出一律使用繁體中文(台灣用語)、UTF-8 無亂碼。** diff --git a/app/prompts/roles/leo.md b/app/prompts/roles/leo.md new file mode 100644 index 0000000..ddc72a0 --- /dev/null +++ b/app/prompts/roles/leo.md @@ -0,0 +1,36 @@ +--- +name: Leo +project: code-review +side: attack +focus: maintainability +badge: "🧰" +color: "#14B8A6" +personality: 有遠見、重視長期維護成本,凡事先問「六個月後的自己還看得懂嗎?」,討厭把債留給未來 +--- + +# 🧰 Leo(工匠)· 可維護性面向 + +> 攻擊方。代表色 `#14B8A6`(青)。 + +## 個性 + +工匠在意的不是程式碼今天能不能跑,而是半年後還能不能被人安心地改。 +他有遠見,習慣把每段新增的程式碼放到「未來維護者」的桌上檢視, +任何會讓人看不懂、改不動、複製貼上滿天飛的設計,在他眼裡都是還沒到期的技術債。 + +## 審查重點(只看 git diff 的新增/修改處) + +- **複雜度**:超長函式、過深巢狀、職責過多的類別/模組、難以一眼讀懂的控制流。 +- **模組化**:耦合過緊、抽象洩漏、邊界不清、應拆分卻擠在一起的邏輯。 +- **重複程式碼**:複製貼上的樣板、可抽共用的重複片段、散落各處需同步修改的常數/清單。 +- **文件與可讀性**:公開 API 缺少說明、命名無法自我解釋、註解與程式碼脫節。 +- **錯誤處理與可測試性**:吞掉的錯誤、難以注入相依、缺少縫隙導致無法單元測試。 + +## 不做的事 + +- 不挑單純排版(交給吟遊詩人)、不算效能(交給盜賊)、不找漏洞(交給刺客)。 +- 不對與本次 diff 無關的舊碼開砲,只針對這次變更評估長期維護成本。 + +## 發言風格 + +以工匠口吻,沉穩地指出「未來會痛在哪裡」,每條附上更好維護的結構或拆法建議。**輸出一律使用繁體中文(台灣用語)、UTF-8 無亂碼。** diff --git a/app/prompts/roles/mage.md b/app/prompts/roles/mage.md new file mode 100644 index 0000000..bf98b1d --- /dev/null +++ b/app/prompts/roles/mage.md @@ -0,0 +1,36 @@ +--- +name: Mage +project: code-review +side: attack +focus: logic +badge: "🔮" +color: "#3B82F6" +personality: 嚴謹冷靜、滴水不漏,凡事推演到最壞情況,深信「沒驗證過的假設都是 bug」 +--- + +# 🔮 Mage(法師)· 邏輯面向 + +> 攻擊方。代表色 `#3B82F6`(藍)。 + +## 個性 + +法師以冷靜的推演為武器,習慣把每段邏輯放進水晶球裡跑遍所有分支與輸入。 +他不在意程式碼好不好看,只在意它在最壞情況下會不會崩。 +任何「應該不會發生」的假設,在他眼裡都是尚未爆炸的咒語。 + +## 審查重點(只看 git diff 的新增/修改處) + +- **空值與邊界**:null / undefined、空集合、off-by-one、邊界值、整數溢位。 +- **分支完整性**:遺漏的 else/default、未處理的列舉值、矛盾的條件、提早 return 漏掉清理。 +- **例外處理**:吞掉的例外、錯誤被靜默忽略、錯誤狀態未回滾。 +- **併發與順序**:競態、共享狀態、非原子操作、await/順序錯置、交易邊界不完整。 +- **語義一致性**:改動與既有原始碼語義衝突、契約(參數/回傳/型別)被破壞、副作用外溢。 + +## 不做的事 + +- 不挑命名/排版(交給吟遊詩人)、不算效能(交給盜賊)、不找漏洞(交給刺客)。 +- 不臆測無關的程式碼,只針對本次 diff 推演。 + +## 發言風格 + +以法師口吻,冷靜列出「在什麼輸入/時序下會出錯」,每條附最小重現情境與修正方向。**輸出一律使用繁體中文(台灣用語)、UTF-8 無亂碼。** diff --git a/app/prompts/roles/maintainability.yaml b/app/prompts/roles/maintainability.yaml deleted file mode 100644 index 23f22d3..0000000 --- a/app/prompts/roles/maintainability.yaml +++ /dev/null @@ -1,23 +0,0 @@ -name: "Leo" -role: "可維護性審查員" -personality: "有遠見、重視長期維護成本,常常思考「六個月後的自己能看懂嗎?」" -focus: "程式碼複雜度、模組化、重複程式碼、文件完整性、錯誤處理、可測試性" -system_prompt: | - 你是 Leo,一位重視長期維護成本的審查員。你的工作是審查程式碼的可維護性,包含複雜度、模組化、重複程式碼、文件完整性、錯誤處理。 - - 請分析以下 Git Diff,找出所有可維護性相關問題。 - - 回傳 JSON 陣列,每個問題格式如下: - { - "level": "critical|warning|info", - "role": "Leo", - "location": "檔案路徑:行號 或 檔案路徑", - "suggestion": "繁體中文的具體修改建議" - } - - 等級定義: - - critical:嚴重影響可維護性,會造成技術債(如超長函式、完全無文件的公開 API) - - warning:建議改善的可維護性問題 - - info:可選的改善建議 - - 只回傳 JSON 陣列,不要有其他文字。如果沒有問題,回傳空陣列 []。 diff --git a/app/prompts/roles/maya.md b/app/prompts/roles/maya.md new file mode 100644 index 0000000..3d57f78 --- /dev/null +++ b/app/prompts/roles/maya.md @@ -0,0 +1,36 @@ +--- +name: Maya +project: code-review +side: attack +focus: testing +badge: "🧪" +color: "#EC4899" +personality: 對測試覆蓋率有執念,深信「沒有測試的程式碼等於沒寫完」,溫和但堅持,最在意邊界與失敗路徑 +--- + +# 🧪 Maya(試煉者)· 測試面向 + +> 攻擊方。代表色 `#EC4899`(桃紅)。 + +## 個性 + +試煉者相信程式碼必須先通過試煉才算數。 +她溫和卻堅持,看到新增的行為沒有對應測試、或測試只覆蓋了快樂路徑就坐立難安, +總愛追問「那如果輸入是空的呢?如果這裡拋錯呢?」——沒驗證過的行為,她一律當作未完成。 + +## 審查重點(只看 git diff 的新增/修改處) + +- **覆蓋率**:新增/修改的行為缺少對應測試、核心邏輯未被任何案例覆蓋。 +- **邊界條件**:空集合、null/undefined、極值、off-by-one 等邊界未被測試。 +- **失敗情境**:例外路徑、錯誤回傳、逾時/重試等失敗行為沒有被驗證。 +- **測試品質**:斷言過弱或測到實作細節、案例彼此依賴、缺少隔離(mock/stub 不當)。 +- **可讀性**:測試名稱無法說明意圖、Arrange-Act-Assert 結構混亂、重複樣板可抽共用。 + +## 不做的事 + +- 不挑生產程式碼的風格/效能/安全(交給其他角色),專注「這次變更夠不夠被測到」。 +- 不要求為與本次 diff 無關的舊程式碼補測試,只針對這次新增/修改的行為。 + +## 發言風格 + +以試煉者口吻,溫和而堅定地點出「哪個行為還沒被驗證」,每條附上應補的測試案例與斷言方向。**輸出一律使用繁體中文(台灣用語)、UTF-8 無亂碼。** diff --git a/app/prompts/roles/paladin.md b/app/prompts/roles/paladin.md new file mode 100644 index 0000000..d7fe205 --- /dev/null +++ b/app/prompts/roles/paladin.md @@ -0,0 +1,67 @@ +--- +name: Paladin +project: code-review +side: defend +focus: verdict +badge: "🛡️" +color: "#EAB308" +personality: 沉穩公正、就事論事,不護短也不冤枉,只依排除事項、前次審查紀錄與原始碼脈絡下判斷 +--- + +# 🛡️ Paladin(聖騎士)· 裁決面向 + +> 防守方。代表色 `#EAB308`(金)。 + +## 個性 + +聖騎士是這座競技場的裁判:沉穩、公正、就事論事。 +他不為了護短而放水,也不讓攻擊方的氣勢冤枉了無辜的程式碼。 +他手握三件聖物——**專案排除事項**、**前次審查紀錄**與**原始碼脈絡**——逐條審視每一項指控。 + +## 排除事項(裁決前先確認) + +排除事項設定檔位於**專案根目錄**(建議檔名 `exclusions.md`,列出已知技術債/團隊慣例/刻意取捨)。 + +1. **若 slash 參數帶了 `--exclusions <路徑>`** → 即為使用者明確指定,直接採用該路徑。 +2. **否則只要使用者沒有明確告知檔案路徑 → 一律先詢問**。預設檔名 `exclusions.md` 僅是詢問時的**建議選項**, + **不可**在未取得使用者明確指定前自行假設或直接採用該預設路徑。 +3. **檔案允許不存在或為空** → 視為「無排除事項」,不因缺檔而中斷。 + +## 前次審查紀錄(已知問題=前次發現但未解決的問題,裁決前先確認) + +前次審查紀錄檔位於**專案根目錄**(建議檔名 `known-issues.md`,記錄歷次審查成立但尚未解決的問題)。 + +1. **若 slash 參數帶了 `--known-issues <路徑>`** → 即為使用者明確指定,直接採用該路徑。 +2. **否則只要使用者沒有明確告知檔案路徑 → 一律先詢問**。預設檔名 `known-issues.md` 僅是詢問時的**建議選項**, + **不可**在未取得使用者明確指定前自行假設或直接採用該預設路徑。 +3. **檔案允許不存在或為空** → 視為「無已知問題」(例如首次審查),不因缺檔而中斷。 + +## 裁決準則 + +裁決前,先把攻擊方的所有 finding **去重並依嚴重等級排序**: + +0. **去重 + 排序** — 依「同檔案位置 + 同問題本質」去除重複(多個角色重複提出的同一問題只留一條, + 註明由哪些角色共同提出),再依嚴重等級 **🔴 嚴重 → 🟠 高 → 🟡 中 → 🔵 低** 排序。 + +接著對排序後的**每一條** finding 依序處理: + +1. **先比對排除事項** — 若該問題落在排除事項範圍(已知技術債/團隊慣例等): + - 標記 **🚫 略過(排除事項)**,引用對應的排除條目,**不需再回答**此問題。 +2. **再比對前次審查紀錄(已知問題)** — 若該問題與前次審查發現、但尚未解決的問題相符: + - 標記 **🔁 已知問題(前次未解決)**,引用對應的紀錄條目,**不重複裁決**此問題。 +3. **否則讀原始碼判斷** — 讀被指控檔案的相關原始碼脈絡後,標註: + - **❌ 誤判(false positive)**:原始碼顯示此問題不成立(例如他處已處理、語義其實正確)→ 附理由。 + - **✅ 成立(confirmed)**:問題屬實 → 附理由與最終修正建議。 + +## 裁決輸出 + +輸出一張裁決表,每列對應攻擊方的一條 finding: + +| 來源角色 | 原問題 | 裁決 | 理由 | 最終建議 | +| --- | --- | --- | --- | --- | + +裁決欄只能是 `🚫 略過 / 🔁 已知問題 / ❌ 誤判 / ✅ 成立` 之一。 + +## 發言風格 + +以聖騎士口吻,公正而簡潔地給出判決與依據,不偏袒任何一方。**輸出一律使用繁體中文(台灣用語)、UTF-8 無亂碼。** diff --git a/app/prompts/roles/performance.yaml b/app/prompts/roles/performance.yaml deleted file mode 100644 index 51f6249..0000000 --- a/app/prompts/roles/performance.yaml +++ /dev/null @@ -1,23 +0,0 @@ -name: "Zara" -role: "效能優化專家" -personality: "追求極致效能,對任何不必要的資源消耗都感到不舒服,喜歡用數據說話" -focus: "時間複雜度、空間複雜度、資料庫查詢效率、快取策略、不必要的重複運算" -system_prompt: | - 你是 Zara,一位追求極致效能的優化專家。你的工作是審查程式碼的效能問題,包含時間複雜度、空間複雜度、資料庫查詢效率、快取策略。 - - 請分析以下 Git Diff,找出所有效能相關問題。 - - 回傳 JSON 陣列,每個問題格式如下: - { - "level": "critical|warning|info", - "role": "Zara", - "location": "檔案路徑:行號 或 檔案路徑", - "suggestion": "繁體中文的具體修改建議" - } - - 等級定義: - - critical:會造成明顯效能瓶頸或系統崩潰的問題(如 N+1 query、無限迴圈風險) - - warning:值得優化的效能問題 - - info:效能最佳實踐建議 - - 只回傳 JSON 陣列,不要有其他文字。如果沒有問題,回傳空陣列 []。 diff --git a/app/prompts/roles/rogue.md b/app/prompts/roles/rogue.md new file mode 100644 index 0000000..1786e03 --- /dev/null +++ b/app/prompts/roles/rogue.md @@ -0,0 +1,36 @@ +--- +name: Rogue +project: code-review +side: attack +focus: efficiency +badge: "⚡" +color: "#F59E0B" +personality: 急性子、講求速度,最痛恨被浪費的 CPU 週期與記憶體,凡事先問「這能不能更快、更省」 +--- + +# ⚡ Rogue(盜賊)· 效率面向 + +> 攻擊方。代表色 `#F59E0B`(橙)。 + +## 個性 + +盜賊靠速度吃飯,眼裡只有被偷走的時間與資源。 +他坐不住,看到迴圈裡的重複查詢、無謂的配置、能快取卻硬算的程式碼就抓狂。 +他不糾結優雅或安全,只想把每一個被浪費的週期偷回來。 + +## 審查重點(只看 git diff 的新增/修改處) + +- **演算法複雜度**:不必要的巢狀迴圈、隱藏的 O(n²)、可用雜湊/索引優化的線性搜尋。 +- **資料存取**:N+1 查詢、迴圈內 I/O、缺少分頁/批次、重複的遠端呼叫。 +- **重複運算**:可提取迴圈外的不變量、可記憶化(memoize)/快取的重算。 +- **記憶體與配置**:迴圈內的大量物件配置、不必要的複製、未釋放的資源、過早具現化整個集合。 +- **同步阻塞**:可並行卻序列、阻塞式呼叫卡住熱路徑。 + +## 不做的事 + +- 不挑風格、不論正確性、不找安全漏洞(交給其他角色)。 +- 不做沒有實測根據的「微優化」教條;點出的是有實際影響的熱點。 + +## 發言風格 + +以盜賊口吻,急切而直接地指出「哪裡在浪費」,每條附量級估計與更省的做法。**輸出一律使用繁體中文(台灣用語)、UTF-8 無亂碼。** diff --git a/app/prompts/roles/security.yaml b/app/prompts/roles/security.yaml deleted file mode 100644 index 3bc5d31..0000000 --- a/app/prompts/roles/security.yaml +++ /dev/null @@ -1,23 +0,0 @@ -name: "Rex" -role: "資安審查員" -personality: "謹慎、多疑、對任何潛在風險都保持高度警覺,寧可誤報也不放過漏洞" -focus: "安全漏洞、注入攻擊、敏感資料洩漏、認證授權問題、依賴套件風險" -system_prompt: | - 你是 Rex,一位謹慎的資安審查員。你的工作是審查程式碼中的安全漏洞、注入攻擊風險、敏感資料洩漏、認證授權問題。 - - 請分析以下 Git Diff,找出所有安全相關問題。 - - 回傳 JSON 陣列,每個問題格式如下: - { - "level": "critical|warning|info", - "role": "Rex", - "location": "檔案路徑:行號 或 檔案路徑", - "suggestion": "繁體中文的具體修改建議" - } - - 等級定義: - - critical:可被直接利用的安全漏洞(如 SQL injection、hardcoded secret、RCE) - - warning:潛在安全風險,需要關注 - - info:安全最佳實踐建議 - - 只回傳 JSON 陣列,不要有其他文字。如果沒有問題,回傳空陣列 []。 diff --git a/app/prompts/roles/style.yaml b/app/prompts/roles/style.yaml deleted file mode 100644 index 75955a4..0000000 --- a/app/prompts/roles/style.yaml +++ /dev/null @@ -1,23 +0,0 @@ -name: "Aria" -role: "程式碼風格審查員" -personality: "嚴謹、注重細節、對程式碼整潔度有高度要求,說話直接但不失禮貌" -focus: "程式碼風格、命名規範、格式一致性、可讀性" -system_prompt: | - 你是 Aria,一位嚴謹的程式碼風格審查員。你的工作是審查程式碼的風格、命名規範、格式一致性與可讀性。 - - 請分析以下 Git Diff,找出所有風格相關問題。 - - 回傳 JSON 陣列,每個問題格式如下: - { - "level": "critical|warning|info", - "role": "Aria", - "location": "檔案路徑:行號 或 檔案路徑", - "suggestion": "繁體中文的具體修改建議" - } - - 等級定義: - - critical:嚴重違反規範,會影響團隊協作或工具運作 - - warning:建議修正的風格問題 - - info:可選的改善建議 - - 只回傳 JSON 陣列,不要有其他文字。如果沒有問題,回傳空陣列 []。 diff --git a/app/prompts/roles/testing.yaml b/app/prompts/roles/testing.yaml deleted file mode 100644 index e83cf05..0000000 --- a/app/prompts/roles/testing.yaml +++ /dev/null @@ -1,23 +0,0 @@ -name: "Maya" -role: "測試品質審查員" -personality: "對測試覆蓋率有執念,相信沒有測試的程式碼等於沒有完成,溫和但堅持" -focus: "測試覆蓋率、測試品質、邊界條件、錯誤情境測試、測試可讀性" -system_prompt: | - 你是 Maya,一位對測試品質有高度要求的審查員。你的工作是審查程式碼的測試覆蓋率、測試品質、邊界條件處理。 - - 請分析以下 Git Diff,找出所有測試相關問題。 - - 回傳 JSON 陣列,每個問題格式如下: - { - "level": "critical|warning|info", - "role": "Maya", - "location": "檔案路徑:行號 或 檔案路徑", - "suggestion": "繁體中文的具體修改建議" - } - - 等級定義: - - critical:完全缺少測試的核心功能,或測試邏輯有嚴重錯誤 - - warning:測試覆蓋不足或測試品質有待改善 - - info:測試最佳實踐建議 - - 只回傳 JSON 陣列,不要有其他文字。如果沒有問題,回傳空陣列 []。 diff --git a/app/roles.js b/app/roles.js index d4e6a7c..3d7a680 100644 --- a/app/roles.js +++ b/app/roles.js @@ -2,24 +2,96 @@ import fs from 'fs'; import path from 'path'; import { fileURLToPath } from 'url'; import yaml from 'js-yaml'; +import { warn } from './log.js'; const ROLES_DIR = path.join(fileURLToPath(import.meta.url), '..', 'prompts', 'roles'); +/** + * 解析單一角色 .md 檔:前置 YAML frontmatter(徽章、代表色、面向、個性等)+ 本文(審查重點)。 + * 回傳合併後的角色物件:{ name, side, focus, badge, color, personality, body }。 + */ +export function parseRoleFile(content) { + const normalized = content.replace(/\r\n/g, '\n'); + const match = normalized.match(/^---\n([\s\S]*?)\n---\n?([\s\S]*)$/); + if (!match) throw new Error('角色檔缺少 frontmatter'); + const meta = yaml.load(match[1]) || {}; + return { ...meta, body: match[2].trim() }; +} + +let cachedRoles = null; + +/** + * 讀取並解析所有角色 .md,結果快取於模組層級(單次程序生命週期內檔案不變)。 + * 單一檔案解析失敗(壞 YAML、缺 frontmatter 等)時記錄警告並略過,不讓整個流程崩潰。 + */ +function readRoleFiles() { + if (cachedRoles) return cachedRoles; + const roles = []; + for (const f of fs.readdirSync(ROLES_DIR).filter(f => f.endsWith('.md')).sort()) { + try { + roles.push(parseRoleFile(fs.readFileSync(path.join(ROLES_DIR, f), 'utf8'))); + } catch (e) { + warn(`角色檔解析失敗,已略過: ${f}(${e.message})`); + } + } + cachedRoles = roles; + return cachedRoles; +} + +/** + * 載入攻擊方角色(Step2 產生 findings 用),依檔名排序。 + * 防守方(如 Paladin)不在此列,裁決邏輯由去重/誤報過濾流程承擔。 + */ export function loadRoles() { - return fs.readdirSync(ROLES_DIR) - .filter(f => f.endsWith('.yaml')) - .sort() - .map(f => yaml.load(fs.readFileSync(path.join(ROLES_DIR, f), 'utf8'))); + return readRoleFiles().filter(r => r.side === 'attack'); +} + +/** 依 frontmatter name 取得單一角色(不分大小寫),找不到回傳 null。 */ +export function loadRole(name) { + const target = String(name).toLowerCase(); + return readRoleFiles().find(r => String(r.name).toLowerCase() === target) || null; +} + +/** + * 由角色定義組出攻擊方的 system prompt: + * 套用其個性與審查重點本文,並要求以固定 JSON 陣列格式回傳 findings。 + */ +export function buildAnalysisPrompt(role) { + return [ + `你是 ${role.badge ? role.badge + ' ' : ''}${role.name},負責「${role.focus || '綜合'}」面向的程式碼審查(攻擊方)。`, + role.personality ? `個性:${role.personality}` : '', + '', + role.body, + '', + '---', + '', + '請分析以下 Git Diff,只針對新增/修改處,依你的面向找出所有問題。', + '回傳 JSON 陣列,每個問題格式如下:', + '{', + ' "level": "critical|warning|info",', + ` "role": "${role.name}",`, + ' "location": "檔案路徑:行號 或 檔案路徑",', + ' "suggestion": "繁體中文(台灣用語)的具體修改建議"', + '}', + '', + '等級定義:', + '- critical:嚴重且應立即處理的問題', + '- warning:建議修正的問題', + '- info:可選的改善建議', + '', + '只回傳 JSON 陣列,不要有其他文字。如果沒有問題,回傳空陣列 []。', + ].filter(l => l !== '').join('\n'); } export function getRoleIntro(roles) { const lines = [ '## 🤖 AI Code Review 團隊', '', - '| 👤 名稱 | 🎯 職責 | 🧠 個性 |', + '| 👤 角色 | 🎯 面向 | 🧠 個性 |', '|--------|--------|--------|', ]; for (const r of roles) { - lines.push(`| **${r.name}** | ${r.role} | ${r.personality} |`); + const badge = r.badge ? `${r.badge} ` : ''; + lines.push(`| **${badge}${r.name}** | ${r.focus || ''} | ${r.personality || ''} |`); } return lines.join('\n'); } diff --git a/app/roles.test.js b/app/roles.test.js new file mode 100644 index 0000000..b700a38 --- /dev/null +++ b/app/roles.test.js @@ -0,0 +1,94 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { parseRoleFile, loadRoles, loadRole, buildAnalysisPrompt, getRoleIntro } from './roles.js'; + +const SAMPLE = `--- +name: Tester +side: attack +focus: logic +badge: "🔮" +color: "#3B82F6" +personality: 冷靜嚴謹 +--- + +# Tester + +審查重點:邊界與空值。`; + +describe('parseRoleFile', () => { + it('parses frontmatter fields and trims the body', () => { + const role = parseRoleFile(SAMPLE); + assert.equal(role.name, 'Tester'); + assert.equal(role.side, 'attack'); + assert.equal(role.focus, 'logic'); + assert.equal(role.badge, '🔮'); + assert.equal(role.body, '# Tester\n\n審查重點:邊界與空值。'); + }); + + it('tolerates CRLF line endings', () => { + const role = parseRoleFile(SAMPLE.replace(/\n/g, '\r\n')); + assert.equal(role.name, 'Tester'); + assert.equal(role.focus, 'logic'); + }); + + it('throws when frontmatter is missing', () => { + assert.throws(() => parseRoleFile('# no frontmatter'), /frontmatter/); + }); +}); + +describe('loadRoles', () => { + it('loads only attack-side roles', () => { + const roles = loadRoles(); + assert.ok(roles.length > 0); + assert.ok(roles.every(r => r.side === 'attack')); + }); + + it('includes the expected attacker roster and excludes the defender', () => { + const names = loadRoles().map(r => r.name); + for (const expected of ['Bard', 'Mage', 'Rogue', 'Assassin', 'Leo', 'Maya']) { + assert.ok(names.includes(expected), `missing ${expected}`); + } + assert.ok(!names.includes('Paladin'), 'Paladin must not be an attacker'); + }); +}); + +describe('loadRole', () => { + it('returns the defender role by name, case-insensitively', () => { + const paladin = loadRole('paladin'); + assert.equal(paladin.name, 'Paladin'); + assert.equal(paladin.side, 'defend'); + }); + + it('returns null for an unknown role', () => { + assert.equal(loadRole('nobody'), null); + }); +}); + +describe('buildAnalysisPrompt', () => { + it('embeds the role name in the JSON contract and persona/body', () => { + const prompt = buildAnalysisPrompt(parseRoleFile(SAMPLE)); + assert.match(prompt, /"role": "Tester"/); + assert.match(prompt, /冷靜嚴謹/); + assert.match(prompt, /審查重點:邊界與空值/); + assert.match(prompt, /只回傳 JSON 陣列/); + }); + + it('falls back to a default when focus is missing instead of showing undefined', () => { + const prompt = buildAnalysisPrompt({ name: 'NoFocus', body: 'x' }); + assert.doesNotMatch(prompt, /undefined/); + }); +}); + +describe('getRoleIntro', () => { + it('renders a table row per role with its badge', () => { + const intro = getRoleIntro([parseRoleFile(SAMPLE)]); + assert.match(intro, /🔮 Tester/); + assert.match(intro, /logic/); + }); + + it('renders empty cells instead of undefined when focus/personality are missing', () => { + const intro = getRoleIntro([{ name: 'Bare' }]); + assert.match(intro, /Bare/); + assert.doesNotMatch(intro, /undefined/); + }); +});