From 8f909e53970eefd693a1163c8fcdde93ed4c01ad Mon Sep 17 00:00:00 2001 From: Jeffery Date: Sat, 11 Jul 2026 12:18:24 +0000 Subject: [PATCH] =?UTF-8?q?fix(=E5=AF=A9=E6=9F=A5=E6=B5=81=E7=A8=8B):=20?= =?UTF-8?q?=E4=BF=AE=E6=AD=A3=20bot=20=E8=B7=B3=E9=81=8E=E3=80=81JSON=20?= =?UTF-8?q?=E9=A9=97=E8=AD=89=E8=88=87=E6=8E=92=E9=99=A4=E6=AF=94=E5=B0=8D?= =?UTF-8?q?=E9=82=8A=E7=95=8C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/comments.js | 3 ++- src/findings.js | 2 +- src/gitea.js | 4 ++-- src/json.js | 5 ++++- src/resolve.js | 28 ++++++++++++++++++++-------- src/test/comments.test.js | 4 ++++ src/test/findings.test.js | 15 +++++++++++++++ src/test/gitea.test.js | 12 ++++++++++-- src/test/json.test.js | 12 ++++++++++++ src/test/resolve.test.js | 24 ++++++++++++++++++++++++ 10 files changed, 94 insertions(+), 15 deletions(-) diff --git a/src/comments.js b/src/comments.js index 2332873..3d298a5 100644 --- a/src/comments.js +++ b/src/comments.js @@ -69,7 +69,8 @@ export function parseLocation(location) { if (trimmed.includes(',')) return null; const match = trimmed.match(/^(.+?):(\d+)(?:-\d+)?$/); if (!match) return null; - return { file: match[1], line: Number(match[2]) }; + const line = Number(match[2]); + return line > 0 ? { file: match[1], line } : null; } /** 行內 comment 內容：等級／審查員／建議 */ diff --git a/src/findings.js b/src/findings.js index e65ef81..df740cf 100644 --- a/src/findings.js +++ b/src/findings.js @@ -548,7 +548,7 @@ export function applyExclusions(findings, exclusions) { const fPath = String(f.location).split(':')[0]; const exPath = ex.filePath || (ex.location ? String(ex.location).split(':')[0] : null); const findingText = normalizeText(f.suggestion || f.title || ''); - const exclusionText = ex.textKey || normalizeText(ex.text || ex.suggestion || ex.title || ''); + const exclusionText = normalizeText(ex.text || ex.original_finding || ex.suggestion || ex.title || ex.textKey || ''); const locationMatches = (!exPath || fPath === exPath); const roleMatches = (!ex.role || ex.role === f.role); const textMatches = !exclusionText || !findingText || findingText.includes(exclusionText) || exclusionText.includes(findingText); diff --git a/src/gitea.js b/src/gitea.js index 2786387..3c379a4 100644 --- a/src/gitea.js +++ b/src/gitea.js @@ -146,10 +146,10 @@ export async function getBranchHeadCommitMessage(branch = PR_HEAD_BRANCH) { */ export async function shouldSkipBotCommit({ sha = PR_HEAD_SHA || process.env.GITHUB_SHA, branch = PR_HEAD_BRANCH } = {}) { const shaMessage = await getCommitMessageBySha(sha); - if (sha && shaMessage.includes('[ai-review-bot]')) return true; + if (sha && shaMessage.includes('[ai-review-bot]') && getBotReviewOutcome(shaMessage) !== 'failure') return true; const branchMessage = await getBranchHeadCommitMessage(branch); - if (branch && branchMessage.includes('[ai-review-bot]')) return true; + if (branch && branchMessage.includes('[ai-review-bot]') && getBotReviewOutcome(branchMessage) !== 'failure') return true; return false; } diff --git a/src/json.js b/src/json.js index a9ced67..eaf8daa 100644 --- a/src/json.js +++ b/src/json.js @@ -109,7 +109,10 @@ export async function validateJSONArrayFile(fullPath, label, repairer = repairJS const repaired = await repairer(fullPath, label, original); const normalized = repaired.endsWith('\n') ? repaired : `${repaired}\n`; // 先驗證修復結果是否為合法 JSON；無效就在寫檔前丟出，避免用毀損內容覆寫原檔。 - JSON.parse(normalized); + const parsed = JSON.parse(normalized); + if (!Array.isArray(parsed)) { + throw new Error(`${label} 修復後內容不是 JSON 陣列`); + } fs.writeFileSync(fullPath, normalized, 'utf8'); ok(`${label} 已由 AI 修正並通過再次驗證`); return { exists: true, valid: true, repaired: true }; diff --git a/src/resolve.js b/src/resolve.js index 3e452d9..40e1118 100644 --- a/src/resolve.js +++ b/src/resolve.js @@ -78,16 +78,18 @@ export function groupConversations(comments) { const lineNum = Number(c?.position) || Number(c?.new_position) || Number(c?.original_position) || 0; const key = `${filePath}|${lineNum}`; if (!groups.has(key)) { - groups.set(key, { key, path: filePath, line: lineNum, commentIds: [], bodies: [], resolved: false, botFinding: null }); + groups.set(key, { key, path: filePath, line: lineNum, commentIds: [], bodies: [], resolved: false, botFinding: null, botFindings: [] }); } const g = groups.get(key); if (c?.id != null) g.commentIds.push(c.id); const body = typeof c?.body === 'string' ? c.body : ''; if (body) g.bodies.push(body); if (c?.resolver) g.resolved = true; - if (!g.botFinding) { - const finding = parseBotReviewComment(body); - if (finding) g.botFinding = { ...finding, location: lineNum ? `${filePath}:${lineNum}` : filePath }; + const finding = parseBotReviewComment(body); + if (finding) { + const normalizedFinding = { ...finding, location: lineNum ? `${filePath}:${lineNum}` : filePath }; + g.botFindings.push(normalizedFinding); + if (!g.botFinding) g.botFinding = normalizedFinding; } } return [...groups.values()].map(g => ({ ...g, thread: g.bodies.join('\n---\n') })); @@ -146,8 +148,12 @@ export async function judgeConversations(items, chatFn = chatJSON) { * @returns {void} */ function pushCarried(target, conversation) { - if (!conversation.botFinding) return; - target.push({ ...conversation.botFinding, is_new: false }); + const findings = conversation.botFindings?.length + ? conversation.botFindings + : (conversation.botFinding ? [conversation.botFinding] : []); + for (const finding of findings) { + target.push({ ...finding, is_new: false }); + } } /** @@ -273,10 +279,16 @@ export async function reconcileConversations(deps = {}) { const verdict = verdictByIdx.get(i) || 'open'; if (verdict === 'resolved') { resolvedCount += 1; - if (c.botFinding) resolvedFindings.push({ ...c.botFinding, is_new: false }); + const findings = c.botFindings?.length ? c.botFindings : (c.botFinding ? [c.botFinding] : []); + for (const finding of findings) { + resolvedFindings.push({ ...finding, is_new: false }); + } } else if (verdict === 'false_positive') { falsePositiveCount += 1; - if (c.botFinding) excludedFindings.push(toExclusion(c.botFinding)); + const findings = c.botFindings?.length ? c.botFindings : (c.botFinding ? [c.botFinding] : []); + for (const finding of findings) { + excludedFindings.push(toExclusion(finding)); + } } else { openCount += 1; pushCarried(carriedFindings, c); diff --git a/src/test/comments.test.js b/src/test/comments.test.js index bd438f8..b7f7656 100644 --- a/src/test/comments.test.js +++ b/src/test/comments.test.js @@ -87,6 +87,10 @@ describe('parseLocation', () => { assert.equal(parseLocation('app/preflight.test.js'), null); }); + it('returns null when the parsed line number is zero', () => { + assert.equal(parseLocation('app/preflight.js:0'), null); + }); + it('returns null when multiple files are listed', () => { assert.equal(parseLocation('Dockerfile, app/git.js, app/gitea.js'), null); }); diff --git a/src/test/findings.test.js b/src/test/findings.test.js index 8ba4849..71edada 100644 --- a/src/test/findings.test.js +++ b/src/test/findings.test.js @@ -144,6 +144,21 @@ describe('findings exclusions', () => { assert.equal(filtered[0].location, 'README.md:12'); }); + it('applies pure text exclusions using the original finding text', () => { + const findings = [ + { location: 'src/app.ts:10', role: 'Maya', suggestion: 'Update tests' }, + { location: 'src/app.ts:11', role: 'Maya', suggestion: 'Keep this' }, + ]; + const exclusions = [ + { original_finding: 'update tests' }, + ]; + + const filtered = applyExclusions(findings, exclusions); + + assert.equal(filtered.length, 1); + assert.equal(filtered[0].suggestion, 'Keep this'); + }); + it('dedupes repeated exclusions when loading exclusions', () => { const fullPath = path.join(workspace, EXCLUSIONS_PATH); fs.mkdirSync(path.dirname(fullPath), { recursive: true }); diff --git a/src/test/gitea.test.js b/src/test/gitea.test.js index b6a4748..dbba29d 100644 --- a/src/test/gitea.test.js +++ b/src/test/gitea.test.js @@ -197,17 +197,25 @@ describe('gitea', () => { assert.equal(await getFileContentAtRef('missing.js', 'ref'), ''); }); - it('shouldSkipBotCommit returns true when either sha or branch head is bot commit', async () => { + it('shouldSkipBotCommit returns true when either sha or branch head is a bot success commit, but not failure', async () => { mock.method(axios, 'get', async (url) => { if (url.includes('/git/commits/sha-bot')) { return { data: { message: 'chore: update ai-review findings [ai-review-bot][failure]' } }; } + if (url.includes('/git/commits/sha-success')) { + return { data: { message: 'chore: update ai-review findings [ai-review-bot][success]' } }; + } if (url.includes('/branches/feat%2Ftest')) { return { data: { commit: { id: 'sha-bot' } } }; } + if (url.includes('/branches/feat%2Fsuccess')) { + return { data: { commit: { id: 'sha-success' } } }; + } return { data: { message: 'regular commit' } }; }); - await assert.equal(await shouldSkipBotCommit({ sha: 'sha-bot', branch: 'feat/test' }), true); + await assert.equal(await shouldSkipBotCommit({ sha: 'sha-bot', branch: 'feat/test' }), false); + await assert.equal(await shouldSkipBotCommit({ sha: 'sha-success', branch: 'feat/success' }), true); + await assert.equal(await shouldSkipBotCommit({ sha: 'sha-success', branch: 'feat/test' }), true); assert.equal(getBotReviewOutcome('chore: update ai-review findings [ai-review-bot][failure]'), 'failure'); assert.equal(getBotReviewOutcome('chore: update ai-review findings [ai-review-bot][success]'), 'success'); assert.equal(getBotReviewOutcome('chore: update ai-review findings [ai-review-bot]'), 'unknown'); diff --git a/src/test/json.test.js b/src/test/json.test.js index 0b8b55d..22672c8 100644 --- a/src/test/json.test.js +++ b/src/test/json.test.js @@ -77,6 +77,18 @@ describe('json helpers', () => { assert.equal(fs.readFileSync(fullPath, 'utf8'), '[]\n'); }); + it('rejects repaired JSON that is not an array', async () => { + const fullPath = path.join(workspace, '.gitea/ai-review/findings.json'); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + fs.writeFileSync(fullPath, '{broken', 'utf8'); + + await assert.rejects( + () => validateJSONArrayFile(fullPath, '.gitea/ai-review/findings.json', async () => '{"ok":true}'), + /不是 JSON 陣列/, + ); + assert.equal(fs.readFileSync(fullPath, 'utf8'), '{broken'); + }); + it('reads a valid JSON file whose size equals the maximum limit', async () => { const fullPath = path.join(workspace, '.gitea/ai-review/findings.json'); fs.mkdirSync(path.dirname(fullPath), { recursive: true }); diff --git a/src/test/resolve.test.js b/src/test/resolve.test.js index ed07dfa..364fa9f 100644 --- a/src/test/resolve.test.js +++ b/src/test/resolve.test.js @@ -83,6 +83,17 @@ describe('groupConversations', () => { const convos = groupConversations([{ id: 1, path: 'a.js', original_position: 7, body: 'x' }]); assert.equal(convos[0].line, 7); }); + + it('keeps multiple bot findings on the same path and line', () => { + const comments = [ + { id: 1, path: 'a.js', position: 10, body: reviewBody('🔴 嚴重', 'Assassin', 'p1', 's1') }, + { id: 2, path: 'a.js', position: 10, body: reviewBody('🟡 警告', 'Mage', 'p2', 's2') }, + ]; + const convos = groupConversations(comments); + assert.equal(convos.length, 1); + assert.equal(convos[0].botFindings.length, 2); + assert.deepEqual(convos[0].botFindings.map(f => f.role), ['Assassin', 'Mage']); + }); }); describe('codeWindow', () => { @@ -207,6 +218,19 @@ describe('reconcileConversations', () => { assert.equal(result.closedCount, 3); }); + it('preserves multiple bot findings when a grouped conversation is still open', async () => { + const deps = baseDeps(); + deps.listComments = async () => [ + { id: 10, path: 'a.js', position: 5, body: reviewBody('🔴 嚴重', 'Assassin', 'p', 's10') }, + { id: 11, path: 'a.js', position: 5, body: reviewBody('🟡 警告', 'Mage', 'p', 's11') }, + ]; + deps.judge = async (items) => items.map(it => ({ idx: it.idx, verdict: 'open' })); + + const result = await reconcileConversations(deps); + + assert.deepEqual(result.carriedFindings.map(f => f.suggestion).sort(), ['s10', 's11']); + }); + it('counts only successful closes when some resolve calls fail', async () => { const deps = baseDeps(); // a.js(id1) 關閉成功、b.js(id2) 關閉失敗（c.js 已 resolved 略過）