open/voyage
1
0
Fork 0
Code Issues Releases 13 Activity

refactor: simplify pass (trekexecute)

Browse source 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-11-03 12:48:04 +01:00
commit f1f5ab5528
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
5 changed files with 30 additions and 42 deletions
Download patch file Download diff file Expand all files Collapse all files

2
lib/review/engine-compare.mjs
View file

@ -44,7 +44,7 @@ export { tripletOf, normTitle, rowOf, trimmedTitle };
const PUT_BACK_SEVERITIES = ['BLOCKER', 'MAJOR'];
const VERDICT_RANK = { ALLOW: 0, WARN: 1, BLOCK: 2 };
const ENGINE_OF_REVIEWER = Object.freeze(Object.fromEntries(Object.entries(ENGINES).map(([engine, s]) => [s.reviewer, engine])));
export const ENGINE_OF_REVIEWER = Object.freeze(Object.fromEntries(Object.entries(ENGINES).map(([engine, s]) => [s.reviewer, engine])));
const isPlainObject = (x) => x !== null && typeof x === 'object' && !Array.isArray(x);
/**

29
lib/review/review-answers.mjs
View file

@ -80,14 +80,14 @@ import { spawnSync } from 'node:child_process';
import { accessSync, constants as fsConstants, lstatSync, 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 { preExistingCheck, scopeShas } from './pre-existing.mjs';
import { archiveName, frontmatterField, runId, sameFinding } from './review-rounds.mjs';
import { archiveName, frontmatterField, isSecurityFinding, runId, sameFinding } from './review-rounds.mjs';
import {
appendApproval, isHollowAnswer, logPath, normalizeSubject, readLog, verifyLog,
} from '../util/approval-log.mjs';
import { commitArtifacts, trailerProblem } from '../util/artifact-commit.mjs';
import { remediationStepSources } from '../validators/review-validator.mjs';
import { isMain } from '../util/is-main.mjs';
export const ANSWER_VALUES = Object.freeze(['accepted', 'fixed']);
@ -203,13 +203,8 @@ export function parseAnswers(body) {
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';
}
/** A security finding (defined in review-rounds.mjs; re-exported here for the existing callers). */
export { isSecurityFinding };
/**
* Every MAJOR finding (case-insensitive) that counts toward the lock. A finding
@ -240,22 +235,6 @@ function bodyOf(text) {
return m ? String(text).slice(m[0].length) : String(text ?? '');
}
/** Finding IDs named by `Source finding:` lines under `## Remediation Steps` step headings. */
// Copied from lib/validators/review-validator.mjs (remediationStepSources; not exported there).
function remediationStepSources(body) {
const start = body.search(/^##\s+Remediation Steps\b/m);
if (start === -1) return new Set();
const rest = body.slice(start).replace(/^##\s+Remediation Steps[^\n]*\n/, '');
const next = rest.search(/^##\s+/m);
const section = next === -1 ? rest : rest.slice(0, next);
const ids = new Set();
for (const step of section.split(/^###\s+Step\s+R\d+\b/m).slice(1)) {
const m = step.match(/Source finding:\**\s*`?([0-9a-f]{40})`?/);
if (m) ids.add(m[1]);
}
return ids;
}
/** The latest K2 record of kind `review` for `subject`, or null. */
function latestReviewRecord(records, subject) {
for (let i = records.length - 1; i >= 0; i--) {

24
lib/review/review-finalize.mjs
View file

@ -107,10 +107,10 @@ import { FULL_SHA_RE } from './reviewed-commit.mjs';
import { markFindings, scopeShas } from './pre-existing.mjs';
import { changedPaths, instructionSet } from './instruction-scope.mjs';
import { isSecurityFinding } from './review-answers.mjs';
import { ENGINES } from './engine-findings.mjs';
import { carry, frontmatterField, MAX_REVIEW_ROUNDS, runId } from './review-rounds.mjs';
import { applyPolicy, readPolicy } from './review-policy.mjs';
import { putBack } from './engine-compare.mjs';
import { ENGINES } from './engine-findings.mjs';
import { ENGINE_OF_REVIEWER, putBack } from './engine-compare.mjs';
import { isMain } from '../util/is-main.mjs';
const SEVERITIES = ['BLOCKER', 'MAJOR', 'MINOR', 'SUGGESTION'];
@ -121,11 +121,17 @@ const SOURCE_RE = /Source finding:\**\s*`?([0-9a-f]{40})`?/;
const ARCHIVE_RE = /^review-round-([0-9a-f]{12})-(\d+)\.md$/;
const OUT_RE = /^\.review-out-(.+)\.md$/;
const PAYLOAD_RE = /^\.review-engine-payload-.+\.json$/;
const ENGINE_OF_REVIEWER = Object.freeze(Object.fromEntries(Object.entries(ENGINES).map(([engine, s]) => [s.reviewer, engine])));
const isPlainObject = (x) => x !== null && typeof x === 'object' && !Array.isArray(x);
const sevOf = (f) => String(f?.severity ?? '').toUpperCase();
/** `{SEVERITY: n}` over the findings marked pre-existing. */
function preExistingCounts(findings) {
const counts = Object.fromEntries(SEVERITIES.map((s) => [s, 0]));
for (const f of findings) if (f && f.pre_existing === true && sevOf(f) in counts) counts[sevOf(f)] += 1;
return counts;
}
/** The review split at the trailing closed `json` block: {head, fence, body, tail}, or null. */
function splitTrailing(text) {
const last = fencedBlocks(text).at(-1);
@ -384,11 +390,7 @@ function passPreExisting(payload, { repo, text }) {
const carried = (f) => Boolean(f && typeof f === 'object' && f.carried_from);
markFindings({ findings: payload.findings.filter((f) => !carried(f)) }, { repo, start, end });
for (const f of payload.findings) if (carried(f)) f.pre_existing = false;
const counts = Object.fromEntries(SEVERITIES.map((s) => [s, 0]));
for (const f of payload.findings) {
const sev = sevOf(f);
if (f && f.pre_existing === true && sev in counts) counts[sev] += 1;
}
const counts = preExistingCounts(payload.findings);
payload.pre_existing_counts = counts;
const section = preExistingSection(payload.findings, counts);
return {
@ -484,8 +486,7 @@ function passPolicy(payload, ctx) {
ctx.recount = true;
// The pre-existing pass ran first: its counts and list keep only the findings still reported.
if (isPlainObject(payload.pre_existing_counts)) {
const counts = Object.fromEntries(SEVERITIES.map((sv) => [sv, 0]));
for (const f of payload.findings) if (f.pre_existing === true && sevOf(f) in counts) counts[sevOf(f)] += 1;
const counts = preExistingCounts(payload.findings);
payload.pre_existing_counts = counts;
preSection = preExistingSection(payload.findings, counts);
}
@ -621,7 +622,8 @@ function placeEntries(head, findings, added, putBackSet) {
lines = appendToSection(lines, findingsHeading(sev), `## Findings (${sev})`, entryLines(f, putBackSet), findingsBefore(sev));
continue;
}
const at = lines.findIndex((l, i) => i > fmEnd && new RegExp(`^###\\s+${f.id}\\s*$`).test(l));
const heading = new RegExp(`^###\\s+${f.id}\\s*$`);
const at = lines.findIndex((l, i) => i > fmEnd && heading.test(l));
if (at === -1) continue;
let h = at - 1;
while (h > fmEnd && !/^##\s/.test(lines[h])) h--;

15
lib/review/review-rounds.mjs
View file

@ -127,7 +127,8 @@ export function prepare({ projectDir, since, repo, workflow = false, dryRun = fa
const review = readRegular(reviewPath);
if (reviewThere && review === null) return notMeasured('review.md is not a readable regular file');
const text = review === null ? null : review.toString('utf8');
const sameRun = text !== null && scopeShas(text).start !== '' && scopeShas(text).start.toLowerCase() === String(since).toLowerCase();
const start = text === null ? '' : scopeShas(text).start;
const sameRun = start !== '' && start.toLowerCase() === String(since).toLowerCase();
if (!sameRun) {
// Round 1, on either path.
return { exit: 0, round: 1, round_base: null, previous: null, run, deleted: clean() };
@ -220,13 +221,19 @@ function bestMatch(p, current, taken) {
return best;
}
/** A security finding (the review-answers.mjs `isSecurityFinding` rule; not imported, that module imports this one). */
const isSecurityLike = (f) => getRule(f?.rule_key)?.category === 'security' || f?.engine === 'security-review';
/**
* A security finding: its catalogue category is `security`, or it came from
* the `security-review` engine. Never accepted, never left out as pre-existing.
* Defined here and re-exported by review-answers.mjs, which imports this module.
*/
export function isSecurityFinding(f) {
return getRule(f?.rule_key)?.category === 'security' || f?.engine === 'security-review';
}
/** The folded finding: the current one's place and text, the higher severity, a security origin never lost. */
function fold(p, c) {
const out = { ...c, severity: higher(sevOf(p), sevOf(c)), carried_from: p.id };
if (isSecurityLike(p) && !isSecurityLike(c)) {
if (isSecurityFinding(p) && !isSecurityFinding(c)) {
out.rule_key = p.rule_key;
if (p.engine !== undefined) out.engine = p.engine;
}

2
lib/validators/review-validator.mjs
View file

@ -57,7 +57,7 @@ function blockerIdsFromBody(body) {
}
/** Finding IDs named by `Source finding:` lines under `## Remediation Steps` step headings. */
function remediationStepSources(body) {
export function remediationStepSources(body) {
const start = body.search(/^##\s+Remediation Steps\b/m);
if (start === -1) return new Set();
const rest = body.slice(start).replace(/^##\s+Remediation Steps[^\n]*\n/, '');