處理 AI review findings 並改寫 Node.js entrypoint #1

Merged
jiantw83 merged 58 commits from ai-review-resolve/20260624102518 into develop 2026-06-24 14:09:27 +00:00
Showing only changes of commit 83de97b35a - Show all commits
+105 -27
View File
@@ -1,38 +1,78 @@
#!/usr/bin/env node #!/usr/bin/env node
const fs = require("fs"); const fs = require("fs");
const os = require("os");
const path = require("path"); const path = require("path");
const crypto = require("crypto"); const crypto = require("crypto");
const { spawn } = require("child_process"); const { spawn } = require("child_process");
const DEFAULT_PROMPT = "請自我介紹"; const DEFAULT_PROMPT = "請自我介紹";
const createdPaths = new Set(); const FILE_MODE_PRIVATE = 0o600;
const DIR_MODE_PRIVATE = 0o700;
const DEFAULT_CODEX_TIMEOUT_MS = 30 * 60 * 1000;
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Bard
問題:全域變數 createdPaths 在檔案層級被宣告,讓函式產生強依賴,缺乏封裝性,讀起來不夠優雅。
建議:將臨時檔案管理邏輯封裝成一個類別(如 TempFileRegistry),讓狀態更具備物件導向的封裝性。

**嚴重等級**:🔵 建議 **審查員**:Bard **問題**:全域變數 `createdPaths` 在檔案層級被宣告,讓函式產生強依賴,缺乏封裝性,讀起來不夠優雅。 **建議**:將臨時檔案管理邏輯封裝成一個類別(如 `TempFileRegistry`),讓狀態更具備物件導向的封裝性。
const DEFAULT_OUTPUT_LIMIT_BYTES = 1024 * 1024;
function removeIfCreated(filePath) { class TempFileRegistry {
if (!filePath || !createdPaths.has(filePath)) { constructor() {
this.files = new Set();
this.dirs = new Set();
}
trackFile(filePath) {
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Leo
問題:在 removeIfCreated 函式中靜默捕捉錯誤 (catch { ... }),這會遮蔽潛在的權限或檔案系統問題,使除錯困難。
建議:建議至少加上 console.error 或在開發/除錯模式下將錯誤拋出,以便在清除失敗時能收到警示。

**嚴重等級**:🟡 警告 **審查員**:Leo **問題**:在 `removeIfCreated` 函式中靜默捕捉錯誤 (`catch { ... }`),這會遮蔽潛在的權限或檔案系統問題,使除錯困難。 **建議**:建議至少加上 `console.error` 或在開發/除錯模式下將錯誤拋出,以便在清除失敗時能收到警示。
this.files.add(filePath);
}
trackDir(dirPath) {
this.dirs.add(dirPath);
}
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Rogue
問題:在 makeTempFile 中,使用了 Math.random().toString(16).slice(2) 來生成隨機檔名。對於高頻率呼叫的場景,這會產生不必要的計算開銷與效能損耗。
建議:建議使用 Node.js 內建的 crypto.randomBytescrypto.randomUUID,雖然效能略有差異但更具安全性與標準化,且能減少字串轉換次數。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:在 `makeTempFile` 中,使用了 `Math.random().toString(16).slice(2)` 來生成隨機檔名。對於高頻率呼叫的場景,這會產生不必要的計算開銷與效能損耗。 **建議**:建議使用 Node.js 內建的 `crypto.randomBytes` 或 `crypto.randomUUID`,雖然效能略有差異但更具安全性與標準化,且能減少字串轉換次數。
removeFile(filePath) {
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Bard
問題:隨機檔案名稱的生成邏輯過於冗長且複雜,破壞了程式碼的簡潔美感。
建議:建議使用 Node.js 原生的 crypto 模組,例如 crypto.randomBytes(16).toString('hex'),讓產生的字串更優雅、清晰。

**嚴重等級**:🔵 建議 **審查員**:Bard **問題**:隨機檔案名稱的生成邏輯過於冗長且複雜,破壞了程式碼的簡潔美感。 **建議**:建議使用 Node.js 原生的 `crypto` 模組,例如 `crypto.randomBytes(16).toString('hex')`,讓產生的字串更優雅、清晰。
if (!filePath || !this.files.has(filePath)) {
return; return;
} }
try { try {
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Mage
問題:在 makeTempFile 中使用了 fs.openSync(filePath, "wx", 0o600)。如果在 fs.closeSync(fd) 之前程式因例外或強制終止(SIGKILL),該檔案會留在硬碟上直到下次清理或手動刪除,且其檔案描述子會持續開啟直到 process 結束。
建議:建議使用 fs.mkdtempSync 建立獨立目錄,將所有臨時檔案放入該目錄,並在 cleanup 時直接移除整個目錄,以確保清理的原子性與完整性。

**嚴重等級**:🟡 警告 **審查員**:Mage **問題**:在 `makeTempFile` 中使用了 `fs.openSync(filePath, "wx", 0o600)`。如果在 `fs.closeSync(fd)` 之前程式因例外或強制終止(SIGKILL),該檔案會留在硬碟上直到下次清理或手動刪除,且其檔案描述子會持續開啟直到 process 結束。 **建議**:建議使用 `fs.mkdtempSync` 建立獨立目錄,將所有臨時檔案放入該目錄,並在 `cleanup` 時直接移除整個目錄,以確保清理的原子性與完整性。
Review

嚴重等級🔵 建議
審查員:Bard
問題:檔案權限(如 0o600, 0o700)以數字字面量多次出現,散落在程式碼中,降低了可讀性與一致性。
建議:在檔案上方定義權限常數(例如 const FILE_MODE_PRIVATE = 0o600;),讓語義更清晰。

**嚴重等級**:🔵 建議 **審查員**:Bard **問題**:檔案權限(如 `0o600`, `0o700`)以數字字面量多次出現,散落在程式碼中,降低了可讀性與一致性。 **建議**:在檔案上方定義權限常數(例如 `const FILE_MODE_PRIVATE = 0o600;`),讓語義更清晰。
fs.rmSync(filePath, { force: true }); fs.rmSync(filePath, { force: true });
this.files.delete(filePath);
} catch (error) { } catch (error) {
console.error(`Unable to remove temporary file: ${error.message}`); console.error(`Unable to remove temporary file: ${error.message}`);
} }
} }
function cleanup() { cleanup() {
for (const filePath of Array.from(createdPaths).reverse()) { for (const filePath of Array.from(this.files).reverse()) {
removeIfCreated(filePath); this.removeFile(filePath);
} }
for (const dirPath of Array.from(this.dirs).reverse()) {
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Rogue
問題:在 TempFileRegistrycleanup 方法中,每次呼叫都使用 Array.from 將 Set 轉換為陣列,這在頻繁清理時會產生無謂的記憶體開銷。
建議:若無強烈反向迭代的需求,可考慮直接使用 forEach 遍歷 Set。若有嚴格順序需求,建議改用其他結構管理,避免每次 cleanup 都額外配置陣列。

**嚴重等級**:🔵 建議 **審查員**:Rogue **問題**:在 `TempFileRegistry` 的 `cleanup` 方法中,每次呼叫都使用 `Array.from` 將 Set 轉換為陣列,這在頻繁清理時會產生無謂的記憶體開銷。 **建議**:若無強烈反向迭代的需求,可考慮直接使用 `forEach` 遍歷 Set。若有嚴格順序需求,建議改用其他結構管理,避免每次 cleanup 都額外配置陣列。
try {
fs.rmSync(dirPath, { force: true, recursive: true });
this.dirs.delete(dirPath);
} catch (error) {
console.error(`Unable to remove temporary directory: ${error.message}`);
}
}
}
}
const tempFiles = new TempFileRegistry();
function cleanup() {
tempFiles.cleanup();
}
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Rogue
問題:在 validateAuth 中,為了驗證 base64 字串是否符合 base64 格式,進行了多次正規表達式替換與編解碼運算(如 encodedAuth.replaceBuffer.fromdecoded.toString('base64') 等),這在每次執行都會發生的情況下,浪費了不必要的 CPU 週期。
建議:如果目的只是驗證結構,建議盡量簡化邏輯。可以直接將字串嘗試轉換為 Buffer 並檢查 toString('base64') 是否匹配,避免多重正規表達式替換。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:在 `validateAuth` 中,為了驗證 base64 字串是否符合 base64 格式,進行了多次正規表達式替換與編解碼運算(如 `encodedAuth.replace`、`Buffer.from`、`decoded.toString('base64')` 等),這在每次執行都會發生的情況下,浪費了不必要的 CPU 週期。 **建議**:如果目的只是驗證結構,建議盡量簡化邏輯。可以直接將字串嘗試轉換為 Buffer 並檢查 `toString('base64')` 是否匹配,避免多重正規表達式替換。
function makeTempDir(dir) {
const tempDir = fs.mkdtempSync(path.join(dir, ".codex-action-"));
fs.chmodSync(tempDir, DIR_MODE_PRIVATE);
tempFiles.trackDir(tempDir);
return tempDir;
} }
function makeTempFile(dir, prefix) { function makeTempFile(dir, prefix) {
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Bard
問題:Base64 的驗證過程充滿了複雜的字串正規化與取代操作,讀起來像是在解迷宮,而非驗證身分。
建議:將驗證邏輯拆解或簡化,明確劃分「解碼」、「正規化」與「比較」三個步驟,提升程式碼的可讀性與可維護性。

**嚴重等級**:🟡 警告 **審查員**:Bard **問題**:Base64 的驗證過程充滿了複雜的字串正規化與取代操作,讀起來像是在解迷宮,而非驗證身分。 **建議**:將驗證邏輯拆解或簡化,明確劃分「解碼」、「正規化」與「比較」三個步驟,提升程式碼的可讀性與可維護性。
const random = crypto.randomBytes(16).toString("hex"); const random = crypto.randomBytes(16).toString("hex");
const filePath = path.join(dir, `${prefix}.${random}`); const filePath = path.join(dir, `${prefix}.${random}`);
const fd = fs.openSync(filePath, "wx", 0o600); const fd = fs.openSync(filePath, "wx", FILE_MODE_PRIVATE);
fs.closeSync(fd); fs.closeSync(fd);
createdPaths.add(filePath); tempFiles.trackFile(filePath);
return filePath; return filePath;
} }
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Mage
問題:在 validateAuth 中,僅透過 JSON.parse 檢查 JSON 格式,但未針對 Codex 預期的 auth.json 結構(如必要的欄位)進行 Schema 驗證。如果傳入的 JSON 格式正確但內容無效,可能會導致 codex exec 在後續執行時失敗。
建議:建議加入對 JSON 內容的簡單結構驗證(例如確認是否有 token 或必要的連線設定欄位)。

**嚴重等級**:🟡 警告 **審查員**:Mage **問題**:在 `validateAuth` 中,僅透過 `JSON.parse` 檢查 JSON 格式,但未針對 Codex 預期的 auth.json 結構(如必要的欄位)進行 Schema 驗證。如果傳入的 JSON 格式正確但內容無效,可能會導致 `codex exec` 在後續執行時失敗。 **建議**:建議加入對 JSON 內容的簡單結構驗證(例如確認是否有 `token` 或必要的連線設定欄位)。
Review

嚴重等級🟡 警告
審查員:Mage
問題:在 appendGithubOutput 函式中,當 output 內容極大時,此處會將整個 output 字串在記憶體中進行檢查(output.includes(delimiter))與多次複製。這可能導致在處理極端長度輸出時發生記憶體不足的問題。
建議:建議限制 delimiter 嘗試次數,或在檢查時避免讀取整個 output 字串,改用串流處理方式。

**嚴重等級**:🟡 警告 **審查員**:Mage **問題**:在 `appendGithubOutput` 函式中,當 `output` 內容極大時,此處會將整個 `output` 字串在記憶體中進行檢查(`output.includes(delimiter)`)與多次複製。這可能導致在處理極端長度輸出時發生記憶體不足的問題。 **建議**:建議限制 `delimiter` 嘗試次數,或在檢查時避免讀取整個 `output` 字串,改用串流處理方式。
Review

嚴重等級🟡 警告
審查員:Maya
問題:OutputCollector 的截斷機制未被測試。
建議:增加測試案例模擬輸出超過 DEFAULT_OUTPUT_LIMIT_BYTES,驗證截斷提示。

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:OutputCollector 的截斷機制未被測試。 **建議**:增加測試案例模擬輸出超過 DEFAULT_OUTPUT_LIMIT_BYTES,驗證截斷提示。
1
@@ -50,7 +90,7 @@ function appendGithubOutput(status, output) {
fs.appendFileSync( fs.appendFileSync(
outputFile, outputFile,
`status=${status}\noutput<<${delimiter}\n${output}${output.endsWith("\n") ? "" : "\n"}${delimiter}\n`, `status=${status}\noutput<<${delimiter}\n${output}${output.endsWith("\n") ? "" : "\n"}${delimiter}\n`,
{ encoding: "utf8", mode: 0o600 }, { encoding: "utf8", mode: FILE_MODE_PRIVATE },
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Bard
問題:makeTempDir 內部使用了硬編碼的 '.codex-action-' 前綴。
建議:將前綴提取為常數或設定檔參數。

**嚴重等級**:🔵 建議 **審查員**:Bard **問題**:makeTempDir 內部使用了硬編碼的 '.codex-action-' 前綴。 **建議**:將前綴提取為常數或設定檔參數。
); );
} }
5
@@ -75,7 +115,7 @@ function validateAuth(encodedAuth, authFile) {
fail("OAUTH must be valid base64 encoded Codex auth.json."); fail("OAUTH must be valid base64 encoded Codex auth.json.");
} }
fs.writeFileSync(authFile, decoded, { mode: 0o600 }); fs.writeFileSync(authFile, decoded, { mode: FILE_MODE_PRIVATE });
let parsed; let parsed;
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Bard
問題:validateAuth 函式中對於 Base64 的正規化與驗證邏輯混雜在一起,使用了大量的取代與判斷,讀起來節奏凌亂,缺乏優雅感。
建議:將驗證邏輯與基礎轉換邏輯抽離,建議提取一個輔助函式專門負責 Base64 格式檢查,使主要流程清晰明瞭。

**嚴重等級**:🟡 警告 **審查員**:Bard **問題**:validateAuth 函式中對於 Base64 的正規化與驗證邏輯混雜在一起,使用了大量的取代與判斷,讀起來節奏凌亂,缺乏優雅感。 **建議**:將驗證邏輯與基礎轉換邏輯抽離,建議提取一個輔助函式專門負責 Base64 格式檢查,使主要流程清晰明瞭。
try { try {
4
@@ -89,8 +129,29 @@ function validateAuth(encodedAuth, authFile) {
} }
} }
function parsePositiveInteger(value, fallback) {
const parsed = Number.parseInt(value || "", 10);
return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback;
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Maya
問題:測試檔案 tests/entrypoint_test.sh 有測試 missing_codex_command,這很好。但實作中對於 codex 執行失敗的各種細節(如權限不足、找不到 binary 等)都統一處理為 status: 1 和簡單的訊息,測試僅驗證了 failure 狀態,未驗證具體錯誤來源。
建議:考慮在 main.js 中根據不同的錯誤類型回傳更細緻的 status code,並在測試中驗證這些 code,能更精確地協助 CI 使用者除錯。

**嚴重等級**:🔵 建議 **審查員**:Maya **問題**:測試檔案 `tests/entrypoint_test.sh` 有測試 `missing_codex_command`,這很好。但實作中對於 `codex` 執行失敗的各種細節(如權限不足、找不到 binary 等)都統一處理為 `status: 1` 和簡單的訊息,測試僅驗證了 failure 狀態,未驗證具體錯誤來源。 **建議**:考慮在 `main.js` 中根據不同的錯誤類型回傳更細緻的 status code,並在測試中驗證這些 code,能更精確地協助 CI 使用者除錯。
Review

嚴重等級🔵 建議
審查員:Maya
問題:對於 codex 執行失敗的各種細節(權限不足、找不到 binary 等)都統一處理為 status: 1,測試未驗證具體錯誤來源。
建議:根據錯誤類型回傳細緻 status code,並在測試中驗證這些 code。

**嚴重等級**:🔵 建議 **審查員**:Maya **問題**:對於 codex 執行失敗的各種細節(權限不足、找不到 binary 等)都統一處理為 status: 1,測試未驗證具體錯誤來源。 **建議**:根據錯誤類型回傳細緻 status code,並在測試中驗證這些 code。
}
function truncateOutput(chunks, nextChunk, maxBytes) {
const currentSize = chunks.reduce((total, chunk) => total + chunk.length, 0);
const available = maxBytes - currentSize;
if (available <= 0) {
return false;
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Rogue
問題:validateAuth 中重複執行 decoded.toString('base64') 並進行 normalizedBase64 處理,極度浪費資源。
建議:移除這些無謂的重新編碼比較,直接嘗試解碼。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:validateAuth 中重複執行 decoded.toString('base64') 並進行 normalizedBase64 處理,極度浪費資源。 **建議**:移除這些無謂的重新編碼比較,直接嘗試解碼。
}
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Assassin
問題:將 OAUTH 環境變數內容解碼並直接寫入 auth.json。雖然有檢查 base64 格式與 JSON 結構,但若解碼後的 JSON 內容包含惡意配置(如惡意插件路徑或偽造的 API 憑證),可能導致後續 codex CLI 在執行時被劫持或洩漏資料。
建議:除了驗證 JSON 結構外,應進一步驗證 auth.json 內的欄位是否符合預期格式,並限制其檔案權限為 600(已做),確保容器內其他行程無法讀取。

**嚴重等級**:🟡 警告 **審查員**:Assassin **問題**:將 `OAUTH` 環境變數內容解碼並直接寫入 `auth.json`。雖然有檢查 base64 格式與 JSON 結構,但若解碼後的 JSON 內容包含惡意配置(如惡意插件路徑或偽造的 API 憑證),可能導致後續 `codex` CLI 在執行時被劫持或洩漏資料。 **建議**:除了驗證 JSON 結構外,應進一步驗證 `auth.json` 內的欄位是否符合預期格式,並限制其檔案權限為 `600`(已做),確保容器內其他行程無法讀取。
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Leo
問題main 函式過於龐大且職責過多,它同時負責了訊號處理、路徑創建、檔案鎖定、認證驗證以及執行核心邏輯,這降低了程式碼的可讀性與單元測試的困難度。
建議:建議將 main 拆分為 validateInputsetupAuthrunCodexActioncleanup 等子函式,讓職責分離。

**嚴重等級**:🟡 警告 **審查員**:Leo **問題**:`main` 函式過於龐大且職責過多,它同時負責了訊號處理、路徑創建、檔案鎖定、認證驗證以及執行核心邏輯,這降低了程式碼的可讀性與單元測試的困難度。 **建議**:建議將 `main` 拆分為 `validateInput`、`setupAuth`、`runCodexAction` 與 `cleanup` 等子函式,讓職責分離。
chunks.push(nextChunk.length > available ? nextChunk.subarray(0, available) : nextChunk);
return nextChunk.length <= available;
}
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Maya
問題:在 runCodex 函數中雖然有處理 child.on('error', ...),但若 codex 指令本身不存在(spawn ENOENT),這裡捕捉到的 error stack trace 可能會包含完整的系統路徑資訊,這在 CI 環境中屬於資訊洩漏風險。
建議:建議在錯誤處理中,針對 error.code === 'ENOENT' 做明確判斷,回傳簡潔的錯誤訊息(例如「找不到 codex 指令」),而非直接回傳完整的 error.message

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:在 `runCodex` 函數中雖然有處理 `child.on('error', ...)`,但若 `codex` 指令本身不存在(spawn ENOENT),這裡捕捉到的 error stack trace 可能會包含完整的系統路徑資訊,這在 CI 環境中屬於資訊洩漏風險。 **建議**:建議在錯誤處理中,針對 `error.code === 'ENOENT'` 做明確判斷,回傳簡潔的錯誤訊息(例如「找不到 codex 指令」),而非直接回傳完整的 `error.message`。
Review

嚴重等級🟡 警告
審查員:Maya
問題:spawn ENOENT 錯誤處理可能洩漏系統路徑資訊。
建議:針對 error.code === 'ENOENT' 做明確判斷,回傳簡潔錯誤訊息而非完整 stack trace。

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:spawn ENOENT 錯誤處理可能洩漏系統路徑資訊。 **建議**:針對 error.code === 'ENOENT' 做明確判斷,回傳簡潔錯誤訊息而非完整 stack trace。
function runCodex(model, prompt) { function runCodex(model, prompt) {
return new Promise((resolve) => { return new Promise((resolve) => {
const timeoutMs = parsePositiveInteger(process.env.CODEX_TIMEOUT_MS, DEFAULT_CODEX_TIMEOUT_MS);
const outputLimitBytes = parsePositiveInteger(process.env.CODEX_OUTPUT_LIMIT_BYTES, DEFAULT_OUTPUT_LIMIT_BYTES);
const workspace = process.env.GITHUB_WORKSPACE || process.cwd();
// This Docker Action runs inside an ephemeral CI container where Codex must be // This Docker Action runs inside an ephemeral CI container where Codex must be
// able to edit the checked-out workspace without interactive approvals. // able to edit the checked-out workspace without interactive approvals.
const child = spawn( const child = spawn(
@@ -103,32 +164,48 @@ function runCodex(model, prompt) {
model, model,
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Maya
問題:當 child.on('close', ...) 觸發時,若 code 為 null,預設回傳 1。雖然這處理了非預期終止,但缺少對 signal 終止(例如 SIGKILL)的具體紀錄,只知道失敗,無法區分是指令執行錯誤還是被系統殺掉。
建議:在 close 事件中,若 code 為 null,可以檢查 signal 參數(若有),並在 output 中加入被哪個 signal 終止的資訊,增加除錯便利性。

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:當 `child.on('close', ...)` 觸發時,若 `code` 為 null,預設回傳 1。雖然這處理了非預期終止,但缺少對 signal 終止(例如 SIGKILL)的具體紀錄,只知道失敗,無法區分是指令執行錯誤還是被系統殺掉。 **建議**:在 `close` 事件中,若 `code` 為 null,可以檢查 `signal` 參數(若有),並在 output 中加入被哪個 signal 終止的資訊,增加除錯便利性。
Review

嚴重等級🟡 警告
審查員:Maya
問題:close 事件缺少對 signal 終止(如 SIGKILL)的具體紀錄。
建議:檢查 signal 參數,並在 output 中加入被哪個 signal 終止的資訊。

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:close 事件缺少對 signal 終止(如 SIGKILL)的具體紀錄。 **建議**:檢查 signal 參數,並在 output 中加入被哪個 signal 終止的資訊。
prompt, prompt,
], ],
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Bard
問題:runCodex 函式過於臃腫,包含了執行、超時處理、輸出截斷與錯誤捕捉等多重責任,這段旋律太過冗長且複雜。
建議:建議將輸出處理 (Output truncation logic) 與超時設定分離為獨立函式,以提升函式的可讀性與維護性。

**嚴重等級**:🟡 警告 **審查員**:Bard **問題**:runCodex 函式過於臃腫,包含了執行、超時處理、輸出截斷與錯誤捕捉等多重責任,這段旋律太過冗長且複雜。 **建議**:建議將輸出處理 (Output truncation logic) 與超時設定分離為獨立函式,以提升函式的可讀性與維護性。
Review

嚴重等級🟡 警告
審查員:Rogue
問題:在 runCodex 的輸出處理中,每收到一塊資料就進行 Buffer.concattoString,若資料量大或封包碎,會產生大量不必要的記憶體配置與垃圾回收 (GC) 壓力。
建議:只在輸出完成、達到限制或必須輸出結果時才進行合併與轉型,不要在處理每一塊資料時都執行。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:在 `runCodex` 的輸出處理中,每收到一塊資料就進行 `Buffer.concat` 與 `toString`,若資料量大或封包碎,會產生大量不必要的記憶體配置與垃圾回收 (GC) 壓力。 **建議**:只在輸出完成、達到限制或必須輸出結果時才進行合併與轉型,不要在處理每一塊資料時都執行。
Review

嚴重等級🟡 警告
審查員:Bard
問題:runCodex 函式過於臃腫,包含了過多職責。
建議:建議將輸出處理與超時設定分離為獨立函式。

**嚴重等級**:🟡 警告 **審查員**:Bard **問題**:runCodex 函式過於臃腫,包含了過多職責。 **建議**:建議將輸出處理與超時設定分離為獨立函式。
{ stdio: ["ignore", "pipe", "pipe"] }, { cwd: workspace, stdio: ["ignore", "pipe", "pipe"] },
); );
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Maya
問題:parsePositiveInteger 的輸入回退機制未經測試。
建議:針對設定變數傳入無效數字或非法格式場景,驗證預設值套用。

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:parsePositiveInteger 的輸入回退機制未經測試。 **建議**:針對設定變數傳入無效數字或非法格式場景,驗證預設值套用。
const outputChunks = []; const outputChunks = [];
let outputTruncated = false;
const appendOutput = (chunk) => { const appendOutput = (chunk) => {
outputChunks.push(chunk); if (!truncateOutput(outputChunks, chunk, outputLimitBytes)) {
outputTruncated = true;
}
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Mage
問題:在 child.on('close', ...) 事件中,使用 Buffer.concat(outputChunks).toString() 將所有輸出轉為單一字串。如果 outputChunks 總大小接近 DEFAULT_OUTPUT_LIMIT_BYTES (1MB),這會導致瞬間記憶體使用量增加,且對於極大輸出,字串轉換本身亦有潛在的負載。
建議:考慮使用 Buffer 處理後續輸出,或在達到 outputLimitBytes 時,僅保存 Buffer 片段即可,不必轉為大字串。

**嚴重等級**:🟡 警告 **審查員**:Mage **問題**:在 `child.on('close', ...)` 事件中,使用 `Buffer.concat(outputChunks).toString()` 將所有輸出轉為單一字串。如果 `outputChunks` 總大小接近 `DEFAULT_OUTPUT_LIMIT_BYTES` (1MB),這會導致瞬間記憶體使用量增加,且對於極大輸出,字串轉換本身亦有潛在的負載。 **建議**:考慮使用 `Buffer` 處理後續輸出,或在達到 `outputLimitBytes` 時,僅保存 `Buffer` 片段即可,不必轉為大字串。
return Buffer.concat(outputChunks).toString(); return Buffer.concat(outputChunks).toString();
}; };
const timeout = setTimeout(() => {
child.kill("SIGTERM");
appendOutput(Buffer.from(`Codex execution timed out after ${timeoutMs} ms.\n`));
}, timeoutMs);
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Bard
問題:spawn 函式參數陣列過長且散亂,閱讀性較差。
建議:將參數拆分為數組變數並展開傳遞。

**嚴重等級**:🟡 警告 **審查員**:Bard **問題**:spawn 函式參數陣列過長且散亂,閱讀性較差。 **建議**:將參數拆分為數組變數並展開傳遞。
child.stdout.pipe(process.stdout);
child.stderr.pipe(process.stdout);
child.stdout.on("data", (chunk) => { child.stdout.on("data", (chunk) => {
process.stdout.write(chunk); if (!truncateOutput(outputChunks, chunk, outputLimitBytes)) {
outputChunks.push(chunk); outputTruncated = true;
Ghost marked this conversation as resolved
Review

嚴重等級🔴 嚴重
審查員:Mage
問題:在 setupAuth 中,lockPath 使用 os.tmpdir()。在共享環境中,如果 CODEX_HOME 字串相同,會導致所有 process 競爭同一個鎖檔,且如果其他無關的 process 也剛好在 os.tmpdir() 中建立相同名稱的檔案,會導致誤判或鎖定失敗。
建議:應在 CODEX_HOME 內部建立鎖檔,而非使用全域的 os.tmpdir(),或者包含更具唯一性的識別碼(如 PID 或更長的路徑雜湊)以確保鎖的隔離性。

**嚴重等級**:🔴 嚴重 **審查員**:Mage **問題**:在 `setupAuth` 中,`lockPath` 使用 `os.tmpdir()`。在共享環境中,如果 `CODEX_HOME` 字串相同,會導致所有 process 競爭同一個鎖檔,且如果其他無關的 process 也剛好在 `os.tmpdir()` 中建立相同名稱的檔案,會導致誤判或鎖定失敗。 **建議**:應在 `CODEX_HOME` 內部建立鎖檔,而非使用全域的 `os.tmpdir()`,或者包含更具唯一性的識別碼(如 PID 或更長的路徑雜湊)以確保鎖的隔離性。
Review

嚴重等級🔴 嚴重
審查員:Maya
問題:Codex 超時處理路徑未經測試,無法確保 SIGTERM 能成功發送與訊息正確產出。
建議:增加測試案例模擬長期睡眠(如 sleep 10),驗證超時機制與輸出訊息。

**嚴重等級**:🔴 嚴重 **審查員**:Maya **問題**:Codex 超時處理路徑未經測試,無法確保 SIGTERM 能成功發送與訊息正確產出。 **建議**:增加測試案例模擬長期睡眠(如 sleep 10),驗證超時機制與輸出訊息。
}
}); });
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Rogue
問題:在 setupAuth 中,lockPath 檔名產生使用了 Buffer.from(codexHome).toString("hex")。如果 codexHome 非常長,這個檔名可能會超過作業系統的檔案名稱長度限制(通常為 255 bytes),導致鎖定失敗,進而阻斷整個流程。
建議:改用 crypto.createHash('sha256').update(codexHome).digest('hex') 來產生固定長度的雜湊值作為檔名的一部分,既安全又保證長度可控。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:在 setupAuth 中,`lockPath` 檔名產生使用了 `Buffer.from(codexHome).toString("hex")`。如果 `codexHome` 非常長,這個檔名可能會超過作業系統的檔案名稱長度限制(通常為 255 bytes),導致鎖定失敗,進而阻斷整個流程。 **建議**:改用 `crypto.createHash('sha256').update(codexHome).digest('hex')` 來產生固定長度的雜湊值作為檔名的一部分,既安全又保證長度可控。
child.stderr.on("data", (chunk) => { child.stderr.on("data", (chunk) => {
process.stdout.write(chunk); if (!truncateOutput(outputChunks, chunk, outputLimitBytes)) {
outputChunks.push(chunk); outputTruncated = true;
}
}); });
child.on("error", (error) => { child.on("error", (error) => {
clearTimeout(timeout);
const output = appendOutput(Buffer.from(`${error.message}\n`)); const output = appendOutput(Buffer.from(`${error.message}\n`));
resolve({ status: 1, output }); resolve({ status: 1, output });
}); });
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Mage
問題:在 setupAuth 中,在 fs.copyFileSync(authFile, authPath) 後立即 removeIfCreated(authFile),但在這期間如果發生 process 中斷,auth.json 可能會以不安全的權限(預設)或不完整的狀態寫入。
建議:建議使用 fs.renameSync 或在完成寫入與權限設定後再進行清理,並確保寫入過程中發生異常時能正確刪除該部分寫入的檔案。

**嚴重等級**:🟡 警告 **審查員**:Mage **問題**:在 `setupAuth` 中,在 `fs.copyFileSync(authFile, authPath)` 後立即 `removeIfCreated(authFile)`,但在這期間如果發生 process 中斷,auth.json 可能會以不安全的權限(預設)或不完整的狀態寫入。 **建議**:建議使用 `fs.renameSync` 或在完成寫入與權限設定後再進行清理,並確保寫入過程中發生異常時能正確刪除該部分寫入的檔案。
child.on("close", (code) => { child.on("close", (code) => {
const output = Buffer.concat(outputChunks).toString(); clearTimeout(timeout);
const truncationMessage = outputTruncated ? "\n[Output truncated]\n" : "";
const output = `${Buffer.concat(outputChunks).toString()}${truncationMessage}`;
resolve({ status: code ?? 1, output }); resolve({ status: code ?? 1, output });
}); });
}); });
@@ -165,19 +242,21 @@ function readConfig() {
function setupAuth(oauth, codexHome) { function setupAuth(oauth, codexHome) {
try { try {
fs.mkdirSync(codexHome, { recursive: true, mode: 0o700 }); fs.mkdirSync(codexHome, { recursive: true, mode: DIR_MODE_PRIVATE });
} catch { } catch {
fail("Unable to create CODEX_HOME."); fail("Unable to create CODEX_HOME.");
} }
const authFile = makeTempFile(codexHome, "auth"); const tempDir = makeTempDir(codexHome);
const authFile = makeTempFile(tempDir, "auth");
const authPath = path.join(codexHome, "auth.json"); const authPath = path.join(codexHome, "auth.json");
const lockPath = path.join(os.tmpdir(), `codex-auth-${Buffer.from(codexHome).toString("hex")}.lock`); const lockName = crypto.createHash("sha256").update(codexHome).digest("hex");
const lockPath = path.join(codexHome, `.codex-auth-${lockName}.lock`);
Ghost marked this conversation as resolved
Review

嚴重等級🔴 嚴重
審查員:Mage
問題:在 setupAuth 中,使用了 fs.renameSync 來確保原子性。然而在某些檔案系統中,若 authFileauthPath 不在同一個分區,renameSync 可能會失敗。此外,如果 codexHome 已存在且權限配置錯誤,fs.mkdirSync 可能會被忽略但後續存取失敗。
建議:建議確保 authFilecodexHome 處於相同掛載點,並增加對 fs.mkdirSync 後權限檢查的驗證。

**嚴重等級**:🔴 嚴重 **審查員**:Mage **問題**:在 `setupAuth` 中,使用了 `fs.renameSync` 來確保原子性。然而在某些檔案系統中,若 `authFile` 與 `authPath` 不在同一個分區,`renameSync` 可能會失敗。此外,如果 `codexHome` 已存在且權限配置錯誤,`fs.mkdirSync` 可能會被忽略但後續存取失敗。 **建議**:建議確保 `authFile` 與 `codexHome` 處於相同掛載點,並增加對 `fs.mkdirSync` 後權限檢查的驗證。
let lockHandle; let lockHandle;
try { try {
lockHandle = fs.openSync(lockPath, "wx", 0o600); lockHandle = fs.openSync(lockPath, "wx", FILE_MODE_PRIVATE);
createdPaths.add(lockPath); tempFiles.trackFile(lockPath);
} catch { } catch {
fail("Unable to lock Codex auth.json."); fail("Unable to lock Codex auth.json.");
} }
@@ -188,10 +267,9 @@ function setupAuth(oauth, codexHome) {
fail("Refusing to overwrite existing Codex auth.json."); fail("Refusing to overwrite existing Codex auth.json.");
} }
fs.copyFileSync(authFile, authPath); fs.renameSync(authFile, authPath);
fs.chmodSync(authPath, 0o600); fs.chmodSync(authPath, FILE_MODE_PRIVATE);
createdPaths.add(authPath); tempFiles.trackFile(authPath);
removeIfCreated(authFile);
return lockHandle; return lockHandle;
} }
1