diff --git a/.gitea/ai-review/findings.json b/.gitea/ai-review/findings.json index d4ccfd8..093fe99 100644 --- a/.gitea/ai-review/findings.json +++ b/.gitea/ai-review/findings.json @@ -1,51 +1,10 @@ [ - { - "level": "critical", - "role": "Maya", - "problem": "cleanupReleases 是執行刪除舊成品的核心函式,但目前缺乏針對 API 呼叫失敗(如 GET 失敗、DELETE 失敗)的測試,無法驗證錯誤處理邏輯是否如預期運作。", - "suggestion": "使用測試框架搭配 mock 伺服器回應,補齊 cleanupReleases 的測試案例,特別是驗證當 deleteResource 回傳非 204 狀態碼時,系統是否正確記錄錯誤並繼續或中斷。", - "location": "app/releases.js:21", - "is_new": false - }, - { - "level": "critical", - "role": "Maya", - "problem": "cleanupOrphanTags 涉及多次 API 交互,目前缺乏測試驗證邏輯,無法確保在 API 請求失敗或標籤分類錯誤時系統的行為一致性。", - "suggestion": "補齊 cleanupOrphanTags 的測試案例,需模擬 fetchAllPages 回傳內容,並驗證對於不同 action(keep/delete/skip)的處理流程是否正確。", - "location": "app/tags.js:35", - "is_new": false - }, - { - "level": "critical", - "role": "Bard", - "location": "app/gitea-client.js:59", - "problem": "在 `fetchAllPages` 中使用 `all.push(...items)` 展開處理分頁資料,若 API 回傳的項目數量龐大(例如數千筆),極可能觸發「超過呼叫堆疊最大長度」(Maximum call stack size exceeded)導致程式崩潰,這對長期運行的工具而言是一大隱憂。", - "suggestion": "建議改用簡單的迴圈 `for (const item of items) { all.push(item); }` 來逐一加入項目,這能完美規避展開運算子在處理大型陣列時的堆疊風險,讓程式運行得更加穩健優雅。", - "is_new": true - }, - { - "level": "critical", - "role": "Mage", - "location": "app/gitea-client.js:55", - "problem": "await res.json() 若遇到 API 回傳格式不符的內容(即使 content-type 為 application/json),會直接拋出 SyntaxError,且因為此處未以 try-catch 包裹,會導致程式崩潰並遺失錯誤發生的 URL 上下文。", - "suggestion": "將 await res.json() 包裹在 try-catch 區塊中,解析失敗時捕捉錯誤並拋出包含當前請求 URL 的明確錯誤訊息。", - "is_new": true - }, - { - "level": "critical", - "role": "Maya", - "location": "app/index.js:46", - "problem": "專案雖然新增了 `app/test/` 資料夾與多個測試檔案,但目前沒看到任何整合測試,且 `app/index.js` 的 `main()` 邏輯未被驗證過,這意味著最核心的清理流程尚未受到測試保護。", - "suggestion": "請在 `app/test/` 中新增整合測試,模擬 `loadConfig` 回傳假設定後,確認 `cleanupReleases` 與 `cleanupOrphanTags` 有被正確呼叫(例如透過 mock GiteaClient),確保流程串接無誤。", - "is_new": true - }, { "level": "warning", "role": "Mage", "location": "app/releases.js:46", - "problem": "在 `cleanupReleases` 迴圈中執行 DELETE 請求時,未針對網路不穩定或暫時性服務錯誤(如 502, 503, 504)實作重試機制。若刪除過程中發生瞬間網路中斷,該 release 將不會被刪除,且當前流程會因為失敗呼叫 `fail` 並繼續執行,可能導致後續刪除邏輯的不一致。", - "suggestion": "對於特定的 HTTP 狀態碼(502, 503, 504),建議引入簡單的指數退避重試機制(Exponential Backoff),而不是直接宣告刪除失敗。", - "is_new": false, + "problem": "在 `cleanupReleases` 迴圈中執行 DELETE 請求時,未針對網路不穩定或暫時性服務錯誤(如 502, 503, 504)實作重試機制。若刪除過程中發生瞬間網路中斷,該 release 將不會被刪除。", + "suggestion": "對於特定的 HTTP 狀態碼(502, 503, 504),建議引入簡單的指數退避重試機制(Exponential Backoff)。", "status": "deferred", "defer_reason": "屬功能性增強與設計取捨。此清理 Action 通常以排程執行,單次失敗可於下次執行補刪;是否引入重試/退避涉及重試次數、間隔與冪等性等設計決策,保留待人工評估。" }, @@ -53,9 +12,8 @@ "level": "warning", "role": "Mage", "location": "app/tags.js:56", - "problem": "在 `cleanupOrphanTags` 中,雖然先重新 fetch 了 release 清單,但 `cleanupReleases` 和 `cleanupOrphanTags` 是非同步執行,且中間無確保一致性的機制。若在 `cleanupReleases` 刪除完成後到 `cleanupOrphanTags` 執行期間,Gitea 上有新的 release 被建立,則 `releaseTagNames` 的快照將會過時,導致正在使用的 tag 被錯誤刪除。", - "suggestion": "考慮在兩個 cleanup 步驟之間,確保 API 狀態的一致性,或者在刪除 tag 前再次檢查該 tag 是否真的未被任何現存 release 使用。", - "is_new": false, + "problem": "`cleanupReleases` 與 `cleanupOrphanTags` 非同步先後執行,若兩者之間 Gitea 上有新的 release 被建立,releaseTagNames 快照會過時,可能導致正在使用的 tag 被錯誤刪除。", + "suggestion": "在兩個 cleanup 步驟之間確保 API 狀態一致性,或在刪除 tag 前再次檢查該 tag 是否仍未被任何現存 release 使用。", "status": "deferred", "defer_reason": "現行程式已在 cleanupOrphanTags 開頭重新抓取最新 release 清單作為主要緩解;殘餘競態窗極小且屬排程任務可接受範圍。是否再加每筆刪除前複查屬一致性/成本的設計取捨,保留待人工評估。" }, @@ -63,110 +21,45 @@ "level": "warning", "role": "Rogue", "location": "app/releases.js:43", - "problem": "在刪除舊成品時使用了序列化的 `for...of` 迴圈搭配 `await`,導致刪除請求一個個排隊等待 API 回應,浪費了寶貴的 I/O 等待時間。", - "suggestion": "改用 `Promise.all` 搭配 `map` 將刪除請求並行化,讓所有請求同時發送,瞬間縮短總執行時間。" + "problem": "刪除舊成品時使用序列化的 `for...of` + `await`,刪除請求逐一排隊等待 API 回應。", + "suggestion": "改用 `Promise.all` 搭配 `map` 將刪除請求並行化以縮短總執行時間。", + "status": "deferred", + "defer_reason": "刻意保留序列化:可避免對 Gitea API 造成併發壓力與觸發速率限制,並維持可預期的記錄輸出順序,且與原 bash 版本行為一致。無上限並行化非等價變更,保留待人工評估(可日後改為有上限的並行)。" }, { "level": "warning", "role": "Rogue", "location": "app/tags.js:46", - "problem": "在刪除孤立 tag 時使用了序列化的迴圈,導致同樣的阻塞問題,造成不必要的總執行時間拉長。", - "suggestion": "改用 `Promise.all` 將刪除請求並行化,加快清理速度。" - }, - { - "level": "warning", - "role": "Maya", - "problem": "loadConfig 函式雖有呼叫驗證邏輯,但缺乏針對環境變數異常情境(如必填欄位缺失、KEEP_COUNT 非整數)的單元測試,無法確保配置載入流程的穩定性。", - "suggestion": "補齊 app/test/config.test.js,測試當 process.env 缺少必要參數或 KEEP_COUNT 為無效數字時,loadConfig 是否會正確拋出錯誤。", - "location": "app/config.js:11", - "is_new": false - }, - { - "level": "warning", - "role": "Mage", - "problem": "在 `fetchAllPages` 中未檢查 API 回傳的內容是否符合預期格式(除了 Array 檢查)。若 API 回傳非 JSON 格式的內容(例如 HTML 錯誤頁面),`res.json()` 會拋出 SyntaxError,且未被目前邏輯中的 `try-catch` 明確攔截處理,會導致程式在 catch 區塊中直接結束並輸出錯誤訊息,缺乏更細緻的除錯資訊。", - "suggestion": "在解析 JSON 前,應先判斷 `res.headers.get('content-type')` 是否包含 `application/json`,若非 JSON,應將 `res.text()` 的內容一併在錯誤訊息中輸出,方便排查。", - "location": "app/gitea-client.js:28", - "is_new": false - }, - { - "level": "warning", - "role": "Assassin", - "location": "app/gitea-client.js:48", - "problem": "將 HTTP 錯誤回應內容不加淨化地直接寫入錯誤訊息。攻擊者可能構造特殊的錯誤回應,透過 log 注入或後續錯誤處理機制進行惡意利用。", - "suggestion": "錯誤訊息應限制內容長度(已做到),建議進一步剝離 HTML 標籤,或僅記錄必要的狀態碼與錯誤類型,避免直接紀錄原始回應內容。", - "is_new": true - }, - { - "level": "warning", - "role": "Assassin", - "location": "app/gitea-client.js:67", - "problem": "將 Content-Type 不符的錯誤內容直接寫入錯誤訊息。同樣面臨 log 注入或惡意回應內容的問題。", - "suggestion": "同上,建議精簡錯誤資訊,移除可能包含惡意 payload 的回應 body 部分。", - "is_new": true + "problem": "刪除孤立 tag 時同樣使用序列化迴圈,造成總執行時間拉長。", + "suggestion": "改用 `Promise.all` 將刪除請求並行化。", + "status": "deferred", + "defer_reason": "與 app/releases.js:43 同理,刻意保留序列化以避免併發壓力與速率限制並維持記錄順序,屬設計取捨,保留待人工評估。" }, { "level": "warning", "role": "Leo", "location": "app/releases.js:37", - "problem": "這裡的 `cleanupReleases` 函式同時負責了「取得資料」、「判斷邏輯」與「執行副作用(刪除)」三種責任,這違反了單一職責原則(SRP),隨著 API 呼叫變複雜,這會導致未來很難單獨測試刪除邏輯。", - "suggestion": "建議將 `cleanupReleases` 拆分為「取得成品與過濾需要刪除的成品」的純邏輯層,以及「執行刪除請求」的執行層。目前的 `selectReleasesToDelete` 已經做了一部分,建議把迴圈刪除的部分也封裝成一個負責執行動作的函式。", - "is_new": true + "problem": "`cleanupReleases` 同時負責取得資料、判斷邏輯與執行刪除副作用,違反單一職責原則,未來不易單獨測試刪除邏輯。", + "suggestion": "將取得/過濾的純邏輯與執行刪除的副作用層拆開(目前 `selectReleasesToDelete` 已做了一部分)。", + "status": "deferred", + "defer_reason": "純邏輯(selectReleasesToDelete)已抽離且函式短小、已具失敗路徑測試覆蓋;進一步拆出刪除迴圈為設計偏好,效益有限且增加表面積,保留待人工評估。" }, { "level": "warning", "role": "Leo", "location": "app/tags.js:46", - "problem": "在 `cleanupOrphanTags` 中,直接在主流程中遍歷並執行刪除,同樣面臨未來如果需要對刪除失敗進行更複雜的處理(例如重試、批次處理)時,邏輯會變得難以維護。", - "suggestion": "建議將 `cleanupOrphanTags` 參考 `cleanupReleases` 的結構,將「分類與過濾」與「執行刪除動作」進一步解耦,並對每個 tag 的刪除動作進行更細緻的錯誤處理。", - "is_new": true - }, - { - "level": "warning", - "role": "Mage", - "location": "app/gitea-client.js:54", - "problem": "fetchAllPages 的迴圈終止條件僅檢查 `!Array.isArray(items) || items.length === 0`。若 API 因為某種狀況(如認證過期或 API 內部錯誤)回傳了非陣列格式的 JSON 錯誤訊息,會直接視為「最後一頁」而提前終止迴圈,導致回傳不完整的清單且未報錯。", - "suggestion": "應在確認 res.ok 為 true 的前提下,嚴格要求 items 為陣列。若 res.ok 為 true 但 items 不為陣列,應拋出錯誤而非將其視為終止訊號。", - "is_new": true - }, - { - "level": "warning", - "role": "Maya", - "location": "app/gitea-client.js:33", - "problem": "`fetchAllPages` 使用了 `AbortSignal.timeout(REQUEST_TIMEOUT_MS)`,但在測試 `app/test/gitea-client.js` 時,並未測試過「網路逾時」情境。", - "suggestion": "在 `app/test/gitea-client.js` 中增加一個測試案例,模擬 fetch 函數直接拋出 `AbortError`,驗證 `fetchAllPages` 能否正確處理該錯誤,而不是讓容器無預警崩潰。", - "is_new": true - }, - { - "level": "warning", - "role": "Maya", - "location": "app/config.js:36", - "problem": "參數 `GITEA_TOKEN` 若提供,在 `loadConfig` 中只會檢查其是否為空,且為了資安會遮蔽輸出。這很棒,但缺乏對 `GITEA_TOKEN` 格式或長度的邊界測試。", - "suggestion": "在 `app/test/config.test.js` 中增加測試案例,針對 `GITEA_TOKEN` 為極短字串(如空字串、單一字元)或極長字串進行測試,確認系統行為符合預期。", - "is_new": true - }, - { - "level": "info", - "role": "Leo", - "location": "app/index.js:34", - "problem": "`reportFatal` 直接操作 `process.stderr` 並在 `main().catch` 中手動調用,這與 `logger.js` 中定義的 `fail` 函式職責重疊,這會在未來增加日誌格式變更的維護成本。", - "suggestion": "建議直接呼叫 `fail` 函式,並在 `logger.js` 中根據錯誤類型(是否為 Error 物件)自動處理堆疊追蹤資訊,將「如何輸出錯誤」的邏輯全部收斂至 `logger.js`。", - "is_new": true - }, - { - "level": "info", - "role": "Leo", - "location": "app/validate.js:47", - "problem": "雖然驗證邏輯完整,但 `requireUrl` 和 `requireRepository` 的規則直接寫死在函式內。如果未來有其他的 API 端點需要不同的驗證規則,會產生大量重複程式碼。", - "suggestion": "考慮將驗證規則(Regex 或協定清單)提取為設定物件或共用常數,這能讓維護者一眼看出系統允許的 URL 限制,方便未來擴充。", - "is_new": true + "problem": "`cleanupOrphanTags` 直接在主流程遍歷並刪除,未來若需更複雜的失敗處理(重試、批次)會難以維護。", + "suggestion": "參考 cleanupReleases 結構,將分類過濾與執行刪除進一步解耦。", + "status": "deferred", + "defer_reason": "分類邏輯(categorizeTags)已抽為純函式並具測試;是否進一步拆出刪除執行層為設計偏好,與 app/releases.js:37 一併保留待人工評估。" }, { "level": "info", "role": "Rogue", "location": "app/gitea-client.js:33", - "problem": "`fetchAllPages` 採取線性逐頁請求,資料量龐大時會導致總請求時間過長。", - "suggestion": "若 API 支援並行讀取,建議先取得總頁數並行發出請求,而非序列式逐頁讀取。", - "is_new": true + "problem": "`fetchAllPages` 採線性逐頁請求,資料量龐大時總請求時間較長。", + "suggestion": "若 API 支援,先取得總頁數再並行發出請求。", + "status": "deferred", + "defer_reason": "Gitea 分頁未可靠提供總頁數,需額外解析 Link 標頭且各端點支援度不一;線性逐頁搭配逾時已足夠穩健且簡單,並行化屬最佳化取捨,保留待人工評估。" } ]