From 402630fa695c0427fa8227c0c62da69b8b7ccbbd Mon Sep 17 00:00:00 2001 From: Jeffery Date: Fri, 26 Jun 2026 15:26:08 +0800 Subject: [PATCH] =?UTF-8?q?fix(json/comments):=20validateJSONArrayFile=20?= =?UTF-8?q?=E5=85=88=E9=A9=97=E8=AD=89=E5=86=8D=E5=AF=AB=E5=85=A5=E9=81=BF?= =?UTF-8?q?=E5=85=8D=E6=AF=80=E6=90=8D=E5=8E=9F=E6=AA=94=E3=80=81findingRo?= =?UTF-8?q?w=20=E5=B0=8D=E7=A9=BA=E5=80=BC=E9=98=B2=E5=91=86?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- app/comments.js | 11 +++--- app/json.js | 6 ++-- app/test/json.test.js | 79 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 6 deletions(-) diff --git a/app/comments.js b/app/comments.js index f3c21c1..2332873 100644 --- a/app/comments.js +++ b/app/comments.js @@ -7,17 +7,20 @@ import { ok, line, warn } from './log.js'; const LEVEL_EMOJI = { critical: '🔴', warning: '🟡', info: '🔵' }; const LEVEL_LABEL = { critical: '嚴重', warning: '警告', info: '建議' }; const LEVEL_ORDER = ['critical', 'warning', 'info']; +// 預先把等級對應到排序索引，bySeverity 排序時直接 O(1) 取值，省去每次比較的 includes + indexOf 掃描。 +const LEVEL_RANK = new Map(LEVEL_ORDER.map((level, index) => [level, index])); /** * 將單一 finding 格式化為 Markdown 表格的一列（等級｜審查員｜位置｜建議）。 * * @param {{ level?: string, role?: string, location?: string, suggestion?: string }} f * 單筆審查問題物件。`level` 若不在 critical/warning/info 之內，emoji 留空、標籤回退為原始 level 值； - * `role`、`location`、`suggestion` 直接內嵌字串（未定義時會輸出 undefined 字樣）。傳入 null/undefined 會拋 TypeError（需人工確認是否需防呆）。 - * @returns {string} 形如 `| 🔴 嚴重 | role | location | suggestion |` 的表格列字串。 + * `role`、`location`、`suggestion` 直接內嵌字串（未定義時會輸出 undefined 字樣）。傳入 null/undefined 時回傳空字串（已防呆，不會拋例外）。 + * @returns {string} 形如 `| 🔴 嚴重 | role | location | suggestion |` 的表格列字串；`f` 為空值時回傳空字串。 * @remarks 內部輔助函式，供 {@link buildTable} 逐列組裝表格使用，本身不含換行。 */ function findingRow(f) { + if (!f) return ''; return `| ${LEVEL_EMOJI[f.level] || ''} ${LEVEL_LABEL[f.level] || f.level} | ${f.role} | ${f.location} | ${f.suggestion} |`; } @@ -50,8 +53,8 @@ const levelText = f => `${LEVEL_EMOJI[f.level] || ''} ${LEVEL_LABEL[f.level] || * @remarks 不在 LEVEL_ORDER 內的等級一律視為最低優先（排在最後）；location 未定義時以空字串參與比較，因此排序穩定不會丟例外。 */ const bySeverity = (a, b) => { - const aLevel = LEVEL_ORDER.includes(a.level) ? LEVEL_ORDER.indexOf(a.level) : LEVEL_ORDER.length; - const bLevel = LEVEL_ORDER.includes(b.level) ? LEVEL_ORDER.indexOf(b.level) : LEVEL_ORDER.length; + const aLevel = LEVEL_RANK.has(a.level) ? LEVEL_RANK.get(a.level) : LEVEL_ORDER.length; + const bLevel = LEVEL_RANK.has(b.level) ? LEVEL_RANK.get(b.level) : LEVEL_ORDER.length; if (aLevel !== bLevel) return aLevel - bLevel; return String(a.location || '').localeCompare(String(b.location || '')); }; diff --git a/app/json.js b/app/json.js index 4529ca0..a9ced67 100644 --- a/app/json.js +++ b/app/json.js @@ -107,8 +107,10 @@ export async function validateJSONArrayFile(fullPath, label, repairer = repairJS try { const original = readJSONText(fullPath, label); const repaired = await repairer(fullPath, label, original); - fs.writeFileSync(fullPath, repaired.endsWith('\n') ? repaired : `${repaired}\n`, 'utf8'); - JSON.parse(readJSONText(fullPath, label)); + const normalized = repaired.endsWith('\n') ? repaired : `${repaired}\n`; + // 先驗證修復結果是否為合法 JSON；無效就在寫檔前丟出，避免用毀損內容覆寫原檔。 + JSON.parse(normalized); + fs.writeFileSync(fullPath, normalized, 'utf8'); ok(`${label} 已由 AI 修正並通過再次驗證`); return { exists: true, valid: true, repaired: true }; } catch (repairErr) { diff --git a/app/test/json.test.js b/app/test/json.test.js index 6ea7f79..0b8b55d 100644 --- a/app/test/json.test.js +++ b/app/test/json.test.js @@ -140,6 +140,85 @@ describe('json helpers', () => { }); }); +describe('validateJSONArrayFile repair failure paths', () => { + let workspace; + + beforeEach(() => { + workspace = fs.mkdtempSync(path.join(os.tmpdir(), 'json-test-repair-')); + }); + + afterEach(() => { + fs.rmSync(workspace, { recursive: true, force: true }); + }); + + it('overwrites the invalid file with the valid array returned by the repairer', async () => { + const fullPath = path.join(workspace, '.gitea/ai-review/findings.json'); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + fs.writeFileSync(fullPath, '{ this is not json', 'utf8'); + + let receivedOriginal; + const result = await validateJSONArrayFile( + fullPath, + '.gitea/ai-review/findings.json', + async (passedPath, passedLabel, original) => { + // repairer is called with (fullPath, label, original) per json.js line 109 + assert.equal(passedPath, fullPath); + assert.equal(passedLabel, '.gitea/ai-review/findings.json'); + receivedOriginal = original; + return '[{"id":1},{"id":2}]'; + } + ); + + assert.equal(receivedOriginal, '{ this is not json'); + assert.deepEqual(result, { exists: true, valid: true, repaired: true }); + // file is overwritten with the repaired content, trailing newline appended (line 110) + const written = fs.readFileSync(fullPath, 'utf8'); + assert.equal(written, '[{"id":1},{"id":2}]\n'); + assert.deepEqual(JSON.parse(written), [{ id: 1 }, { id: 2 }]); + }); + + it('throws when the repaired text is still invalid JSON and does NOT overwrite the original file', async () => { + const fullPath = path.join(workspace, '.gitea/ai-review/findings.json'); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + const originalBytes = '{ still broken'; + fs.writeFileSync(fullPath, originalBytes, 'utf8'); + + const stillInvalid = 'not a json array either'; + await assert.rejects( + () => validateJSONArrayFile( + fullPath, + '.gitea/ai-review/findings.json', + async () => stillInvalid + ), + SyntaxError + ); + + // json.js validates the repaired text in-memory BEFORE writing, so an invalid + // repair throws without corrupting the file — the original bytes are preserved. + const after = fs.readFileSync(fullPath, 'utf8'); + assert.equal(after, originalBytes); + }); + + it('propagates an error thrown by the repairer', async () => { + const fullPath = path.join(workspace, '.gitea/ai-review/findings.json'); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + fs.writeFileSync(fullPath, '{ broken', 'utf8'); + + const boom = new Error('repairer exploded'); + await assert.rejects( + () => validateJSONArrayFile( + fullPath, + '.gitea/ai-review/findings.json', + async () => { throw boom; } + ), + /repairer exploded/ + ); + + // repairer threw before any write happened, so the original bytes remain untouched (line 109) + assert.equal(fs.readFileSync(fullPath, 'utf8'), '{ broken'); + }); +}); + describe('repairJSONArrayWithAI', () => { it('returns a clean JSON array string parseable by the caller', async () => { const repaired = await repairJSONArrayWithAI(