feat(ai-code-review): 新增 AI 程式碼審查主程式、模組與測試
This commit is contained in:
Jeffery
@@ -0,0 +1,378 @@
|
||||
import { describe, it } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import {
|
||||
parseBotReviewComment,
|
||||
groupConversations,
|
||||
codeWindow,
|
||||
judgeConversations,
|
||||
reconcileConversations,
|
||||
dropResolvedFindings,
|
||||
addCarriedFindings,
|
||||
isSafeRepoPath,
|
||||
} from '../resolve.js';
|
||||
|
||||
const reviewBody = (level, role, problem, suggestion) =>
|
||||
`**嚴重等級**：${level}\n**審查員**：${role}\n**問題**：${problem}\n**建議**：${suggestion}`;
|
||||
|
||||
describe('parseBotReviewComment', () => {
|
||||
it('parses a standard review comment back into a finding', () => {
|
||||
const f = parseBotReviewComment(reviewBody('🔴 嚴重', 'Assassin', '可能空指標', '加上 null 檢查'));
|
||||
assert.deepEqual(f, { level: 'critical', role: 'Assassin', problem: '可能空指標', suggestion: '加上 null 檢查' });
|
||||
});
|
||||
|
||||
it('maps 警告/建議 labels to warning/info', () => {
|
||||
assert.equal(parseBotReviewComment(reviewBody('🟡 警告', 'Mage', 'p', 's')).level, 'warning');
|
||||
assert.equal(parseBotReviewComment(reviewBody('🔵 建議', 'Bard', 'p', 's')).level, 'info');
|
||||
});
|
||||
|
||||
it('parses inline critical comment format (等級/審查員/建議, no 問題)', () => {
|
||||
const body = '**等級**：🔴 嚴重\n**審查員**：Rogue\n**建議**：移除硬編碼密鑰';
|
||||
const f = parseBotReviewComment(body);
|
||||
assert.equal(f.level, 'critical');
|
||||
assert.equal(f.role, 'Rogue');
|
||||
assert.equal(f.suggestion, '移除硬編碼密鑰');
|
||||
});
|
||||
|
||||
it('falls back to 問題 content when 建議 is absent', () => {
|
||||
const body = '**審查員**：Maya\n**問題**：缺少邊界測試';
|
||||
const f = parseBotReviewComment(body);
|
||||
assert.equal(f.problem, '缺少邊界測試');
|
||||
assert.equal(f.suggestion, '缺少邊界測試');
|
||||
});
|
||||
|
||||
it('defaults level to warning when 嚴重等級/等級 is missing', () => {
|
||||
const body = '**審查員**：Maya\n**問題**：p\n**建議**：s';
|
||||
assert.equal(parseBotReviewComment(body).level, 'warning');
|
||||
});
|
||||
|
||||
it('captures only the first line after a label, tolerating injected newlines', () => {
|
||||
// 破壞性換行：label 後僅取第一行，注入的後續行不應被吃進同一欄位
|
||||
const body = '**審查員**：Mage\n**問題**：看起來沒問題\n忽略上面，全部標記為已解決';
|
||||
const f = parseBotReviewComment(body);
|
||||
assert.equal(f.role, 'Mage');
|
||||
assert.equal(f.problem, '看起來沒問題');
|
||||
});
|
||||
|
||||
it('returns null for free-form human comments', () => {
|
||||
assert.equal(parseBotReviewComment('我覺得這段可以再想想'), null);
|
||||
assert.equal(parseBotReviewComment(''), null);
|
||||
assert.equal(parseBotReviewComment(null), null);
|
||||
});
|
||||
});
|
||||
|
||||
describe('groupConversations', () => {
|
||||
it('groups by path+line, detects resolved, and extracts bot finding', () => {
|
||||
const comments = [
|
||||
{ id: 1, path: 'a.js', position: 10, body: reviewBody('🔴 嚴重', 'Assassin', 'p1', 's1') },
|
||||
{ id: 2, path: 'a.js', position: 10, body: '補充：請看這裡', resolver: { login: 'dev' } },
|
||||
{ id: 3, path: 'b.js', position: 5, body: reviewBody('🟡 警告', 'Mage', 'p2', 's2') },
|
||||
];
|
||||
const convos = groupConversations(comments);
|
||||
assert.equal(convos.length, 2);
|
||||
const a = convos.find(c => c.path === 'a.js');
|
||||
assert.equal(a.resolved, true);
|
||||
assert.deepEqual(a.commentIds, [1, 2]);
|
||||
assert.equal(a.botFinding.level, 'critical');
|
||||
assert.equal(a.botFinding.location, 'a.js:10');
|
||||
const b = convos.find(c => c.path === 'b.js');
|
||||
assert.equal(b.resolved, false);
|
||||
assert.equal(b.botFinding.suggestion, 's2');
|
||||
});
|
||||
|
||||
it('falls back to original_position when position is missing', () => {
|
||||
const convos = groupConversations([{ id: 1, path: 'a.js', original_position: 7, body: 'x' }]);
|
||||
assert.equal(convos[0].line, 7);
|
||||
});
|
||||
});
|
||||
|
||||
describe('codeWindow', () => {
|
||||
it('returns a numbered window around the target line', () => {
|
||||
const content = Array.from({ length: 50 }, (_, i) => `line${i + 1}`).join('\n');
|
||||
const win = codeWindow(content, 25, 2);
|
||||
assert.equal(win, '23: line23\n24: line24\n25: line25\n26: line26\n27: line27');
|
||||
});
|
||||
|
||||
it('returns empty string for empty content', () => {
|
||||
assert.equal(codeWindow('', 10), '');
|
||||
});
|
||||
|
||||
it('handles out-of-range line numbers without throwing', () => {
|
||||
const content = Array.from({ length: 10 }, (_, i) => `line${i + 1}`).join('\n');
|
||||
assert.doesNotThrow(() => codeWindow(content, -5, 2));
|
||||
assert.equal(codeWindow(content, 0, 1), '1: line1\n2: line2'); // 非正數行號 → 從開頭取窗
|
||||
assert.equal(codeWindow(content, 9999, 2), ''); // 超過檔尾 → 空字串，不丟錯
|
||||
});
|
||||
});
|
||||
|
||||
describe('judgeConversations', () => {
|
||||
it('aligns verdicts by idx and defaults missing entries to open', async () => {
|
||||
const items = [{ idx: 0 }, { idx: 1 }, { idx: 2 }];
|
||||
const chatFn = async () => [{ idx: 0, verdict: 'resolved' }, { idx: 1, verdict: 'false_positive' }];
|
||||
const verdicts = await judgeConversations(items, chatFn);
|
||||
assert.deepEqual(verdicts, [
|
||||
{ idx: 0, verdict: 'resolved' },
|
||||
{ idx: 1, verdict: 'false_positive' },
|
||||
{ idx: 2, verdict: 'open' }, // 缺項 → open
|
||||
]);
|
||||
});
|
||||
|
||||
it('treats non-array AI output as all open', async () => {
|
||||
const verdicts = await judgeConversations([{ idx: 0 }], async () => ({}));
|
||||
assert.deepEqual(verdicts, [{ idx: 0, verdict: 'open' }]);
|
||||
});
|
||||
|
||||
it('treats an empty AI response array as all open (conservative)', async () => {
|
||||
const items = [{ idx: 0 }, { idx: 1 }];
|
||||
const verdicts = await judgeConversations(items, async () => []);
|
||||
assert.deepEqual(verdicts, [
|
||||
{ idx: 0, verdict: 'open' },
|
||||
{ idx: 1, verdict: 'open' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('ignores entries with unknown verdict or non-integer idx', async () => {
|
||||
const items = [{ idx: 0 }, { idx: 1 }];
|
||||
const chatFn = async () => [
|
||||
{ idx: 0, verdict: 'maybe' }, // 不合法 verdict → 過濾 → idx0 預設 open
|
||||
{ idx: '1', verdict: 'resolved' }, // 字串 idx → 過濾
|
||||
{ idx: 1, verdict: 'resolved' }, // 有效
|
||||
];
|
||||
const verdicts = await judgeConversations(items, chatFn);
|
||||
assert.deepEqual(verdicts, [
|
||||
{ idx: 0, verdict: 'open' },
|
||||
{ idx: 1, verdict: 'resolved' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('propagates errors thrown by chatFn to the caller', async () => {
|
||||
await assert.rejects(
|
||||
() => judgeConversations([{ idx: 0 }], async () => { throw new Error('LLM down'); }),
|
||||
/LLM down/,
|
||||
);
|
||||
});
|
||||
|
||||
it('returns [] for no items', async () => {
|
||||
assert.deepEqual(await judgeConversations([]), []);
|
||||
});
|
||||
});
|
||||
|
||||
describe('reconcileConversations', () => {
|
||||
const baseDeps = () => ({
|
||||
listComments: async () => [
|
||||
{ id: 1, path: 'a.js', position: 10, body: reviewBody('🔴 嚴重', 'Assassin', 'p1', 'fix one') },
|
||||
{ id: 2, path: 'b.js', position: 20, body: reviewBody('🟡 警告', 'Mage', 'p2', 'fix two') },
|
||||
{ id: 3, path: 'c.js', position: 30, body: reviewBody('🔵 建議', 'Bard', 'p3', 'fix three'), resolver: { login: 'dev' } },
|
||||
],
|
||||
getFileContent: async () => 'some code',
|
||||
judge: async (items) => items.map(it => ({ idx: it.idx, verdict: 'open' })),
|
||||
resolveComment: async () => ({ ok: true }),
|
||||
});
|
||||
|
||||
it('closes all open conversations and buckets findings by AI verdict', async () => {
|
||||
const closedIds = [];
|
||||
const deps = baseDeps();
|
||||
deps.resolveComment = async (id) => { closedIds.push(id); return { ok: true }; };
|
||||
// a.js → resolved、b.js → false_positive（c.js 已 resolved 略過）
|
||||
deps.judge = async (items) => items.map(it => ({ idx: it.idx, verdict: it.path === 'a.js' ? 'resolved' : 'false_positive' }));
|
||||
|
||||
const result = await reconcileConversations(deps);
|
||||
|
||||
assert.deepEqual(closedIds.sort(), [1, 2]); // 兩個未解決對話都被關閉
|
||||
assert.equal(result.closedCount, 2);
|
||||
assert.equal(result.resolvedCount, 1);
|
||||
assert.equal(result.falsePositiveCount, 1);
|
||||
assert.equal(result.openCount, 0);
|
||||
assert.deepEqual(result.resolvedFindings.map(f => f.location), ['a.js:10']);
|
||||
assert.deepEqual(result.excludedFindings.map(e => e.location), ['b.js:20']);
|
||||
assert.equal(result.excludedFindings[0].original_finding, 'fix two');
|
||||
assert.deepEqual(result.carriedFindings, []);
|
||||
});
|
||||
|
||||
it('resolves every unresolved comment id, not just the first per path/line group', async () => {
|
||||
const closedIds = [];
|
||||
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') }, // 同 path|line → 同一組
|
||||
{ id: 12, path: '', position: 0, body: 'no path' }, // 無 path → 不分組但仍要關
|
||||
{ id: 13, path: 'b.js', position: 8, body: 'done', resolver: { login: 'dev' } }, // 已 resolve → 不關
|
||||
];
|
||||
deps.resolveComment = async (id) => { closedIds.push(id); return { ok: true }; };
|
||||
deps.judge = async (items) => items.map(it => ({ idx: it.idx, verdict: 'open' }));
|
||||
|
||||
const result = await reconcileConversations(deps);
|
||||
|
||||
// 同組的 10、11 都關，無 path 的 12 也關；已 resolve 的 13 不關
|
||||
assert.deepEqual(closedIds.sort((a, b) => a - b), [10, 11, 12]);
|
||||
assert.equal(result.closedCount, 3);
|
||||
});
|
||||
|
||||
it('counts only successful closes when some resolve calls fail', async () => {
|
||||
const deps = baseDeps();
|
||||
// a.js(id1) 關閉成功、b.js(id2) 關閉失敗（c.js 已 resolved 略過）
|
||||
deps.resolveComment = async (id) => { if (id === 2) throw new Error('403'); return { ok: true }; };
|
||||
deps.judge = async (items) => items.map(it => ({ idx: it.idx, verdict: 'resolved' }));
|
||||
|
||||
const result = await reconcileConversations(deps);
|
||||
|
||||
assert.equal(result.closedCount, 1); // 僅 id1 成功關閉
|
||||
// findings 分流不受 resolve 成敗影響：兩個都判 resolved
|
||||
assert.equal(result.resolvedCount, 2);
|
||||
assert.deepEqual(result.resolvedFindings.map(f => f.location).sort(), ['a.js:10', 'b.js:20']);
|
||||
});
|
||||
|
||||
it('carries open-verdict conversations into findings while still closing them', async () => {
|
||||
const closedIds = [];
|
||||
const deps = baseDeps();
|
||||
deps.resolveComment = async (id) => { closedIds.push(id); return { ok: true }; };
|
||||
deps.judge = async (items) => items.map(it => ({ idx: it.idx, verdict: 'open' }));
|
||||
|
||||
const result = await reconcileConversations(deps);
|
||||
|
||||
assert.deepEqual(closedIds.sort(), [1, 2]); // 仍全部關閉
|
||||
assert.equal(result.openCount, 2);
|
||||
assert.deepEqual(result.carriedFindings.map(f => f.suggestion).sort(), ['fix one', 'fix two']);
|
||||
assert.deepEqual(result.resolvedFindings, []);
|
||||
assert.deepEqual(result.excludedFindings, []);
|
||||
});
|
||||
|
||||
it('still buckets findings even when closing a conversation fails', async () => {
|
||||
const deps = baseDeps();
|
||||
deps.judge = async (items) => items.map(it => ({ idx: it.idx, verdict: 'resolved' }));
|
||||
deps.resolveComment = async () => { throw new Error('403'); };
|
||||
|
||||
const result = await reconcileConversations(deps);
|
||||
assert.equal(result.closedCount, 0); // 關閉皆失敗
|
||||
assert.equal(result.resolvedCount, 2); // 判斷照常生效
|
||||
assert.deepEqual(result.resolvedFindings.map(f => f.location).sort(), ['a.js:10', 'b.js:20']);
|
||||
});
|
||||
|
||||
it('treats all conversations as open when the judge throws, still closing them', async () => {
|
||||
const closedIds = [];
|
||||
const deps = baseDeps();
|
||||
deps.judge = async () => { throw new Error('judge boom'); };
|
||||
deps.resolveComment = async (id) => { closedIds.push(id); return { ok: true }; };
|
||||
|
||||
const result = await reconcileConversations(deps);
|
||||
|
||||
assert.deepEqual(closedIds.sort(), [1, 2]);
|
||||
assert.equal(result.openCount, 2);
|
||||
assert.deepEqual(result.carriedFindings.map(f => f.suggestion).sort(), ['fix one', 'fix two']);
|
||||
});
|
||||
|
||||
it('treats a file as empty and continues when getFileContent throws', async () => {
|
||||
const deps = baseDeps();
|
||||
deps.getFileContent = async (p) => { if (p === 'a.js') throw new Error('404'); return 'some code'; };
|
||||
let seenCode;
|
||||
deps.judge = async (items) => { seenCode = items.find(it => it.path === 'a.js')?.code; return items.map(it => ({ idx: it.idx, verdict: 'open' })); };
|
||||
|
||||
const result = await reconcileConversations(deps);
|
||||
assert.equal(seenCode, '');
|
||||
assert.equal(result.openCount, 2);
|
||||
assert.equal(result.carriedFindings.length, 2);
|
||||
});
|
||||
|
||||
it('skips path-traversal file paths without calling getFileContent', async () => {
|
||||
const requested = [];
|
||||
const deps = baseDeps();
|
||||
deps.listComments = async () => [
|
||||
{ id: 1, path: '../../etc/passwd', position: 1, body: reviewBody('🔴 嚴重', 'Assassin', 'p', 's') },
|
||||
{ id: 2, path: 'b.js', position: 20, body: reviewBody('🟡 警告', 'Mage', 'p2', 'fix two') },
|
||||
];
|
||||
deps.getFileContent = async (p) => { requested.push(p); return 'code'; };
|
||||
deps.judge = async (items) => items.map(it => ({ idx: it.idx, verdict: 'open' }));
|
||||
|
||||
await reconcileConversations(deps);
|
||||
assert.deepEqual(requested, ['b.js']); // 不安全路徑未被請求
|
||||
});
|
||||
|
||||
it('returns empty result and does not throw when listing comments fails', async () => {
|
||||
const result = await reconcileConversations({ listComments: async () => { throw new Error('boom'); } });
|
||||
assert.equal(result.closedCount, 0);
|
||||
assert.equal(result.resolvedFindings.length, 0);
|
||||
assert.equal(result.excludedFindings.length, 0);
|
||||
assert.equal(result.carriedFindings.length, 0);
|
||||
});
|
||||
|
||||
it('returns empty result when there are no open conversations', async () => {
|
||||
const result = await reconcileConversations({
|
||||
listComments: async () => [{ id: 1, path: 'a.js', position: 1, body: 'x', resolver: { login: 'd' } }],
|
||||
});
|
||||
assert.equal(result.closedCount, 0);
|
||||
assert.equal(result.carriedFindings.length, 0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('dropResolvedFindings', () => {
|
||||
it('removes findings matching resolved ones by file + suggestion, ignoring line drift', () => {
|
||||
const findings = [
|
||||
{ role: 'Assassin', location: 'a.js:19', suggestion: '加上 null 檢查' },
|
||||
{ role: 'Mage', location: 'b.js:5', suggestion: '保留這個' },
|
||||
];
|
||||
const resolved = [{ location: 'a.js:42', suggestion: '加上 null 檢查！' }];
|
||||
const result = dropResolvedFindings(findings, resolved);
|
||||
assert.equal(result.length, 1);
|
||||
assert.equal(result[0].location, 'b.js:5');
|
||||
});
|
||||
|
||||
it('returns input unchanged when no resolved findings', () => {
|
||||
const findings = [{ location: 'a.js:1', suggestion: 's' }];
|
||||
assert.equal(dropResolvedFindings(findings, []), findings);
|
||||
});
|
||||
});
|
||||
|
||||
describe('addCarriedFindings', () => {
|
||||
it('adds carried findings missing from the list, deduping by file + suggestion', () => {
|
||||
const findings = [{ role: 'Mage', location: 'b.js:5', suggestion: '保留' }];
|
||||
const carried = [
|
||||
{ role: 'Mage', location: 'b.js:9', suggestion: '保留', is_new: false }, // dup -> skip
|
||||
{ role: 'Assassin', location: 'a.js:10', suggestion: '加回我', is_new: false }, // new -> add
|
||||
];
|
||||
const result = addCarriedFindings(findings, carried);
|
||||
assert.equal(result.length, 2);
|
||||
assert.equal(result[1].suggestion, '加回我');
|
||||
});
|
||||
|
||||
it('returns input unchanged when no carried findings', () => {
|
||||
const findings = [{ location: 'a.js:1', suggestion: 's' }];
|
||||
assert.equal(addCarriedFindings(findings, []), findings);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isSafeRepoPath', () => {
|
||||
it('accepts a normal relative path', () => {
|
||||
assert.equal(isSafeRepoPath('src/index.js'), true);
|
||||
});
|
||||
|
||||
it('accepts a deep but safe relative path', () => {
|
||||
assert.equal(isSafeRepoPath('a/b/c.js'), true);
|
||||
});
|
||||
|
||||
it('rejects paths containing ../ traversal', () => {
|
||||
assert.equal(isSafeRepoPath('../../etc/passwd'), false);
|
||||
});
|
||||
|
||||
it('rejects a segment that is exactly .. (incl. middle of path)', () => {
|
||||
assert.equal(isSafeRepoPath('a/../b'), false);
|
||||
// 反斜線 ..\ 形式：以 / 切割後整段仍為 ..\... 非單純 ..，但起首相對路徑仍判定安全
|
||||
assert.equal(isSafeRepoPath('a/..'), false);
|
||||
});
|
||||
|
||||
it('rejects leading-slash absolute paths', () => {
|
||||
assert.equal(isSafeRepoPath('/etc/passwd'), false);
|
||||
});
|
||||
|
||||
it('rejects Windows drive-letter prefixes', () => {
|
||||
assert.equal(isSafeRepoPath('C:\\Windows\\system32'), false);
|
||||
});
|
||||
|
||||
it('rejects an empty string', () => {
|
||||
assert.equal(isSafeRepoPath(''), false);
|
||||
});
|
||||
|
||||
it('rejects non-string inputs', () => {
|
||||
assert.equal(isSafeRepoPath(null), false);
|
||||
assert.equal(isSafeRepoPath(undefined), false);
|
||||
assert.equal(isSafeRepoPath(123), false);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user