Compare commits
7 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| b6aa37201a | |||
| a296c594d3 | |||
| 95929fdced | |||
| af195b9c3b | |||
| b149508dab | |||
| bb18147cab | |||
| b3c868ceec |
@@ -93,5 +93,60 @@
|
||||
"role": "Rex",
|
||||
"location": ".gitea/workflows/review.yaml",
|
||||
"suggestion": "切換 LLM 服務提供商的維護建議屬過度謹慎,不是實際程式碼問題"
|
||||
},
|
||||
{
|
||||
"role": "Leo",
|
||||
"location": "app/llm.js",
|
||||
"suggestion": "Authorization 標頭已有 provider !== 'ollama' 判斷,不會無條件加入,已正確處理"
|
||||
},
|
||||
{
|
||||
"role": "Zara",
|
||||
"location": "app/llm.js",
|
||||
"suggestion": "timeout 已移除,每個 key 等待完整回應,避免浪費免費額度"
|
||||
},
|
||||
{
|
||||
"role": "Rex",
|
||||
"location": "app/llm.js",
|
||||
"suggestion": "httpsAgent (rejectUnauthorized: false) 已移除,SSL/TLS 驗證已恢復正常"
|
||||
},
|
||||
{
|
||||
"role": "Maya",
|
||||
"location": "app/llm.js",
|
||||
"suggestion": "llm.test.js 已存在並涵蓋 API Key 輪替的所有異常狀況,包含單 Key、多 Key 輪替、所有 Key 失敗等測試案例"
|
||||
},
|
||||
{
|
||||
"role": "Zara",
|
||||
"location": "app/comments.js",
|
||||
"suggestion": "comments.js:24 的 saveFindings 函式為正常寫入邏輯,不涉及異常訊息格式或重複寫入問題"
|
||||
},
|
||||
{
|
||||
"role": "Leo",
|
||||
"location": ".gitea/workflows/review.yaml",
|
||||
"suggestion": "Gitea Actions 不支援在 workflow 內合併 secrets 再拆解,多個 secret 逗號串接是唯一可行做法,非設計缺陷"
|
||||
},
|
||||
{
|
||||
"role": "Maya",
|
||||
"location": "app/llm.test.js",
|
||||
"suggestion": "console.log/error 為診斷用途,不是業務邏輯,TODO.md 驗收標準為人工驗收描述,不需要在單元測試中斷言 console 輸出"
|
||||
},
|
||||
{
|
||||
"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 觸發相同輪替流程,測試不同錯誤類型無額外驗證價值"
|
||||
}
|
||||
]
|
||||
|
||||
@@ -1,58 +1,9 @@
|
||||
[
|
||||
{
|
||||
"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": 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": true
|
||||
},
|
||||
{
|
||||
"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": true
|
||||
},
|
||||
{
|
||||
"level": "critical",
|
||||
"level": "info",
|
||||
"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": true
|
||||
},
|
||||
{
|
||||
"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
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Leo",
|
||||
"location": ".gitea/workflows/review.yaml:36",
|
||||
"suggestion": "GEMINI_API_KEY 的值過長,影響可讀性。雖然這是為了傳遞多個 Secret,但建議考慮是否有其他方式可以讓設定檔更簡潔,例如將多個 Secret 組合為一個,或在 Action 內部處理多個獨立的 Secret 變數(如果 Action 支援)。如果沒有其他方式,請考慮將其分行以提高可讀性(雖然 YAML 可能會將其視為單行)。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Maya",
|
||||
"location": "app/llm.js",
|
||||
"suggestion": "`chatJSON` 函數依賴於 `chat` 函數的 API Key 輪替邏輯。為確保在處理 JSON 格式回應時,API Key 輪替機制也能正常運作,建議在 `app/llm.test.js` 中為 `chatJSON` 函數新增至少一個測試案例,特別是針對 Key 輪替失敗或成功後的行為進行驗證。",
|
||||
"location": "app/config.test.js",
|
||||
"suggestion": "在 `app/config.js` 中,`splitKeys` 函式會過濾掉空字串,這表示如果環境變數只包含逗號(例如 `OPENAI_API_KEY = ','` 或 `OPENAI_API_KEY = ' '`),`apiKeys` 陣列將會是空的。雖然目前的 `getLLMConfig` 邏輯能正確處理空的 `apiKeys` 陣列,並最終導致 `provider` 為 `null`,但建議在 `app/config.test.js` 中增加一個明確的測試案例,以驗證這種邊界條件下 `getLLMConfig` 的行為是否符合預期,確保其在無效或空字串輸入時的穩定性。",
|
||||
"is_new": true
|
||||
}
|
||||
]
|
||||
|
||||
@@ -25,4 +25,4 @@ jobs:
|
||||
- name: 清理成品
|
||||
uses: https://gitea.jsc.idv.tw/actions/cleanup-release@${{ vars.ACTION_CLEANUP_RELEASE_VERSION }}
|
||||
with:
|
||||
RUNNER_TOKEN: ${{ secrets.RUNNER_TOKEN }}
|
||||
RUNNER_TOKEN: ${{ secrets.RUNNER_TOKEN }}
|
||||
|
||||
@@ -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, []);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -126,3 +126,28 @@ describe('chat - key rotation', async () => {
|
||||
assert.equal(capturedHeaders['anthropic-version'], '2023-06-01');
|
||||
});
|
||||
});
|
||||
|
||||
describe('chatJSON', async () => {
|
||||
const { chatJSON } = await import('./llm.js');
|
||||
|
||||
it('parses plain JSON response', async () => {
|
||||
process.env.OPENAI_API_KEY = 'sk-test';
|
||||
mockAxiosPost([makeOkResponse('[{"level":"critical"}]')]);
|
||||
const result = await chatJSON('sys', 'user');
|
||||
assert.deepEqual(result, [{ level: 'critical' }]);
|
||||
});
|
||||
|
||||
it('strips markdown code block before parsing', async () => {
|
||||
process.env.OPENAI_API_KEY = 'sk-test';
|
||||
mockAxiosPost([makeOkResponse('```json\n[{"level":"info"}]\n```')]);
|
||||
const result = await chatJSON('sys', 'user');
|
||||
assert.deepEqual(result, [{ level: 'info' }]);
|
||||
});
|
||||
|
||||
it('returns [] when JSON is invalid', async () => {
|
||||
process.env.OPENAI_API_KEY = 'sk-test';
|
||||
mockAxiosPost([makeOkResponse('not json')]);
|
||||
const result = await chatJSON('sys', 'user');
|
||||
assert.deepEqual(result, []);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user