Compare commits
6 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| c751a53d43 | |||
| 2aba414d36 | |||
| d565b79feb | |||
| 81d5e3ff13 | |||
| 1ccc2cd560 | |||
| c815c30088 |
@@ -158,5 +158,50 @@
|
||||
"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 讀取與字串拼接"
|
||||
},
|
||||
{
|
||||
"role": "Leo",
|
||||
"location": "app/gitea.js",
|
||||
"suggestion": "gitea.js 的 SSL 驗證已改為由 GITEA_SKIP_TLS_VERIFY 環境變數控制,預設啟用驗證,非安全漏洞"
|
||||
},
|
||||
{
|
||||
"role": "Zara",
|
||||
"location": "Dockerfile",
|
||||
"suggestion": "Dockerfile 已優化層次快取:先 COPY package.json 再 npm install,最後才 COPY 其餘檔案"
|
||||
},
|
||||
{
|
||||
"role": "Aria",
|
||||
"location": "app/package.json",
|
||||
"suggestion": "test 腳本已改為 node --test *.test.js,在 app/ 目錄下執行可自動發現所有測試檔案"
|
||||
},
|
||||
{
|
||||
"role": "Zara",
|
||||
"location": "app/main.js",
|
||||
"suggestion": "deduplicateWithAI 和 filterFalsePositivesWithAI 為循序依賴流程(去重後才能過濾),無法平行化"
|
||||
}
|
||||
]
|
||||
|
||||
@@ -1,9 +1,51 @@
|
||||
[
|
||||
{
|
||||
"level": "info",
|
||||
"level": "critical",
|
||||
"role": "Maya",
|
||||
"location": "app/gitea.js",
|
||||
"suggestion": "`app/gitea.js` 模組負責與 Gitea API 進行所有互動,包括獲取 PR Diff 和發布評論。這些網路操作是 Action 運作的基礎,但目前缺乏單元測試。應補齊測試以驗證 API 請求的正確性(URL、Header、Body)、錯誤處理機制(例如網路中斷、API 錯誤回應)以及 `GITEA_SKIP_TLS_VERIFY` 的行為。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "critical",
|
||||
"role": "Leo",
|
||||
"location": "app/package.json",
|
||||
"suggestion": "在 `app/package.json` 中,`axios` 和 `openai` 等函式庫進行了版本更新,特別是 `openai` 從 `4.28.0` 升級到 `4.104.0`。為了確保長期維護的穩定性和安全性,建議審查這些函式庫的發行說明(changelog),以了解是否有任何重大變更、安全修補或已知漏洞,並確認這些更新不會引入不預期的行為或技術債。",
|
||||
"location": "app/gitea.js:10",
|
||||
"suggestion": "在 `app/gitea.js` 中,`https.Agent` 設定了 `rejectUnauthorized: false`,這會禁用 SSL/TLS 憑證驗證。這是一個嚴重的安全漏洞,會使應用程式容易受到中間人攻擊,並嚴重影響系統的安全性與可維護性。強烈建議移除此設定,或在必要時配置正確的憑證信任鏈。",
|
||||
"is_new": false
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Leo",
|
||||
"location": "app/roles.js:4",
|
||||
"suggestion": "在 `ROLES_DIR` 常數中,使用了硬編碼的路徑 `/action/app/prompts/roles`。這使得在 Docker 容器外部進行本地開發或測試時,需要額外配置才能正確載入角色定義。建議將此路徑設定為可配置的環境變數,或使用相對路徑並在 `main.js` 中傳入基礎路徑,以提高模組的彈性和可測試性。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Zara",
|
||||
"location": "app/main.js:60",
|
||||
"suggestion": "`analyzeWithRole` 函式在迴圈中為每個角色依序呼叫 LLM 進行分析。由於 LLM 呼叫是高延遲操作,這種序列化執行會顯著增加整個 Code Review Action 的總執行時間。考慮使用 `Promise.all` 等方式平行化這些 LLM 呼叫,以縮短總等待時間。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"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": false
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Aria",
|
||||
"location": "app/package.json:5",
|
||||
"suggestion": "`package.json` 中的 `test` 腳本 (`node --test *.test.js`) 僅會執行 `app/` 目錄下的測試檔案。建議修改為 `node --test app/**/*.test.js` 或使用更完善的測試框架(如 `mocha` 或 `jest`),以確保所有測試檔案都能被自動發現並執行。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "info",
|
||||
"role": "Zara",
|
||||
"location": "app/main.js:72, app/main.js:78",
|
||||
"suggestion": "`deduplicateWithAI` 和 `filterFalsePositivesWithAI` 函式也涉及 LLM 呼叫,它們在主流程中依序執行。雖然這些是單次呼叫,但其延遲仍會累積。在設計上,可以考慮是否有可能將這些步驟與其他非依賴的任務平行化,或或評估其對整體效能的影響。",
|
||||
"is_new": true
|
||||
}
|
||||
]
|
||||
|
||||
@@ -34,7 +34,7 @@ jobs:
|
||||
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 }},${{ 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_BASE_URL: https://generativelanguage.googleapis.com/v1
|
||||
GEMINI_MODEL: ${{ vars.GEMINI_MODEL }}
|
||||
permissions:
|
||||
contents: write
|
||||
|
||||
+5
-4
@@ -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"]
|
||||
|
||||
@@ -112,7 +112,7 @@ jobs:
|
||||
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 }},${{ 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_BASE_URL: https://generativelanguage.googleapis.com/v1
|
||||
GEMINI_MODEL: ${{ vars.GEMINI_MODEL }}
|
||||
permissions:
|
||||
contents: write
|
||||
|
||||
@@ -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,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 || '';
|
||||
|
||||
+2
-2
@@ -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}`;
|
||||
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
import { describe, it, afterEach, mock } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import axios from 'axios';
|
||||
|
||||
// gitea.js reads env vars at module load time (ESM cache), so we test
|
||||
// the actual values baked in at import time and verify behavior via axios mocks.
|
||||
|
||||
afterEach(() => mock.restoreAll());
|
||||
|
||||
describe('gitea', async () => {
|
||||
const { getPRDiff, postComment } = await import('./gitea.js');
|
||||
|
||||
it('getPRDiff calls Gitea diff API with Authorization header', async () => {
|
||||
let capturedUrl, capturedOpts;
|
||||
mock.method(axios, 'get', async (url, opts) => {
|
||||
capturedUrl = url;
|
||||
capturedOpts = opts;
|
||||
return { data: 'diff content' };
|
||||
});
|
||||
const result = await getPRDiff();
|
||||
assert.equal(result, 'diff content');
|
||||
assert.ok(capturedUrl.includes('/api/v1/repos/'));
|
||||
assert.ok(capturedUrl.endsWith('.diff'));
|
||||
assert.ok(capturedOpts.headers['Authorization'].startsWith('token '));
|
||||
assert.equal(capturedOpts.headers['Content-Type'], 'application/json');
|
||||
});
|
||||
|
||||
it('postComment calls Gitea issues comments API with body', async () => {
|
||||
let capturedUrl, capturedBody, capturedOpts;
|
||||
mock.method(axios, 'post', async (url, body, opts) => {
|
||||
capturedUrl = url;
|
||||
capturedBody = body;
|
||||
capturedOpts = opts;
|
||||
return { data: { id: 1 } };
|
||||
});
|
||||
const result = await postComment('hello world');
|
||||
assert.deepEqual(result, { id: 1 });
|
||||
assert.ok(capturedUrl.includes('/api/v1/repos/'));
|
||||
assert.ok(capturedUrl.endsWith('/comments'));
|
||||
assert.equal(capturedBody.body, 'hello world');
|
||||
assert.ok(capturedOpts.headers['Authorization'].startsWith('token '));
|
||||
});
|
||||
|
||||
it('does not set httpsAgent by default (GITEA_SKIP_TLS_VERIFY not true)', async () => {
|
||||
let capturedOpts;
|
||||
mock.method(axios, 'get', async (_url, opts) => {
|
||||
capturedOpts = opts;
|
||||
return { data: '' };
|
||||
});
|
||||
await getPRDiff();
|
||||
// httpsAgent is undefined when GITEA_SKIP_TLS_VERIFY !== 'true'
|
||||
assert.equal(capturedOpts.httpsAgent, undefined);
|
||||
});
|
||||
|
||||
it('getPRDiff propagates axios errors', async () => {
|
||||
mock.method(axios, 'get', async () => { throw new Error('network error'); });
|
||||
await assert.rejects(() => getPRDiff(), /network error/);
|
||||
});
|
||||
|
||||
it('postComment propagates axios errors', async () => {
|
||||
mock.method(axios, 'post', async () => { throw new Error('api error'); });
|
||||
await assert.rejects(() => postComment('test'), /api error/);
|
||||
});
|
||||
});
|
||||
+6
-6
@@ -52,13 +52,13 @@ async function main() {
|
||||
|
||||
// Step2: 各角色分析 diff 產生新 findings
|
||||
console.log('\n📊 Step2: Findings 產生');
|
||||
const results = await Promise.allSettled(roles.map(role => analyzeWithRole(role, diff)));
|
||||
const newFindings = [];
|
||||
for (const role of roles) {
|
||||
try {
|
||||
const found = await analyzeWithRole(role, diff);
|
||||
newFindings.push(...found);
|
||||
} catch (e) {
|
||||
console.log(` ⚠️ [${role.name}] 分析失敗(跳過): ${e.message}`);
|
||||
for (let i = 0; i < results.length; i++) {
|
||||
if (results[i].status === 'fulfilled') {
|
||||
newFindings.push(...results[i].value);
|
||||
} else {
|
||||
console.log(` ⚠️ [${roles[i].name}] 分析失敗(跳過): ${results[i].reason?.message}`);
|
||||
}
|
||||
}
|
||||
console.log(` Step2 完成: 新 findings 總計 ${newFindings.length} 筆`);
|
||||
|
||||
+1
-1
@@ -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",
|
||||
|
||||
+2
-1
@@ -1,8 +1,9 @@
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import { fileURLToPath } from 'url';
|
||||
import yaml from 'js-yaml';
|
||||
|
||||
const ROLES_DIR = '/action/app/prompts/roles';
|
||||
const ROLES_DIR = path.join(fileURLToPath(import.meta.url), '..', 'prompts', 'roles');
|
||||
|
||||
export function loadRoles() {
|
||||
return fs.readdirSync(ROLES_DIR)
|
||||
|
||||
Reference in New Issue
Block a user