Compare commits

...

11 Commits

11 changed files with 110 additions and 55 deletions
+50
View File
@@ -133,5 +133,55 @@
"role": "Maya",
"location": "app/llm.test.js",
"suggestion": "輪替邏輯對所有錯誤類型行為一致(catch 全部),401/429/timeout 觸發相同輪替流程,測試不同錯誤類型無額外驗證價值"
},
{
"role": "Aria",
"location": ".gitea/workflows/master.yaml",
"suggestion": "master.yaml 檔案結尾已有換行符號(0x0a),符合 POSIX 慣例,無需修改"
},
{
"role": "Leo",
"location": "app/llm.test.js",
"suggestion": "console.log/error 為診斷用途,不是業務邏輯,TODO.md 驗收標準為人工驗收描述,不需要在單元測試中斷言 console 輸出"
},
{
"role": "Leo",
"location": "app/llm.test.js",
"suggestion": "輪替邏輯對所有錯誤類型行為一致(catch 全部),401/429/timeout 觸發相同輪替流程,測試不同錯誤類型無額外驗證價值"
},
{
"role": "Rex",
"location": "app/package.json",
"suggestion": "審查 changelog 是人工作業,不是程式碼問題,不適合作為 code review 問題"
},
{
"role": "Aria",
"location": "app/llm.js",
"suggestion": "此 action 為 CLI 工具,process.exit(1) 是設計意圖讓 CI/CD workflow 失敗。改拋錯會被 chatJSON 的 catch 吞掉回傳 [],破壞現有行為"
},
{
"role": "Aria",
"location": "Dockerfile",
"suggestion": "Dockerfile 檔案結尾已有換行符號(0x0a),符合 POSIX 慣例"
},
{
"role": "Aria",
"location": "entrypoint.sh",
"suggestion": "entrypoint.sh 檔案結尾已有換行符號(0x0a),符合 POSIX 慣例"
},
{
"role": "Maya",
"location": "app/main.js",
"suggestion": "main.js 整合測試需要真實 Gitea API、LLM API、git 操作,不適合單元測試。各模組已有獨立單元測試覆蓋"
},
{
"role": "Maya",
"location": "app/comments.js",
"suggestion": "comments.js 的 buildTable 為簡單字串拼接,postComment 已透過 gitea.js mock 間接測試,補測試效益低"
},
{
"role": "Maya",
"location": "app/roles.js",
"suggestion": "roles.js 依賴容器內固定路徑 /action/app/prompts/roles,單元測試環境無法存取,且邏輯為簡單 YAML 讀取與字串拼接"
}
]
+37 -44
View File
@@ -2,71 +2,64 @@
{
"level": "critical",
"role": "Leo",
"location": "app/llm.js:22",
"suggestion": "在 `chat` 函式中,`Authorization` 標頭是無條件地被加入到所有 LLM 請求中。這對於不需要 API Key 的服務(如 Ollama)是不必要的,且可能導致錯誤。建議在設定 `Authorization` 標頭之前,先判斷當前的 `provider` 是否需要 API Key,例如 `if (provider !== 'ollama' && apiKeys[i]) { headers['Authorization'] = `Bearer ${apiKeys[i]}`; }`,以提高程式碼的健壯性和正確性。",
"is_new": false
"location": "app/gitea.js:10",
"suggestion": "在 `app/gitea.js` 中,`https.Agent` 設定 `rejectUnauthorized: false`,這會禁用 SSL/TLS 憑證驗證。這是一個嚴重的安全漏洞,會使應用程式容易受到中間人攻擊,並嚴重影響系統的安全性與可維護性。強烈建議移除此設定,或在必要時配置正確的憑證信任鏈。",
"is_new": true
},
{
"level": "critical",
"role": "Zara",
"location": "app/llm.js:20",
"suggestion": "當啟用 API Key 輪替機制時,單一 API 請求的 `timeout` 設定為 120 秒過長。若有多個 Key 且每個 Key 都因逾時而失敗,可能導致整個流程耗時過久(例如 10 個 Key 可能耗時 20 分鐘)。建議將單次請求的逾時時間縮短(例如 10-30 秒),以加速 Key 的輪替,避免 CI/CD 流程長時間阻塞。",
"is_new": false
},
{
"level": "critical",
"role": "Rex",
"location": "app/llm.js:6",
"suggestion": "程式碼中 `https.Agent({ rejectUnauthorized: false })` 停用了 SSL/TLS 憑證驗證。這會使所有 HTTPS 連線容易受到中間人 (Man-in-the-Middle, MITM) 攻擊,攻擊者可以攔截並修改與 LLM 服務提供者的通訊,導致資料洩漏、未經授權的存取或 AI 回應被操縱。請移除 `const httpsAgent = new https.Agent({ rejectUnauthorized: false });` 這一行,並確保 `axios.post` 呼叫中不再使用 `httpsAgent` 選項。預設情況下,Node.js 和 Axios 會執行嚴格的 SSL 憑證驗證,這是確保通訊安全的最佳實踐。如果遇到憑證問題,應調查並解決底層的憑證信任鏈問題,而非禁用驗證。",
"is_new": false
},
{
"level": "critical",
"role": "Maya",
"location": "app/llm.js:10-31",
"suggestion": "`app/llm.js` 中實現的 API Key 輪替功能是本次改動的核心,但目前缺少對應的單元測試。請務必在 `app/llm.test.js` 中新增全面的測試案例,以驗證 `TODO.md` 中「階段八:API Key 輪替」的所有驗收標準:\n* **單一 Key 成功**:傳入單一有效 Key 時,確保行為與原本相同。\n* **多個 Key 輪替成功**:驗證當前 N-1 個 Key 失敗,第 N 個 Key 成功時,系統能依序嘗試並最終成功。\n* **所有 Key 失敗**:驗證當所有傳入的 Key 都失敗時,系統能正確記錄每次失敗,並最終呼叫 `process.exit(1)` 終止流程(測試時需模擬 `process.exit` 以捕獲其調用)。\n* **日誌訊息**:驗證在 Key 失敗時,能正確輸出「key[N/M] 失敗」的日誌;所有 Key 失敗時,能輸出「所有 API Key 均失敗,終止流程」。\n* **錯誤處理**:模擬不同類型的 API 錯誤(例如 401 Unauthorized, 429 Too Many Requests, 網路超時等),確保 Key 輪替機制能穩健處理。",
"is_new": false
},
{
"level": "warning",
"role": "Zara",
"location": "app/comments.js:24",
"suggestion": "在 `saveFindings` 函數中,`fs.writeFileSync` 是一個同步操作。如果 `findings` 陣列可能非常大,或者此函數會被頻繁呼叫,同步寫入檔案可能會阻塞 Node.js 事件迴圈,導致應用程式響應變慢。建議改用 `fs.writeFile` (非同步) 以避免阻塞主執行緒,提升應用程式的響應能力。",
"is_new": false
},
{
"level": "warning",
"role": "Aria",
"location": ".gitea/workflows/master.yaml",
"suggestion": "檔案結尾應包含一個換行符號 (newline at EOF),這是 POSIX 系統的慣例,有助於版本控制系統的正確處理。",
"is_new": false
"location": "Dockerfile",
"suggestion": "檔案結尾缺少換行符號newline character),這不符合 POSIX 慣例,建議在檔案末尾新增一個空行。",
"is_new": true
},
{
"level": "critical",
"role": "Aria",
"location": "entrypoint.sh",
"suggestion": "檔案結尾缺少換行符號(newline character),這不符合 POSIX 慣例,建議在檔案末尾新增一個空行。",
"is_new": true
},
{
"level": "warning",
"role": "Leo",
"location": ".gitea/workflows/review.yaml:36",
"suggestion": "GEMINI_API_KEY 的值過長,影響可讀性。雖然這是為了傳遞多個 Secret,但建議考慮是否有其他方式可以讓設定檔更簡潔,例如將多個 Secret 組合為一個,或在 Action 內部處理多個獨立的 Secret 變數(如果 Action 支援)。如果沒有其他方式,請考慮將其分行以提高可讀性(雖然 YAML 可能會將其視為單行)。",
"is_new": false
"location": "Dockerfile:2",
"suggestion": "在 `Dockerfile` 中,基礎映像檔使用了 `FROM alpine` 而沒有指定具體的版本標籤(例如 `alpine:3.18`)。這可能導致未來 `alpine:latest` 更新時,建置環境發生非預期的變更,影響建置的可重現性和穩定性。為了長期維護性,建議明確指定基礎映像檔的版本。",
"is_new": true
},
{
"level": "warning",
"role": "Maya",
"location": "app/llm.test.js",
"suggestion": "在 `llm.test.js` 中,`TODO.md` 驗收標準明確要求驗證「key[N/M] 失敗」和「所有 API Key 均失敗,終止流程」的日誌訊息。目前的測試雖然驗證了 `process.exit(1)` 的調用,但並未對 `console.log` 和 `console.error` 的輸出進行模擬和斷言。建議使用 `mock.method(console, 'log', ...)` `mock.method(console, 'error', ...)` 來捕獲並驗證這些重要的日誌訊息,確保系統在 API Key 輪替失敗時能提供清晰的診斷資訊。",
"role": "Zara",
"location": "Dockerfile",
"suggestion": "建議調整 Dockerfile 的層次,將 `COPY app/package.json app/package-lock.json /action/app/` 放在 `npm install` 之前,然後再 `COPY app/ /action/app/`。這樣可以利用 Docker 的層次快取,當 `package.json` `package-lock.json` 未變更時,`npm install` 步驟就不會重複執行,顯著加快 Docker 映像檔的建置速度。",
"is_new": true
},
{
"level": "warning",
"role": "Maya",
"location": "app/llm.test.js",
"suggestion": "針對 API Key 輪替的錯誤處理,`TODO.md` 驗收標準中提到「模擬不同類型的 API 錯誤(例如 401 Unauthorized, 429 Too Many Requests, 網路超時等)」。目前的測試僅使用 `new Error('fail')` 進行通用錯誤模擬。建議擴展測試案例,模擬 `axios` 拋出帶有特定 HTTP 狀態碼(如 401, 429)的錯誤,以及模擬網路超時(例如 `axios.isAxiosError` 且 `e.code === 'ECONNABORTED'`),以確保 API Key 輪替機制在面對不同類型的 API 錯誤時都能穩健運作。",
"location": "app/main.js",
"suggestion": "儘管各單元模組(`config`, `git`, `llm`)已有良好的單元測試,但 `app/main.js` 作為整個 Action 的協調者,其整合邏輯和端到端流程缺乏測試。特別是 `TODO.md` 中提到的「阻擋嚴重問題 PR(exit 1)」等關鍵行為,應透過整合測試或端到端測試來驗證,確保各模組協同運作正確,且整體流程符合預期。",
"is_new": true
},
{
"level": "warning",
"role": "Maya",
"location": "app/llm.js",
"suggestion": "`app/llm.js` 中的 `chatJSON` 函數依賴於 `chat` 函數的 API Key 輪替邏輯。雖然 `chat` 函數已新增測試,但 `chatJSON` 函數本身並未有專屬的測試案例。建議在 `app/llm.test.js` 中新增針對 `chatJSON` 的測試,特別是驗證在 API Key 輪替成功和失敗時,`chatJSON` 能正確處理 JSON 格式的回應,並在所有 Key 失敗時也能正確終止流程。",
"location": "app/comments.js",
"suggestion": "`app/comments.js` 模組負責生成評論內容和儲存 findings 檔案。其中 `buildTable` 等函式負責 Markdown 表格的生成,以及 `postOldFindingsComment`, `postNewNonCriticalComment`, `postNewCriticalComments` 中的過濾邏輯,應補齊單元測試,以確保評論內容的格式正確性及問題分類的準確性。",
"is_new": true
},
{
"level": "warning",
"role": "Maya",
"location": "app/roles.js",
"suggestion": "`app/roles.js` 模組負責載入和處理角色定義。`loadRoles` 函式應增加單元測試,以驗證其能正確解析 YAML 檔案,並處理檔案不存在或格式錯誤等異常情況。`getRoleIntro` 函式也應測試其生成的 Markdown 介紹內容是否符合預期格式。",
"is_new": true
},
{
"level": "info",
"role": "Maya",
"location": "app/package.json",
"suggestion": "目前的 `test` 腳本 (`node --test git.test.js config.test.js llm.test.js`) 僅涵蓋了部分單元測試。建議更新此腳本,使其能自動發現並執行所有 `*.test.js` 檔案,以確保所有新增的測試都能被納入 CI 流程中執行。",
"is_new": true
}
]
+1 -1
View File
@@ -33,7 +33,7 @@ jobs:
- name: AI Code Review
uses: https://gitea.jsc.idv.tw/jiantw83/code-review@v${{ needs.version.outputs.version }}
with:
GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }},${{ secrets.GEMINI_API_KEY_1 }},${{ secrets.GEMINI_API_KEY_2 }},${{ secrets.GEMINI_API_KEY_3 }},${{ secrets.GEMINI_API_KEY_4 }},${{ secrets.GEMINI_API_KEY_5 }},${{ secrets.GEMINI_API_KEY_6 }},${{ secrets.GEMINI_API_KEY_7 }},${{ secrets.GEMINI_API_KEY_8 }},${{ secrets.GEMINI_API_KEY_9 }}
GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }},${{ secrets.GEMINI_API_KEY_1 }},${{ secrets.GEMINI_API_KEY_2 }},${{ secrets.GEMINI_API_KEY_3 }},${{ secrets.GEMINI_API_KEY_4 }},${{ secrets.GEMINI_API_KEY_5 }},${{ secrets.GEMINI_API_KEY_6 }},${{ secrets.GEMINI_API_KEY_7 }},${{ secrets.GEMINI_API_KEY_8 }},${{ secrets.GEMINI_API_KEY_9 }},${{ secrets.GEMINI_API_KEY_10 }},${{ secrets.GEMINI_API_KEY_11 }},${{ secrets.GEMINI_API_KEY_12 }},${{ secrets.GEMINI_API_KEY_13 }},${{ secrets.GEMINI_API_KEY_14 }},${{ secrets.GEMINI_API_KEY_15 }},${{ secrets.GEMINI_API_KEY_16 }},${{ secrets.GEMINI_API_KEY_17 }},${{ secrets.GEMINI_API_KEY_18 }},${{ secrets.GEMINI_API_KEY_19 }}
GEMINI_BASE_URL: https://generativelanguage.googleapis.com/v1beta
GEMINI_MODEL: ${{ vars.GEMINI_MODEL }}
permissions:
+5 -4
View File
@@ -1,4 +1,4 @@
FROM alpine
FROM alpine:3.20
RUN apk add --no-cache bash nodejs npm git \
&& node --version \
@@ -7,10 +7,11 @@ RUN apk add --no-cache bash nodejs npm git \
WORKDIR /action
COPY app/package.json /action/app/
RUN cd /action/app && npm install
COPY app/ /action/app/
COPY entrypoint.sh /entrypoint.sh
RUN cd /action/app && npm install && \
chmod +x /entrypoint.sh
RUN chmod +x /entrypoint.sh
ENTRYPOINT ["/entrypoint.sh"]
+1 -1
View File
@@ -111,7 +111,7 @@ jobs:
- name: AI Code Review
uses: https://gitea.jsc.idv.tw/jiantw83/code-review@${{ vars.ACTION_CODE_REVIEW_VERSION }}
with:
GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }},${{ secrets.GEMINI_API_KEY_1 }},${{ secrets.GEMINI_API_KEY_2 }},${{ secrets.GEMINI_API_KEY_3 }},${{ secrets.GEMINI_API_KEY_4 }},${{ secrets.GEMINI_API_KEY_5 }},${{ secrets.GEMINI_API_KEY_6 }},${{ secrets.GEMINI_API_KEY_7 }},${{ secrets.GEMINI_API_KEY_8 }},${{ secrets.GEMINI_API_KEY_9 }}
GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }},${{ secrets.GEMINI_API_KEY_1 }},${{ secrets.GEMINI_API_KEY_2 }},${{ secrets.GEMINI_API_KEY_3 }},${{ secrets.GEMINI_API_KEY_4 }},${{ secrets.GEMINI_API_KEY_5 }},${{ secrets.GEMINI_API_KEY_6 }},${{ secrets.GEMINI_API_KEY_7 }},${{ secrets.GEMINI_API_KEY_8 }},${{ secrets.GEMINI_API_KEY_9 }},${{ secrets.GEMINI_API_KEY_10 }},${{ secrets.GEMINI_API_KEY_11 }},${{ secrets.GEMINI_API_KEY_12 }},${{ secrets.GEMINI_API_KEY_13 }},${{ secrets.GEMINI_API_KEY_14 }},${{ secrets.GEMINI_API_KEY_15 }},${{ secrets.GEMINI_API_KEY_16 }},${{ secrets.GEMINI_API_KEY_17 }},${{ secrets.GEMINI_API_KEY_18 }},${{ secrets.GEMINI_API_KEY_19 }}
GEMINI_BASE_URL: https://generativelanguage.googleapis.com/v1beta
GEMINI_MODEL: ${{ vars.GEMINI_MODEL }}
permissions:
+5
View File
@@ -12,6 +12,10 @@ inputs:
GITEA_REPOSITORY:
description: 'Gitea Repository (owner/repo)'
required: false
GITEA_SKIP_TLS_VERIFY:
description: '跳過 Gitea SSL/TLS 憑證驗證(自簽憑證時使用)'
required: false
default: 'false'
PR_NUMBER:
description: 'Pull Request Number'
required: false
@@ -80,6 +84,7 @@ runs:
GITEA_TOKEN: ${{ inputs.GITEA_TOKEN || secrets.GITEA_TOKEN }}
GITEA_SERVER_URL: ${{ inputs.GITEA_SERVER_URL || gitea.server_url }}
GITEA_REPOSITORY: ${{ inputs.GITEA_REPOSITORY || gitea.repository }}
GITEA_SKIP_TLS_VERIFY: ${{ inputs.GITEA_SKIP_TLS_VERIFY }}
PR_NUMBER: ${{ inputs.PR_NUMBER || gitea.event.pull_request.number }}
PR_HEAD_BRANCH: ${{ inputs.PR_HEAD_BRANCH || gitea.event.pull_request.head.ref }}
PR_BASE_BRANCH: ${{ inputs.PR_BASE_BRANCH || gitea.event.pull_request.base.ref }}
+1
View File
@@ -1,6 +1,7 @@
export const GITEA_TOKEN = process.env.GITEA_TOKEN || '';
export const GITEA_SERVER_URL = process.env.GITEA_SERVER_URL || 'https://gitea.com';
export const GITEA_REPOSITORY = process.env.GITEA_REPOSITORY || '';
export const GITEA_SKIP_TLS_VERIFY = process.env.GITEA_SKIP_TLS_VERIFY === 'true';
export const PR_NUMBER = process.env.PR_NUMBER || '';
export const PR_HEAD_BRANCH = process.env.PR_HEAD_BRANCH || '';
export const PR_BASE_BRANCH = process.env.PR_BASE_BRANCH || '';
+7
View File
@@ -105,4 +105,11 @@ describe('getLLMConfig', () => {
assert.equal(cfg.provider, 'ollama');
assert.equal(cfg.model, 'llama3');
});
it('comma-only api key is treated as not set', () => {
process.env.OPENAI_API_KEY = ',,,';
const cfg = getLLMConfig();
assert.equal(cfg.provider, null);
assert.deepEqual(cfg.apiKeys, []);
});
});
+2 -2
View File
@@ -1,8 +1,8 @@
import axios from 'axios';
import https from 'https';
import { GITEA_TOKEN, GITEA_SERVER_URL, GITEA_REPOSITORY, PR_NUMBER } from './config.js';
import { GITEA_TOKEN, GITEA_SERVER_URL, GITEA_REPOSITORY, GITEA_SKIP_TLS_VERIFY, PR_NUMBER } from './config.js';
const httpsAgent = new https.Agent({ rejectUnauthorized: false });
const httpsAgent = GITEA_SKIP_TLS_VERIFY ? new https.Agent({ rejectUnauthorized: false }) : undefined;
const headers = () => ({ Authorization: `token ${GITEA_TOKEN}`, 'Content-Type': 'application/json' });
const api = (path) => `${GITEA_SERVER_URL.replace(/\/$/, '')}/api/v1${path}`;
-2
View File
@@ -11,7 +11,6 @@ export async function chat(systemPrompt, userContent) {
if (provider === 'claude') headers['anthropic-version'] = '2023-06-01';
const shuffled = [...apiKeys].sort(() => Math.random() - 0.5);
let lastError;
for (let i = 0; i < shuffled.length; i++) {
if (provider !== 'ollama') headers['Authorization'] = `Bearer ${shuffled[i]}`;
try {
@@ -22,7 +21,6 @@ export async function chat(systemPrompt, userContent) {
);
return resp.data.choices[0].message.content;
} catch (e) {
lastError = e;
console.log(` [LLM] key[${i + 1}/${shuffled.length}] 失敗: ${e.message}`);
}
}
+1 -1
View File
@@ -3,7 +3,7 @@
"version": "1.0.0",
"type": "module",
"scripts": {
"test": "node --test git.test.js config.test.js llm.test.js"
"test": "node --test *.test.js"
},
"dependencies": {
"axios": "^1.6.7",