feat(review): a WARN opens the lock only when every counting MAJOR is answered
This commit is contained in:
parent
ce64851a0d
commit
ce7dbc27a9
SSH key fingerprint:
SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
8 changed files with 1044 additions and 64 deletions
466
lib/review/review-answers.mjs
Normal file
466
lib/review/review-answers.mjs
Normal file
|
|
@ -0,0 +1,466 @@
|
|||
// lib/review/review-answers.mjs
|
||||
// The operator's answers to a review's counting MAJOR findings (review_version
|
||||
// 1.2, `## Answers`). A WARN opens the merge lock and the release check only
|
||||
// when every counting MAJOR is answered; the verdict gate
|
||||
// (lib/review/verdict-gate.mjs `decideOpenFindings`) is the one place that
|
||||
// rule is decided, and it reads this module.
|
||||
//
|
||||
// A counting MAJOR is answered only when all of these hold:
|
||||
// - its `## Answers` row says `accepted` with a reason that is not hollow
|
||||
// (lib/util/approval-log.mjs `isHollowAnswer`);
|
||||
// - it is not a security finding (a `security` catalogue key, or a finding
|
||||
// from the `security-review` engine): those are never accepted;
|
||||
// - the latest K2 record of kind `review` with subject
|
||||
// `<review path relative to the git top level>#<id>` carries
|
||||
// `answerSha(...)` over the review's CURRENT trailing JSON block and
|
||||
// `scope_sha_end`, so an answer goes stale with the review it answers;
|
||||
// - the row's Record is that record's hash.
|
||||
// A `fixed` row never answers: the finding stays open as fixed-pending.
|
||||
// BLOCKER findings are never answered (the gate stops on them before this).
|
||||
//
|
||||
// The answer log is the top-level one: `logPath(answerLogRepo(dir))`, whatever
|
||||
// `--repo` or the review's directory names (lib/review/reviewed-commit.mjs
|
||||
// accepts a commit that only appends `review` records to it, R6).
|
||||
//
|
||||
// A reason is always stored, hashed and compared normalized (`normalizeReason`),
|
||||
// so no reason can break the `## Answers` table or open a fence before the
|
||||
// trailing JSON block. The K2 log keeps its trust limits: it records that the
|
||||
// answer step ran against this content; it does not sign who answered.
|
||||
//
|
||||
// CLI (2 is never green):
|
||||
// node lib/review/review-answers.mjs pending --review <p> [--repo <d>] --json
|
||||
// lists the open counting MAJORs; exit 0 = nothing to ask, 1 = a list,
|
||||
// 2 = could not run
|
||||
// node lib/review/review-answers.mjs record --review <p> --finding <id>
|
||||
// --answer accepted|fixed --repo <d> --round <R> --store <git|local>
|
||||
// --trailer "Co-Authored-By: <name> <<email>>" (reason on stdin)
|
||||
// exit 0 = recorded (a failed commit is reported as `committed: false`
|
||||
// with its reason), 1 = refused (bad trailer, empty or hollow reason, an id
|
||||
// that is not a counting MAJOR, a BLOCKER, `accepted` on a security
|
||||
// finding), 2 = could not run
|
||||
//
|
||||
// The accept shape (an answer bound to the reviewed text, never to a BLOCKER)
|
||||
// follows lib/validators/plan-gate.mjs `acceptOpen`.
|
||||
|
||||
import { createHash } from 'node:crypto';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { accessSync, constants as fsConstants, readFileSync, realpathSync, renameSync, writeFileSync } from 'node:fs';
|
||||
import { basename, dirname, join, resolve } from 'node:path';
|
||||
import { fencedBlocks, isJsonBlock } from './findings-schema.mjs';
|
||||
import { getRule } from './rule-catalogue.mjs';
|
||||
import { reviewedSha } from './reviewed-commit.mjs';
|
||||
import {
|
||||
appendApproval, isHollowAnswer, logPath, normalizeSubject, readLog, verifyLog,
|
||||
} from '../util/approval-log.mjs';
|
||||
import { commitArtifacts, trailerProblem } from '../util/artifact-commit.mjs';
|
||||
import { isMain } from '../util/is-main.mjs';
|
||||
|
||||
export const ANSWER_VALUES = Object.freeze(['accepted', 'fixed']);
|
||||
export const ANSWERS_HEADER = '| Finding | Answer | Reason | Record | Carried from |';
|
||||
const ANSWERS_SEPARATOR = '|---|---|---|---|---|';
|
||||
const MAX_REASON = 300;
|
||||
const HEX40_RE = /^[0-9a-f]{40}$/;
|
||||
// Copied from lib/validators/review-validator.mjs (ANSWER_FINDING_RE, SHA256_RE,
|
||||
// CARRIED_FROM_RE; not exported there).
|
||||
const ANSWER_FINDING_RE = /^([0-9a-f]{40})(?:#\d+)?$/;
|
||||
const SHA256_RE = /^[0-9a-f]{64}$/;
|
||||
const CARRIED_FROM_RE = /^review-round-[0-9a-f]{12}-\d+\.md#[0-9a-f]{40}$/;
|
||||
|
||||
const sha256 = (s) => createHash('sha256').update(s).digest('hex');
|
||||
const isMajor = (f) => String(f?.severity ?? '').toUpperCase() === 'MAJOR';
|
||||
const isBlocker = (f) => String(f?.severity ?? '').toUpperCase() === 'BLOCKER';
|
||||
|
||||
/** The trailing fenced block when it is a closed `json` block, else null. */
|
||||
function trailingBlock(text) {
|
||||
const last = fencedBlocks(String(text ?? '')).at(-1);
|
||||
return last && last.closed && isJsonBlock(last) ? last : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* sha256 of the trailing JSON block's body (the one reader the gate uses,
|
||||
* findings-schema `fencedBlocks`), or null when the last block is not a
|
||||
* closed `json` block.
|
||||
* @param {string} text
|
||||
* @returns {string|null}
|
||||
*/
|
||||
export function findingsBlockSha(text) {
|
||||
const last = trailingBlock(text);
|
||||
return last ? sha256(last.body) : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The sha256 a K2 `review` record carries for one answer: the canonical JSON
|
||||
* of {blockSha, scopeShaEnd, findingId, answer, reason}, in that key order.
|
||||
* @returns {string}
|
||||
*/
|
||||
export function answerSha({ blockSha, scopeShaEnd, findingId, answer, reason }) {
|
||||
return sha256(JSON.stringify({ blockSha, scopeShaEnd, findingId, answer, reason }));
|
||||
}
|
||||
|
||||
/**
|
||||
* A reason as it is stored, hashed and compared: backticks removed, `|` made
|
||||
* `/`, every whitespace run (newlines included) one space, trimmed, cut to 300
|
||||
* characters.
|
||||
* @param {string} text
|
||||
* @returns {string}
|
||||
*/
|
||||
export function normalizeReason(text) {
|
||||
return String(text ?? '').replace(/`/g, '').replace(/\|/g, '/').replace(/\s+/g, ' ').trim()
|
||||
.slice(0, MAX_REASON).trim();
|
||||
}
|
||||
|
||||
/** The lines of the `## Answers` section (heading excluded), with their indexes; null when absent. */
|
||||
function answersSection(lines) {
|
||||
const at = lines.findIndex((l) => /^##\s+Answers\s*$/.test(l));
|
||||
if (at === -1) return null;
|
||||
let end = at + 1;
|
||||
while (end < lines.length && !/^##\s/.test(lines[end]) && !/^\s{0,3}(`{3,}|~{3,})/.test(lines[end])) end++;
|
||||
return { at, end };
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse the `## Answers` table (the review_version 1.2 grammar, read as
|
||||
* lib/validators/review-validator.mjs reads it): a header of exactly five
|
||||
* columns, a separator, then one row per finding. An `#n` suffix on the id is
|
||||
* stripped; a second row for the same id is an error; a non-pipe line is ignored.
|
||||
* @param {string} body
|
||||
* @returns {{ rows: Array<{id, answer, reason, record, carriedFrom}>, errors: string[] }}
|
||||
*/
|
||||
export function parseAnswers(body) {
|
||||
const lines = String(body ?? '').split('\n');
|
||||
const rows = [];
|
||||
const errors = [];
|
||||
const sec = answersSection(lines);
|
||||
if (sec === null) return { rows, errors };
|
||||
const table = lines.slice(sec.at + 1, sec.end).map((l) => l.trim()).filter((l) => l.startsWith('|'));
|
||||
if (table.length === 0) return { rows, errors };
|
||||
if (table[0] !== ANSWERS_HEADER) {
|
||||
errors.push(`## Answers header must be exactly "${ANSWERS_HEADER}", got ${JSON.stringify(table[0])}`);
|
||||
return { rows, errors };
|
||||
}
|
||||
if (table.length < 2 || !/^\|(?:\s*:?-{3,}:?\s*\|){5}$/.test(table[1])) {
|
||||
errors.push('## Answers table has no separator line of five columns under its header');
|
||||
return { rows, errors };
|
||||
}
|
||||
const seen = new Set();
|
||||
table.slice(2).forEach((line, i) => {
|
||||
const where = `## Answers row ${i + 1}`;
|
||||
if (!line.endsWith('|')) { errors.push(`${where} does not end with "|"`); return; }
|
||||
const cells = line.slice(1, -1).split('|').map((c) => c.trim());
|
||||
if (cells.length !== 5) { errors.push(`${where} has ${cells.length} cells, not 5`); return; }
|
||||
const [finding, answer, reason, record, carriedFrom] = cells;
|
||||
const m = finding.match(ANSWER_FINDING_RE);
|
||||
if (!m) errors.push(`${where}: Finding ${JSON.stringify(finding)} is not a 40-char hex id (optionally #n)`);
|
||||
if (!ANSWER_VALUES.includes(answer)) errors.push(`${where}: Answer must be accepted|fixed, got ${JSON.stringify(answer)}`);
|
||||
if (answer === 'accepted' && reason === '') errors.push(`${where}: an accepted answer needs a non-empty Reason`);
|
||||
if (!SHA256_RE.test(record)) errors.push(`${where}: Record ${JSON.stringify(record)} is not a 64-char hex record hash`);
|
||||
if (carriedFrom !== '-' && !CARRIED_FROM_RE.test(carriedFrom)) {
|
||||
errors.push(`${where}: Carried from must be "-" or review-round-<run12>-<n>.md#<id>, got ${JSON.stringify(carriedFrom)}`);
|
||||
}
|
||||
if (m) {
|
||||
if (seen.has(m[1])) { errors.push(`${where}: a second row for finding ${m[1]} (at most one row per finding)`); return; }
|
||||
seen.add(m[1]);
|
||||
rows.push({ id: m[1], answer, reason, record, carriedFrom });
|
||||
}
|
||||
});
|
||||
return { rows, errors };
|
||||
}
|
||||
|
||||
/**
|
||||
* A security finding: its catalogue category is `security`, or it came from
|
||||
* the `security-review` engine. Never accepted, never left out as pre-existing.
|
||||
*/
|
||||
export function isSecurityFinding(f) {
|
||||
return getRule(f?.rule_key)?.category === 'security' || f?.engine === 'security-review';
|
||||
}
|
||||
|
||||
/**
|
||||
* Every MAJOR finding (case-insensitive) that counts toward the lock. A finding
|
||||
* is left out only when `preExisting(f)` is true and it is not a security finding.
|
||||
* @param {{findings?: object[]}} payload
|
||||
* @param {{ preExisting?: (f: object) => boolean }} [opts]
|
||||
* @returns {object[]}
|
||||
*/
|
||||
export function countingMajors(payload, { preExisting = () => false } = {}) {
|
||||
const findings = Array.isArray(payload?.findings) ? payload.findings : [];
|
||||
return findings.filter((f) => f && typeof f === 'object' && isMajor(f)
|
||||
&& !(preExisting(f) === true && !isSecurityFinding(f)));
|
||||
}
|
||||
|
||||
/** The parsed trailing JSON block, or null. */
|
||||
function payloadOf(text) {
|
||||
const last = trailingBlock(text);
|
||||
if (!last) return null;
|
||||
try {
|
||||
const p = JSON.parse(last.body);
|
||||
return p && typeof p === 'object' && !Array.isArray(p) ? p : null;
|
||||
} catch { return null; }
|
||||
}
|
||||
|
||||
/** The review body (frontmatter cut off), as the gate's reader sees it. */
|
||||
function bodyOf(text) {
|
||||
const m = /^---\r?\n[\s\S]*?\r?\n---\r?\n/.exec(String(text ?? ''));
|
||||
return m ? String(text).slice(m[0].length) : String(text ?? '');
|
||||
}
|
||||
|
||||
/** The latest K2 record of kind `review` for `subject`, or null. */
|
||||
function latestReviewRecord(records, subject) {
|
||||
for (let i = records.length - 1; i >= 0; i--) {
|
||||
const r = records[i];
|
||||
if (r && r.kind === 'review' && r.subject === subject) return r;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide whether every counting MAJOR of a review is answered. Pure: the K2
|
||||
* records and the review's subject (its path relative to the git top level)
|
||||
* come in `ctx`; with no subject or no records nothing is answered.
|
||||
* @param {string} text - review.md
|
||||
* @param {{ records?: object[], reviewSubject?: string, preExisting?: (f) => boolean, archived?: unknown }} [ctx]
|
||||
* A review with no readable trailing JSON block is `review-not-measured:`
|
||||
* (not ok), never ok.
|
||||
* @returns {{ok: true} | {ok: false, unanswered: string[], fixedPending: string[], reason: string}}
|
||||
*/
|
||||
export function decideAnswers(text, { records = [], reviewSubject, preExisting = () => false } = {}) {
|
||||
const payload = payloadOf(text);
|
||||
// A review whose findings cannot be read is never answered (fail closed).
|
||||
if (!payload) {
|
||||
return { ok: false, unanswered: [], fixedPending: [], reason: 'review-not-measured: no readable trailing JSON block' };
|
||||
}
|
||||
const majors = countingMajors(payload, { preExisting });
|
||||
if (majors.length === 0) return { ok: true };
|
||||
const blockSha = findingsBlockSha(text);
|
||||
const scopeShaEnd = reviewedSha(text) || null;
|
||||
const { rows } = parseAnswers(bodyOf(text));
|
||||
const rowOf = new Map(rows.map((r) => [r.id, r]));
|
||||
const unanswered = [];
|
||||
const fixedPending = [];
|
||||
for (const f of majors) {
|
||||
const row = rowOf.get(f.id);
|
||||
const bound = (answer) => {
|
||||
if (!row || !blockSha || !scopeShaEnd || !reviewSubject) return false;
|
||||
const reason = normalizeReason(row.reason);
|
||||
const rec = latestReviewRecord(Array.isArray(records) ? records : [], `${reviewSubject}#${f.id}`);
|
||||
return Boolean(rec) && rec.hash === row.record
|
||||
&& rec.sha256 === answerSha({ blockSha, scopeShaEnd, findingId: f.id, answer, reason });
|
||||
};
|
||||
if (row && row.answer === 'accepted' && !isSecurityFinding(f)) {
|
||||
const reason = normalizeReason(row.reason);
|
||||
if (reason !== '' && !isHollowAnswer(reason) && bound('accepted')) continue;
|
||||
}
|
||||
if (row && row.answer === 'fixed' && bound('fixed')) { fixedPending.push(f.id); continue; }
|
||||
unanswered.push(f.id);
|
||||
}
|
||||
if (unanswered.length === 0 && fixedPending.length === 0) return { ok: true };
|
||||
const reason = unanswered.length === 0
|
||||
? `review-unanswered: fixed-pending ${fixedPending.length}`
|
||||
: `review-unanswered: ${unanswered.length} MAJOR without an answer`;
|
||||
return { ok: false, unanswered, fixedPending, reason };
|
||||
}
|
||||
|
||||
/**
|
||||
* The repo whose top-level approval log holds the answers: the git top level
|
||||
* of `dir` (real path), or null outside a work tree.
|
||||
* @param {string} dir
|
||||
* @returns {string|null}
|
||||
*/
|
||||
export function answerLogRepo(dir) {
|
||||
if (!dir) return null;
|
||||
const r = spawnSync('git', ['-C', resolve(dir), 'rev-parse', '--show-toplevel'], { encoding: 'utf8' });
|
||||
if (r.status !== 0 || !r.stdout.trim()) return null;
|
||||
try { return realpathSync(r.stdout.trim()); } catch { return null; }
|
||||
}
|
||||
|
||||
/** A review's K2 subject path: relative to the top level when inside it. */
|
||||
export function reviewSubject(top, reviewPath) {
|
||||
return normalizeSubject(top, resolve(reviewPath));
|
||||
}
|
||||
|
||||
/**
|
||||
* The K2 records of the top-level log, or none (with why) when there is no
|
||||
* work tree or the log is broken, diverged or unreadable.
|
||||
* @returns {{ records: object[], problem: string|null, top: string|null }}
|
||||
*/
|
||||
export function answerRecords(dir) {
|
||||
const top = answerLogRepo(dir);
|
||||
if (!top) return { records: [], problem: 'no git work tree for the review', top: null };
|
||||
const v = verifyLog({ repo: top });
|
||||
if (v.exit !== 0) return { records: [], problem: `approval log does not verify (${v.status}: ${v.detail ?? ''})`, top };
|
||||
const log = readLog(top);
|
||||
if (log.error || log.broken) return { records: [], problem: 'approval log does not verify', top };
|
||||
return { records: log.records, problem: null, top };
|
||||
}
|
||||
|
||||
/** Write `text` to `p` through a temporary file in the same directory and a rename. */
|
||||
function writeAtomic(p, text) {
|
||||
const tmp = join(dirname(p), `.${basename(p)}.${process.pid}.tmp`);
|
||||
writeFileSync(tmp, text);
|
||||
renameSync(tmp, p);
|
||||
}
|
||||
|
||||
/**
|
||||
* The review text with `row` written into `## Answers` (replacing the row for
|
||||
* the same finding id). An absent section is created just before the trailing
|
||||
* JSON block, after `## Remediation Steps`. Returns null when the review has no
|
||||
* trailing JSON block to place it before.
|
||||
*/
|
||||
export function withAnswerRow(text, id, row) {
|
||||
const lines = String(text).split('\n');
|
||||
const sec = answersSection(lines);
|
||||
if (sec) {
|
||||
const table = [];
|
||||
for (let i = sec.at + 1; i < sec.end; i++) if (lines[i].trim().startsWith('|')) table.push(i);
|
||||
if (table.length >= 2 && lines[table[0]].trim() === ANSWERS_HEADER) {
|
||||
const same = table.slice(2).find((i) => {
|
||||
const cell = lines[i].trim().slice(1).split('|')[0]?.trim() ?? '';
|
||||
const m = cell.match(ANSWER_FINDING_RE);
|
||||
return m && m[1] === id;
|
||||
});
|
||||
if (same !== undefined) lines[same] = row;
|
||||
else lines.splice(table.at(-1) + 1, 0, row);
|
||||
return lines.join('\n');
|
||||
}
|
||||
if (table.length === 0) {
|
||||
lines.splice(sec.at + 1, 0, '', ANSWERS_HEADER, ANSWERS_SEPARATOR, row);
|
||||
return lines.join('\n');
|
||||
}
|
||||
return null;
|
||||
}
|
||||
const last = trailingBlock(text);
|
||||
if (!last || last.body === '') return null;
|
||||
const bodyAt = String(text).lastIndexOf(last.body);
|
||||
if (bodyAt <= 0) return null;
|
||||
const fenceAt = String(text).lastIndexOf('\n', bodyAt - 2) + 1;
|
||||
const before = String(text).slice(0, fenceAt).replace(/\n*$/, '\n\n');
|
||||
return `${before}## Answers\n\n${ANSWERS_HEADER}\n${ANSWERS_SEPARATOR}\n${row}\n\n${String(text).slice(fenceAt)}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Open counting MAJORs of the review at `reviewPath`, read against the
|
||||
* top-level log of `repo` (else the review's directory).
|
||||
*/
|
||||
export function pendingFindings({ reviewPath, repo }) {
|
||||
let text;
|
||||
try { text = readFileSync(reviewPath, 'utf8'); } catch (e) {
|
||||
return { exit: 2, reason: `cannot read ${reviewPath}: ${e.message}` };
|
||||
}
|
||||
const payload = payloadOf(text);
|
||||
if (!payload) return { exit: 2, reason: `${reviewPath} has no readable trailing JSON block` };
|
||||
const { records, top, problem } = answerRecords(repo ?? dirname(resolve(reviewPath)));
|
||||
const d = decideAnswers(text, { records, reviewSubject: top ? reviewSubject(top, reviewPath) : undefined });
|
||||
if (d.ok) return { exit: 0, pending: [] };
|
||||
const open = new Set([...d.unanswered, ...d.fixedPending]);
|
||||
const pending = countingMajors(payload).filter((f) => open.has(f.id)).map((f) => ({
|
||||
id: f.id, title: f.title ?? '', file: f.file ?? '', line: f.line ?? null,
|
||||
security: isSecurityFinding(f),
|
||||
fixed_pending: d.fixedPending.includes(f.id),
|
||||
detail: String(f.detail || f.recommended_action || ''),
|
||||
}));
|
||||
return { exit: 1, pending, reason: d.reason, ...(problem ? { log: problem } : {}) };
|
||||
}
|
||||
|
||||
/**
|
||||
* Record one answer: refuse, or append the K2 record, write the row and commit
|
||||
* the top-level log.
|
||||
*/
|
||||
export function recordAnswer({ reviewPath, finding, answer, repo, round, store, trailer, reason: rawReason }) {
|
||||
const refuse = (reason) => ({ exit: 1, recorded: false, reason });
|
||||
const cannot = (reason) => ({ exit: 2, recorded: false, reason });
|
||||
if (!reviewPath) return cannot('--review is required');
|
||||
if (!repo) return cannot('--repo is required');
|
||||
if (!ANSWER_VALUES.includes(answer)) return cannot(`--answer must be one of ${ANSWER_VALUES.join('|')}`);
|
||||
if (!/^[1-9]\d*$/.test(String(round ?? ''))) return cannot('--round must be a positive integer');
|
||||
if (!['git', 'local'].includes(store)) return cannot('--store must be one of git|local');
|
||||
const bad = trailerProblem(trailer);
|
||||
if (bad) return refuse(bad);
|
||||
const id = String(finding ?? '').trim();
|
||||
if (!HEX40_RE.test(id)) return refuse(`--finding ${JSON.stringify(finding ?? '')} is not a 40-char hex finding id`);
|
||||
const reason = normalizeReason(rawReason);
|
||||
if (reason === '') return refuse('the reason is empty: an answer needs the operator\'s reason in plain words');
|
||||
if (isHollowAnswer(reason)) return refuse(`the reason "${reason}" is a hollow answer, not a reason`);
|
||||
|
||||
let text;
|
||||
try { text = readFileSync(reviewPath, 'utf8'); } catch (e) { return cannot(`cannot read ${reviewPath}: ${e.message}`); }
|
||||
const payload = payloadOf(text);
|
||||
if (!payload || !Array.isArray(payload.findings)) return cannot(`${reviewPath} has no readable trailing JSON block`);
|
||||
const f = payload.findings.find((x) => x && x.id === id);
|
||||
if (f && isBlocker(f)) return refuse(`${id} is a BLOCKER: a BLOCKER is never answered, it is fixed`);
|
||||
if (!f || !countingMajors(payload).some((x) => x.id === id)) return refuse(`${id} is not a counting MAJOR of this review`);
|
||||
if (answer === 'accepted' && isSecurityFinding(f)) {
|
||||
return refuse(`${id} is a security finding: it is never accepted, only fixed`);
|
||||
}
|
||||
const blockSha = findingsBlockSha(text);
|
||||
const scopeShaEnd = reviewedSha(text);
|
||||
if (!blockSha) return cannot(`${reviewPath}: the last fenced block is not a closed json block`);
|
||||
if (!scopeShaEnd) return cannot(`${reviewPath} has no scope_sha_end that is a commit sha`);
|
||||
|
||||
const top = answerLogRepo(repo);
|
||||
if (!top) return cannot(`${repo} is not inside a git work tree: no approval log to write`);
|
||||
const subject = `${reviewSubject(top, reviewPath)}#${id}`;
|
||||
// Place the row first (a placeholder hash), so nothing is appended to the
|
||||
// log for an answer whose row cannot be written.
|
||||
const rowWith = (hash) => `| ${id} | ${answer} | ${reason} | ${hash} | - |`;
|
||||
if (withAnswerRow(text, id, rowWith('0'.repeat(64))) === null) {
|
||||
return cannot(`${reviewPath}: the ## Answers table could not be placed (a malformed ## Answers section or no trailing JSON block); nothing appended`);
|
||||
}
|
||||
// The atomic write needs to create a file in the review's directory.
|
||||
try { accessSync(dirname(resolve(reviewPath)), fsConstants.W_OK); } catch (e) {
|
||||
return cannot(`cannot write ${reviewPath}: ${e.message}; nothing appended`);
|
||||
}
|
||||
const app = appendApproval({
|
||||
repo: top, kind: 'review', subject, sha: answerSha({ blockSha, scopeShaEnd, findingId: id, answer, reason }),
|
||||
});
|
||||
if (!app.appended) return { exit: app.exit, recorded: false, reason: app.reason };
|
||||
|
||||
const next = withAnswerRow(text, id, rowWith(app.record.hash));
|
||||
const res = {
|
||||
recorded: true, finding: id, answer, reason, record: app.record.hash, subject, log: logPath(top),
|
||||
};
|
||||
if (next === null) {
|
||||
return { ...res, exit: 2, review_written: false, committed: false, commit_reason: 'the ## Answers table could not be placed; the K2 record is appended' };
|
||||
}
|
||||
try { writeAtomic(reviewPath, next); } catch (e) {
|
||||
return { ...res, exit: 2, review_written: false, committed: false, commit_reason: `cannot write ${reviewPath}: ${e.message}` };
|
||||
}
|
||||
const c = commitArtifacts({ repo: top, store, message: `docs(voyage): review answers (round ${round})`, trailer });
|
||||
return {
|
||||
...res, exit: 0, review_written: true, committed: c.committed === true,
|
||||
...(c.commit ? { commit: c.commit } : {}),
|
||||
...(c.committed ? {} : { commit_reason: c.reason ?? 'not committed' }),
|
||||
};
|
||||
}
|
||||
|
||||
// ---- CLI ----
|
||||
|
||||
function opt(args, name) {
|
||||
const i = args.indexOf(name);
|
||||
return i >= 0 ? args[i + 1] : undefined;
|
||||
}
|
||||
|
||||
if (isMain(import.meta.url)) {
|
||||
const [cmd, ...args] = process.argv.slice(2);
|
||||
let result;
|
||||
try {
|
||||
if (cmd === 'pending') {
|
||||
const reviewPath = opt(args, '--review');
|
||||
result = reviewPath ? pendingFindings({ reviewPath, repo: opt(args, '--repo') }) : { exit: 2, reason: '--review is required' };
|
||||
} else if (cmd === 'record') {
|
||||
let reason = '';
|
||||
try { reason = readFileSync(0, 'utf8'); } catch { reason = ''; }
|
||||
result = recordAnswer({
|
||||
reviewPath: opt(args, '--review'), finding: opt(args, '--finding'), answer: opt(args, '--answer'),
|
||||
repo: opt(args, '--repo'), round: opt(args, '--round'), store: opt(args, '--store'),
|
||||
trailer: opt(args, '--trailer'), reason,
|
||||
});
|
||||
} else {
|
||||
process.stderr.write('Usage: review-answers.mjs pending|record — see the header of lib/review/review-answers.mjs\n');
|
||||
process.exit(2);
|
||||
}
|
||||
} catch (e) {
|
||||
result = { exit: 2, reason: `review-answers crashed: ${e.message}` };
|
||||
}
|
||||
const { exit, ...rest } = result;
|
||||
process.stdout.write(JSON.stringify(rest) + '\n');
|
||||
process.exit(exit);
|
||||
}
|
||||
|
|
@ -23,9 +23,20 @@
|
|||
// `--repo <dir>`, a check that could not look (reason
|
||||
// `review-not-measured: …`: the path is missing, it is no git
|
||||
// repository, or a git call failed; M4) — printed as it is, never
|
||||
// as `review-stale`.
|
||||
// as `review-stale`, or an ALLOW/WARN review with a counting MAJOR
|
||||
// that is not answered (reason `review-unanswered: <n> MAJOR without
|
||||
// an answer`, or `review-unanswered: fixed-pending <n>` when every
|
||||
// open one is answered `fixed`; `decideOpenFindings`, read against
|
||||
// the top-level approval log of `--repo`, else of the review file's
|
||||
// git work tree, else no records). The `--json` output carries
|
||||
// `unanswered` and `fixed_pending` id arrays.
|
||||
// Exit 2 is NEVER green.
|
||||
//
|
||||
// The pure `decideVerdict` stays text-only: it never reads the answers. The
|
||||
// two callers that add the open-findings check are `decideVerdictFile` (this
|
||||
// CLI) and scripts/release-check.mjs `checkReview`. The check never keys on
|
||||
// the verdict label: an ALLOW over a MAJOR is counted as a WARN is.
|
||||
//
|
||||
// Fail-closed reading: a missing `verdict` is exit 2, never ALLOW. A verdict
|
||||
// that is GREENER than the file's own findings (ALLOW/WARN over a BLOCKER, or a
|
||||
// frontmatter verdict the trailing JSON block disagrees with) is exit 2 as well
|
||||
|
|
@ -39,11 +50,15 @@
|
|||
// own design — neither source blocks on its own.
|
||||
|
||||
import { readFileSync, existsSync } from 'node:fs';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { dirname } from 'node:path';
|
||||
import { validateReviewContent, VERDICT_VALUES } from '../validators/review-validator.mjs';
|
||||
import { fencedBlocks, isJsonBlock } from './findings-schema.mjs';
|
||||
import { illegalBriefLowering } from './coordinator-contract.mjs';
|
||||
import { reviewedSha, checkReviewedCommit } from './reviewed-commit.mjs';
|
||||
import { isMain } from '../util/is-main.mjs';
|
||||
import { resolveRepo } from '../util/approval-log.mjs';
|
||||
import { answerRecords, decideAnswers, reviewSubject } from './review-answers.mjs';
|
||||
|
||||
export const EXIT = Object.freeze({ PASS: 0, BLOCK: 1, UNDECIDED: 2 });
|
||||
|
||||
|
|
@ -169,9 +184,38 @@ function blockerSectionEntries(body) {
|
|||
return (section.match(/^###\s+\S/gm) ?? []).length;
|
||||
}
|
||||
|
||||
/**
|
||||
* The open-findings check: every counting MAJOR of an ALLOW/WARN review must be
|
||||
* answered (lib/review/review-answers.mjs `decideAnswers`). K2 records come
|
||||
* from the top-level approval log of `repo`, else of the review's directory;
|
||||
* a broken, diverged or unreadable log, or no work tree, means no records.
|
||||
* @param {string} text - review.md
|
||||
* @param {{ repo?: string, reviewPath: string }} opts
|
||||
* @returns {{ exit: 0|2, reason: string, unanswered: string[], fixed_pending: string[] }}
|
||||
*/
|
||||
export function decideOpenFindings(text, { repo, reviewPath } = {}) {
|
||||
const { records, top, problem } = answerRecords(repo ?? (reviewPath ? dirname(reviewPath) : undefined));
|
||||
const subject = top && reviewPath ? reviewSubject(top, reviewPath) : undefined;
|
||||
const d = decideAnswers(text, { records, reviewSubject: subject });
|
||||
if (d.ok) return { exit: EXIT.PASS, reason: 'every counting MAJOR is answered', unanswered: [], fixed_pending: [] };
|
||||
return {
|
||||
exit: EXIT.UNDECIDED,
|
||||
reason: problem ? `${d.reason} (${problem})` : d.reason,
|
||||
unanswered: d.unanswered,
|
||||
fixed_pending: d.fixedPending,
|
||||
};
|
||||
}
|
||||
|
||||
function insideWorkTree(dir) {
|
||||
const r = spawnSync('git', ['-C', dir, 'rev-parse', '--is-inside-work-tree'], { encoding: 'utf8' });
|
||||
return r.status === 0 && r.stdout.trim() === 'true';
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide on a review.md path. With `repo`, an ALLOW/WARN review must also be
|
||||
* of the commit being merged there (R6); BLOCK and exit 2 are returned as they are.
|
||||
* of the commit being merged there (R6); BLOCK and exit 2 are returned as they
|
||||
* are. Then every counting MAJOR must be answered (`decideOpenFindings`),
|
||||
* read against `repo`, else the review's git work tree, else no records.
|
||||
* @param {string} filePath
|
||||
* @param {{ repo?: string }} [opts]
|
||||
*/
|
||||
|
|
@ -184,12 +228,20 @@ export function decideVerdictFile(filePath, { repo } = {}) {
|
|||
return undecided(`cannot read ${filePath}: ${e.message}`);
|
||||
}
|
||||
const decided = decideVerdict(text);
|
||||
if (repo === undefined || decided.exit !== EXIT.PASS) return decided;
|
||||
const sha = reviewedSha(text);
|
||||
if (!sha) return undecided('review-stale: review.md has no scope_sha_end that is a commit sha', decided.verdict);
|
||||
const commit = checkReviewedCommit({ repo, sha });
|
||||
if (!commit.ok) return undecided(commit.reason, decided.verdict);
|
||||
return decided;
|
||||
if (decided.exit !== EXIT.PASS) return decided;
|
||||
if (repo !== undefined) {
|
||||
const sha = reviewedSha(text);
|
||||
if (!sha) return undecided('review-stale: review.md has no scope_sha_end that is a commit sha', decided.verdict);
|
||||
const commit = checkReviewedCommit({ repo, sha });
|
||||
if (!commit.ok) return undecided(commit.reason, decided.verdict);
|
||||
}
|
||||
const here = dirname(filePath);
|
||||
const openRepo = repo !== undefined ? repo : (insideWorkTree(here) ? resolveRepo(undefined, here) : undefined);
|
||||
const open = decideOpenFindings(text, { repo: openRepo, reviewPath: filePath });
|
||||
if (open.exit !== EXIT.PASS) {
|
||||
return { ...undecided(open.reason, decided.verdict), unanswered: open.unanswered, fixed_pending: open.fixed_pending };
|
||||
}
|
||||
return { ...decided, unanswered: [], fixed_pending: [] };
|
||||
}
|
||||
|
||||
if (isMain(import.meta.url)) {
|
||||
|
|
|
|||
|
|
@ -241,19 +241,38 @@ const HOLLOW = [
|
|||
'ok', 'okay', 'sure', 'fine', 'kjør på', 'kjor pa',
|
||||
];
|
||||
|
||||
/** @returns {{ approved: boolean, reason: 'explicit'|'empty'|'hollow'|'not-explicit' }} */
|
||||
// AskUserQuestion's own guidance puts «(Recommended)» after the label it
|
||||
// recommends, so the operator's real «Approve (Recommended)» was refused in the
|
||||
// voyage-lekerepo trial (2026-10-06). The suffix is stripped; what is left must
|
||||
// still be an explicit approval on its own.
|
||||
const RECOMMENDED_SUFFIX = /\s*\((?:recommended|anbefalt)\)\s*$/u;
|
||||
|
||||
export function isExplicitApproval(answer) {
|
||||
const t = String(answer ?? '').trim().toLowerCase().replace(/[\s.!]+$/u, '')
|
||||
function normalizeAnswer(answer) {
|
||||
return String(answer ?? '').trim().toLowerCase().replace(/[\s.!]+$/u, '')
|
||||
.replace(RECOMMENDED_SUFFIX, '').replace(/[\s.!]+$/u, '').replace(/\s+/g, ' ');
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `text` is a hollow answer («sounds good», «whatever you think», …):
|
||||
* the HOLLOW list, read the way isExplicitApproval reads it. The empty answer
|
||||
* is not hollow here; a caller that refuses it says so on its own.
|
||||
* @param {string} text
|
||||
* @returns {boolean}
|
||||
*/
|
||||
export function isHollowAnswer(text) {
|
||||
return hollowNormalized(normalizeAnswer(text));
|
||||
}
|
||||
|
||||
function hollowNormalized(t) {
|
||||
return t !== '' && HOLLOW.some((h) => t === h || (h.includes(' ') && t.includes(h)));
|
||||
}
|
||||
|
||||
/** @returns {{ approved: boolean, reason: 'explicit'|'empty'|'hollow'|'not-explicit' }} */
|
||||
export function isExplicitApproval(answer) {
|
||||
const t = normalizeAnswer(answer);
|
||||
if (!t) return { approved: false, reason: 'empty' };
|
||||
if (EXPLICIT.has(t)) return { approved: true, reason: 'explicit' };
|
||||
if (HOLLOW.some((h) => t === h || (h.includes(' ') && t.includes(h)))) return { approved: false, reason: 'hollow' };
|
||||
if (hollowNormalized(t)) return { approved: false, reason: 'hollow' };
|
||||
return { approved: false, reason: 'not-explicit' };
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ const DEFAULT_MESSAGE = 'docs(voyage): record approval';
|
|||
const TRAILER_RE = /^Co-Authored-By: [^\s<>{}](?:[^<>{}\n]*[^\s<>{}])? <[^\s<>@]+@[^\s<>]+>$/;
|
||||
|
||||
/** @returns {string|null} why `trailer` is refused, or null when it is valid */
|
||||
function trailerProblem(trailer) {
|
||||
export function trailerProblem(trailer) {
|
||||
if (typeof trailer !== 'string' || trailer === '') {
|
||||
return '--trailer "Co-Authored-By: <name> <<email>>" is required — nothing committed';
|
||||
}
|
||||
|
|
|
|||
55
lib/util/doors.mjs
Normal file
55
lib/util/doors.mjs
Normal file
|
|
@ -0,0 +1,55 @@
|
|||
// lib/util/doors.mjs
|
||||
// The `### Doors` list of a CHANGELOG release section: every delivery says
|
||||
// whether it is a one-way door (cannot be walked back without a cost to users)
|
||||
// or a two-way door (can). Moved here from scripts/release-check.mjs, which
|
||||
// re-exports these names, because lib never imports scripts and the review
|
||||
// evidence writer reads the door too. Door types are credited to
|
||||
// mattpocock/skills@c55ee46 (idea only, no text copied).
|
||||
|
||||
export const DOOR_LINE_RE = /^- (one-way|two-way): \S/;
|
||||
|
||||
/**
|
||||
* The CHANGELOG section for `version`, heading line excluded, or null. Matches
|
||||
* `## v1.2.3 …`, `## [1.2.3] …` and `## 1.2.3 …` exactly: 1.1.0 is not 1.10.0
|
||||
* and not 1.1.0-pre.
|
||||
*/
|
||||
export function releaseSection(text, version) {
|
||||
if (typeof text !== 'string' || !version) return null;
|
||||
const lines = text.split('\n');
|
||||
const headingVersion = (line) => {
|
||||
const m = line.match(/^##\s+\[?v?([^\]\s(]+)\]?/);
|
||||
return m ? m[1] : null;
|
||||
};
|
||||
let start = -1;
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
if (/^##\s/.test(lines[i]) && headingVersion(lines[i]) === version) { start = i + 1; break; }
|
||||
}
|
||||
if (start === -1) return null;
|
||||
let end = lines.length;
|
||||
for (let i = start; i < lines.length; i++) {
|
||||
if (/^##\s/.test(lines[i])) { end = i; break; }
|
||||
}
|
||||
return lines.slice(start, end).join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* The `### Doors` list of a release section. Top-level `- ` lines must read
|
||||
* `- one-way: …` or `- two-way: …`; indented lines continue the entry above.
|
||||
* @returns {{ found: boolean, entries: Array<{door: string, text: string}>, bad: string[] }}
|
||||
*/
|
||||
export function parseDoors(section) {
|
||||
const lines = String(section ?? '').split('\n');
|
||||
const start = lines.findIndex((l) => /^###\s+Doors\s*$/.test(l));
|
||||
if (start === -1) return { found: false, entries: [], bad: [] };
|
||||
const entries = [];
|
||||
const bad = [];
|
||||
for (let i = start + 1; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
if (/^#{2,3}\s/.test(line)) break;
|
||||
if (!line.startsWith('- ')) continue;
|
||||
const m = line.match(DOOR_LINE_RE);
|
||||
if (m) entries.push({ door: m[1], text: line.slice(m[0].length - 1).trim() });
|
||||
else bad.push(line);
|
||||
}
|
||||
return { found: true, entries, bad };
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue