An adversarial read of d88245c reproduced 15 problems; all fixed here, each
with a test that fails on the old code.
- verdict-gate: a greener verdict must be backed by the LAST fence being a
`json` block of the right shape, with no BLOCKER in the JSON (any case),
in `## Findings (BLOCKER)`, or a differing finding-id list; two verdict
lines and more than one file argument are exit 2. ({}, null, lowercase
"blocker", a trailing ```jsonc and a later json fence all gave exit 0.)
- engine-findings: a transcribed "narrower" key can no longer lower an
engine tier; absolute paths under --repo-root match the scope; "12" lines
parse; an empty scope is an error; out-of-scope engine findings are
unverified (`engine:out-of-scope`) and forbid ALLOW.
- recurrence: the catch-all engine keys and keys normalised from unknown
ones are never proposed; the same unfixed finding in two iterations is not
a recurrence; proposals are validated before they are offered; \r/U+2028
and damaged markers are refused; writes go through a symlinked CLAUDE.md.
Prose points --repo at the session directory, not the git toplevel.
- Workflow substrate: engines are counted from what arrived, and the
verdict's floor is recomputed from the coordinator's findings; now tested
by running the script with stubbed Workflow globals. Two literal NUL bytes
written as \u0000 (git saw the file as binary).
- Trimming keeps Pass 1's "Also flagged by" provenance; `## Declined to
Judge` and remediation steps are required from review_version 1.1 only
(N-1); the zero-diff review now carries the JSON block the gate needs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
159 lines
7.1 KiB
JavaScript
159 lines
7.1 KiB
JavaScript
// lib/review/engine-findings.mjs
|
|
// Claude Code's review as the engine (ordre 5, P11 — wrap, not a new engine).
|
|
//
|
|
// /trekreview Phase 4.7 starts Claude Code's own `/code-review` and
|
|
// `/security-review` through the Skill tool (never with `--fix`, never anything
|
|
// that edits code). Their findings come back as text: per finding a file
|
|
// location, a one-sentence summary, a failure scenario and an optional category
|
|
// slug (code.claude.com/docs/en/tools-reference.md, `ReportFindings`). The
|
|
// orchestrator transcribes them into a small JSON list; THIS module turns that
|
|
// list into a reviewer payload the coordinator already understands, so the
|
|
// engine's findings meet Voyage's brief control in the same dedup + verdict.
|
|
//
|
|
// Input (one engine per call):
|
|
// { "engine": "code-review" | "security-review",
|
|
// "findings": [ { "file": "...", "line": 12, "summary": "...",
|
|
// "failure_scenario": "...", "category": "correctness",
|
|
// "rule_key": "<optional narrower catalogue key>" } ] }
|
|
//
|
|
// Output: { reviewer, findings: [...catalogue-conformant...], out_of_scope: [...] }
|
|
// - rule_key: the given one when it is in the catalogue AND at least as
|
|
// severe as the engine's own key, else CODE_REVIEW_DEFECT (MAJOR) /
|
|
// SECURITY_REVIEW_VULNERABILITY (BLOCKER). A "narrower" key can never
|
|
// lower an engine finding's tier.
|
|
// - File paths are made repo-relative (an absolute path under `repoRoot`
|
|
// loses that prefix) before they are compared with the scope.
|
|
// - A finding on a file outside the review scope is NOT dropped silently: it
|
|
// lands in `out_of_scope`, which the coordinator counts as unverified
|
|
// (`engine:out-of-scope`) — it forbids ALLOW and is listed under
|
|
// Declined to Judge (`/security-review` diffs against origin's default
|
|
// branch, which can be wider than the review's SHA range).
|
|
// - An empty scope list is an error, never "everything is out of scope".
|
|
//
|
|
// CLI: node lib/review/engine-findings.mjs [--scope <file-list.txt>] [--repo-root <dir>] <engine-findings.json>
|
|
// exit 0 = payload on stdout; 1 = input invalid (errors on stderr); 2 = could not run.
|
|
|
|
import { readFileSync, existsSync, realpathSync } from 'node:fs';
|
|
import { fileURLToPath } from 'node:url';
|
|
import { computeFindingId } from '../parsers/finding-id.mjs';
|
|
import { RULE_KEYS, getRule, SEVERITY_VALUES } from './rule-catalogue.mjs';
|
|
import { validateFindings } from './findings-schema.mjs';
|
|
|
|
export const ENGINES = Object.freeze({
|
|
'code-review': Object.freeze({
|
|
reviewer: 'claude-code-review',
|
|
rule_key: 'CODE_REVIEW_DEFECT',
|
|
brief_ref: 'NFR — code correctness (Claude Code /code-review)',
|
|
}),
|
|
'security-review': Object.freeze({
|
|
reviewer: 'claude-security-review',
|
|
rule_key: 'SECURITY_REVIEW_VULNERABILITY',
|
|
brief_ref: 'NFR — security (Claude Code /security-review)',
|
|
}),
|
|
});
|
|
|
|
function normPath(p, repoRoot) {
|
|
let out = String(p ?? '').trim().replace(/\\/g, '/');
|
|
if (repoRoot) {
|
|
const root = String(repoRoot).replace(/\\/g, '/').replace(/\/+$/, '') + '/';
|
|
if (out.startsWith(root)) out = out.slice(root.length);
|
|
}
|
|
return out.replace(/^(\.\/)+/, '');
|
|
}
|
|
|
|
function parseLine(v) {
|
|
if (Number.isInteger(v) && v >= 0) return v;
|
|
if (typeof v === 'string' && /^\d+$/.test(v.trim())) return Number(v.trim());
|
|
return 0;
|
|
}
|
|
|
|
const tier = (key) => SEVERITY_VALUES.indexOf(getRule(key).severity);
|
|
|
|
/**
|
|
* Convert one engine's transcribed findings into a coordinator payload.
|
|
* @param {{ engine: string, findings: object[] }} input
|
|
* @param {{ scopeFiles?: string[], repoRoot?: string }} [opts]
|
|
* @returns {{ ok: boolean, errors: string[], payload?: { reviewer: string, findings: object[] }, out_of_scope?: object[] }}
|
|
*/
|
|
export function toReviewerPayload(input, opts = {}) {
|
|
const errors = [];
|
|
const spec = ENGINES[input?.engine];
|
|
if (!spec) return { ok: false, errors: [`engine must be one of ${Object.keys(ENGINES).join('|')}, got ${JSON.stringify(input?.engine)}`] };
|
|
if (!Array.isArray(input.findings)) return { ok: false, errors: ['findings must be an array'] };
|
|
if (opts.scopeFiles && opts.scopeFiles.length === 0) {
|
|
return { ok: false, errors: ['scope list is empty — the review scope is unknown, so no engine finding can be placed'] };
|
|
}
|
|
const scope = opts.scopeFiles ? new Set(opts.scopeFiles.map((f) => normPath(f, opts.repoRoot))) : null;
|
|
|
|
const findings = [];
|
|
const out_of_scope = [];
|
|
input.findings.forEach((f, i) => {
|
|
const file = normPath(f?.file, opts.repoRoot);
|
|
const summary = String(f?.summary ?? '').trim();
|
|
if (!file) errors.push(`findings[${i}].file is empty`);
|
|
if (!summary) errors.push(`findings[${i}].summary is empty`);
|
|
if (!file || !summary) return;
|
|
const line = parseLine(f.line);
|
|
const narrower = typeof f.rule_key === 'string' && RULE_KEYS.has(f.rule_key)
|
|
&& tier(f.rule_key) <= tier(spec.rule_key);
|
|
const rule_key = narrower ? f.rule_key : spec.rule_key;
|
|
const out = {
|
|
id: computeFindingId(file, line, rule_key),
|
|
severity: getRule(rule_key).severity,
|
|
rule_key,
|
|
file,
|
|
line,
|
|
brief_ref: spec.brief_ref,
|
|
title: summary,
|
|
detail: [String(f.failure_scenario ?? '').trim(), f.category ? `(engine category: ${f.category})` : '']
|
|
.filter(Boolean).join(' ') || summary,
|
|
recommended_action: `Resolve: ${summary}`,
|
|
engine: input.engine,
|
|
};
|
|
if (scope && !scope.has(file)) out_of_scope.push(out);
|
|
else findings.push(out);
|
|
});
|
|
if (errors.length > 0) return { ok: false, errors };
|
|
|
|
const payload = { reviewer: spec.reviewer, findings };
|
|
const v = validateFindings(payload);
|
|
if (!v.valid) return { ok: false, errors: v.errors.map((e) => `${e.code}: ${e.message}`) };
|
|
return { ok: true, errors: [], payload, out_of_scope };
|
|
}
|
|
|
|
function isMain() {
|
|
try {
|
|
return realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url));
|
|
} catch {
|
|
return false;
|
|
}
|
|
}
|
|
|
|
if (isMain()) {
|
|
const args = process.argv.slice(2);
|
|
const valueOf = (name) => { const i = args.indexOf(name); return i === -1 ? null : args[i + 1]; };
|
|
const scopePath = valueOf('--scope');
|
|
const repoRoot = valueOf('--repo-root');
|
|
const flagValues = new Set([scopePath, repoRoot].filter(Boolean));
|
|
const filePath = args.find((a) => !a.startsWith('--') && !flagValues.has(a));
|
|
if (!filePath || !existsSync(filePath) || (scopePath && !existsSync(scopePath))) {
|
|
process.stderr.write('Usage: engine-findings.mjs [--scope <file-list.txt>] [--repo-root <dir>] <engine-findings.json>\n');
|
|
process.exit(2);
|
|
}
|
|
let input;
|
|
let scopeFiles;
|
|
try {
|
|
input = JSON.parse(readFileSync(filePath, 'utf-8'));
|
|
if (scopePath) scopeFiles = readFileSync(scopePath, 'utf-8').split('\n').map((l) => l.trim()).filter(Boolean);
|
|
} catch (e) {
|
|
process.stderr.write(`engine-findings: could not read input: ${e.message}\n`);
|
|
process.exit(2);
|
|
}
|
|
const r = toReviewerPayload(input, { scopeFiles, repoRoot: repoRoot ?? undefined });
|
|
if (!r.ok) {
|
|
for (const e of r.errors) process.stderr.write(` ERROR ${e}\n`);
|
|
process.exit(1);
|
|
}
|
|
process.stdout.write(JSON.stringify({ ...r.payload, out_of_scope: r.out_of_scope }, null, 2) + '\n');
|
|
process.exit(0);
|
|
}
|