207 lines
8.5 KiB
JavaScript
207 lines
8.5 KiB
JavaScript
// tests/lib/unique-finding-ids.test.mjs
|
|
// [DUP] Every finding id in a review is unique. Two MAJORs that share one id
|
|
// would let one bound `accepted` answer clear both, so the verdict gate
|
|
// (`decideVerdict`), the answers check (`decideAnswers`) and the answer CLI
|
|
// (`recordAnswer`) refuse duplicates: the gate and the check as "not
|
|
// measured" (exit 2), the CLI by appending nothing.
|
|
// `keep:` cases hold at the base already and stay untagged.
|
|
// Modules load through `import * as m`, so a missing export is a failed
|
|
// assertion at the red base, never a crash.
|
|
|
|
import { after, test } from 'node:test';
|
|
import { strict as assert } from 'node:assert';
|
|
import { spawnSync } from 'node:child_process';
|
|
import { appendFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
|
import { tmpdir } from 'node:os';
|
|
import { dirname, join } from 'node:path';
|
|
import { fileURLToPath } from 'node:url';
|
|
import * as gateMod from '../../lib/review/verdict-gate.mjs';
|
|
import * as answersMod from '../../lib/review/review-answers.mjs';
|
|
import { LOG_RELPATH } from '../../lib/util/approval-log.mjs';
|
|
import { childEnv } from '../helpers/child-env.mjs';
|
|
|
|
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
|
|
const GATE = join(ROOT, 'lib', 'review', 'verdict-gate.mjs');
|
|
const ANSWERS = join(ROOT, 'lib', 'review', 'review-answers.mjs');
|
|
const TRAILER = 'Co-Authored-By: Claude Test <noreply@example.invalid>';
|
|
const ANS_REL = '.claude/projects/2026-11-03-d/review.md';
|
|
const fid = (n) => String(n).padStart(40, 'b');
|
|
const parse = (r) => { try { return JSON.parse(r.stdout); } catch { return {}; } };
|
|
|
|
/**
|
|
* A review of `sha` (shape copied from tests/lib/review-answers.test.mjs
|
|
* `ansReview`). `findings` are [{severity, id?}]; an absent id is fid(i + 1).
|
|
* `fmIds` is the frontmatter `findings:` list (absent = the JSON ids, each
|
|
* once, in order).
|
|
*/
|
|
function dupReview({ sha = 'a'.repeat(40), verdict = 'WARN', findings = [{ severity: 'MAJOR' }], fmIds } = {}) {
|
|
const list = findings.map((f, i) => ({
|
|
id: f.id ?? fid(i + 1), severity: f.severity, rule_key: 'MISSING_TEST',
|
|
file: 'a.mjs', line: i + 1, title: `t${i + 1}`, detail: `d${i + 1}`, recommended_action: 'Add a test.',
|
|
}));
|
|
const counts = { BLOCKER: 0, MAJOR: 0, MINOR: 0, SUGGESTION: 0 };
|
|
for (const f of list) counts[f.severity] += 1;
|
|
const json = JSON.stringify({ verdict, counts, allow_blocked_by: [], findings: list }, null, 2);
|
|
const ids = fmIds ?? [...new Set(list.map((f) => f.id))];
|
|
return `---
|
|
type: trekreview
|
|
review_version: "1.2"
|
|
task: "t"
|
|
slug: s
|
|
project_dir: .claude/projects/2026-11-03-d/
|
|
brief_path: .claude/projects/2026-11-03-d/brief.md
|
|
scope_sha_end: ${sha}
|
|
reviewed_files_count: 1
|
|
verdict: ${verdict}
|
|
findings:
|
|
${ids.map((id) => ` - ${id}`).join('\n')}
|
|
---
|
|
|
|
# Review: t
|
|
|
|
## Executive Summary
|
|
|
|
x
|
|
|
|
## Coverage
|
|
|
|
| File | Treatment | Reason |
|
|
|------|-----------|--------|
|
|
| a.mjs | deep-review | default |
|
|
|
|
## Remediation Summary
|
|
|
|
x
|
|
|
|
## Remediation Steps
|
|
|
|
None.
|
|
|
|
## Declined to Judge
|
|
|
|
None.
|
|
|
|
\`\`\`json
|
|
${json}
|
|
\`\`\`
|
|
`;
|
|
}
|
|
|
|
/** git in `cwd` (copied from tests/lib/review-answers.test.mjs `g`). */
|
|
function g(cwd, ...args) {
|
|
const r = spawnSync('git', args, { cwd, encoding: 'utf8', env: childEnv() });
|
|
if (r.status !== 0) throw new Error(`git ${args.join(' ')}: ${r.stderr}`);
|
|
return r.stdout.trim();
|
|
}
|
|
|
|
function putIn(dir, rel, text) {
|
|
mkdirSync(dirname(join(dir, rel)), { recursive: true });
|
|
writeFileSync(join(dir, rel), text);
|
|
}
|
|
|
|
const made = [];
|
|
after(() => { for (const b of made) rmSync(b, { recursive: true, force: true }); });
|
|
|
|
/**
|
|
* A throwaway repo with one reviewed commit and a review of it at ANS_REL
|
|
* (pattern of tests/lib/review-answers.test.mjs `newRepo` + `ansFixture`).
|
|
*/
|
|
function fixture(reviewOpts = {}) {
|
|
const base = mkdtempSync(join(tmpdir(), 'unique-finding-ids-'));
|
|
made.push(base);
|
|
const repo = join(base, 'repo');
|
|
mkdirSync(repo, { recursive: true });
|
|
mkdirSync(join(base, 'hooks-none'));
|
|
g(repo, 'init', '-q', '-b', 'main');
|
|
appendFileSync(join(repo, '.git', 'config'), '[user]\n\tname = Test\n\temail = test@example.invalid\n'
|
|
+ `[commit]\n\tgpgsign = false\n[core]\n\thooksPath = "${join(base, 'hooks-none')}"\n`);
|
|
appendFileSync(join(repo, '.git', 'info', 'exclude'), '.claude/projects/\n');
|
|
putIn(repo, 'a.mjs', 'export const a = 1;\n');
|
|
g(repo, 'add', '-A');
|
|
g(repo, 'commit', '-q', '-m', 'feat: the work');
|
|
const reviewed = g(repo, 'rev-parse', 'HEAD');
|
|
const reviewPath = join(repo, ...ANS_REL.split('/'));
|
|
putIn(repo, ANS_REL, dupReview({ sha: reviewed, ...reviewOpts }));
|
|
const logFile = join(repo, ...LOG_RELPATH.split('/'));
|
|
return {
|
|
repo, reviewPath,
|
|
readLog: () => (existsSync(logFile) ? readFileSync(logFile, 'utf8').split('\n').filter(Boolean) : []),
|
|
read: () => readFileSync(reviewPath, 'utf8'),
|
|
record: ({ finding = fid(1), answer = 'accepted', reason = 'the old behaviour is kept on purpose' } = {}) => spawnSync(
|
|
process.execPath,
|
|
[ANSWERS, 'record', '--review', reviewPath, '--finding', finding, '--answer', answer,
|
|
'--repo', repo, '--round', '1', '--store', 'local', '--trailer', TRAILER],
|
|
{ input: reason, encoding: 'utf8', env: childEnv() },
|
|
),
|
|
gate: () => spawnSync(process.execPath, [GATE, '--json', reviewPath, '--repo', repo], { encoding: 'utf8', env: childEnv() }),
|
|
};
|
|
}
|
|
|
|
const TWO_MAJORS_ONE_ID = [{ severity: 'MAJOR', id: fid(1) }, { severity: 'MAJOR', id: fid(1) }];
|
|
|
|
function assertNotMeasuredDup(reason) {
|
|
assert.match(String(reason), /duplicate finding id/, String(reason));
|
|
assert.doesNotMatch(String(reason), /^review-unanswered:/);
|
|
assert.doesNotMatch(String(reason), /^review-stale/);
|
|
}
|
|
|
|
test('[DUP] two MAJORs that share one id keep the gate at exit 2 although one accepted row is bound', () => {
|
|
const fx = fixture({ findings: TWO_MAJORS_ONE_ID });
|
|
// Succeeds at the base, refused after the fix: its exit is not the point.
|
|
fx.record();
|
|
const r = fx.gate();
|
|
assert.equal(r.status, 2, r.stdout + r.stderr);
|
|
assertNotMeasuredDup(parse(r).reason);
|
|
assert.match(String(parse(r).reason), new RegExp(`duplicate finding id ${fid(1)} in the trailing JSON block`));
|
|
});
|
|
|
|
test('[DUP] the frontmatter list with one id twice exits 2', () => {
|
|
assert.equal(typeof gateMod.decideVerdict, 'function');
|
|
const text = dupReview({ findings: [{ severity: 'MINOR' }], fmIds: [fid(1), fid(1)] });
|
|
const d = gateMod.decideVerdict(text);
|
|
assert.equal(d.exit, 2, JSON.stringify(d));
|
|
assertNotMeasuredDup(d.reason);
|
|
assert.equal(d.reason, `duplicate finding id ${fid(1)} in the frontmatter findings: list`);
|
|
assert.equal(d.verdict, 'WARN');
|
|
});
|
|
|
|
test('[DUP] decideAnswers refuses duplicate ids', () => {
|
|
assert.equal(typeof answersMod.decideAnswers, 'function');
|
|
const d = answersMod.decideAnswers(dupReview({ findings: TWO_MAJORS_ONE_ID }), {});
|
|
assert.deepEqual(d, {
|
|
ok: false, unanswered: [], fixedPending: [], reason: `review-not-measured: duplicate finding id ${fid(1)}`,
|
|
});
|
|
});
|
|
|
|
test('[DUP] record refuses an answer on a review with duplicate ids and appends nothing', () => {
|
|
const fx = fixture({ findings: TWO_MAJORS_ONE_ID });
|
|
const before = fx.read();
|
|
const r = fx.record();
|
|
assert.equal(r.status, 2, r.stdout + r.stderr);
|
|
assert.equal(parse(r).recorded, false, r.stdout);
|
|
assert.match(String(parse(r).reason), new RegExp(`holds a duplicate finding id ${fid(1)}; nothing appended`), r.stdout);
|
|
assert.deepEqual(fx.readLog(), []);
|
|
assert.equal(fx.read(), before);
|
|
});
|
|
|
|
test('keep: a BLOCK review with duplicate ids still exits 1', () => {
|
|
assert.equal(typeof gateMod.decideVerdict, 'function');
|
|
const d = gateMod.decideVerdict(dupReview({ verdict: 'BLOCK', findings: TWO_MAJORS_ONE_ID, fmIds: [fid(1), fid(1)] }));
|
|
assert.equal(d.exit, 1, JSON.stringify(d));
|
|
const fx = fixture({ verdict: 'BLOCK', findings: TWO_MAJORS_ONE_ID });
|
|
const r = fx.gate();
|
|
assert.equal(r.status, 1, r.stdout + r.stderr);
|
|
});
|
|
|
|
test('keep: a review with unique ids decides as before', () => {
|
|
assert.equal(typeof gateMod.decideVerdict, 'function');
|
|
assert.equal(gateMod.decideVerdict(dupReview({ findings: [{ severity: 'MINOR' }, { severity: 'MINOR' }] })).exit, 0);
|
|
const fx = fixture({ findings: [{ severity: 'MAJOR' }, { severity: 'MINOR' }] });
|
|
const open = fx.gate();
|
|
assert.equal(open.status, 2, open.stdout + open.stderr);
|
|
assert.match(String(parse(open).reason), /^review-unanswered: 1 MAJOR without an answer/);
|
|
const rec = fx.record();
|
|
assert.equal(rec.status, 0, rec.stdout + rec.stderr);
|
|
const r = fx.gate();
|
|
assert.equal(r.status, 0, r.stdout + r.stderr);
|
|
});
|