refactor(simplify): one plan read in red proof, one list marker, one put-back rule [skip-docs]
/simplify over ed6699d..HEAD (closing review of the remediation): - red-proof reads and parses the step once (planStep) for the test list and tests_only; normTest goes, plan-schema exports normPlanPath, also used by plan-scope-sweep. - plan-schema builds LIST_ITEM and the 1.9 File: line from one LIST_MARKER; the below-1.9 File: regex ends in a lookahead instead of slicing back one. - the workflow script hoists tripletOf and puts back through dedupByTriplet. - coordinator-contract keeps the rank in its JSDoc only; engine-findings drops a redundant String(). - tests: reuse knownIssues, trekplan19Block, step1Fields, one red-proof cli helper with childEnv, one payload-pair builder; drop a test the decision-20 definition test covers. No behaviour change. Suite 2462 (2460/0/2). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
d7db590c26
commit
bcf3adbba7
SSH key fingerprint:
SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
11 changed files with 90 additions and 127 deletions
|
|
@ -373,9 +373,14 @@ const BUILD_FILE = /^(?!Profile$)(?:[A-Z][a-z]+|GNUmake)file$|^(?:make|rake|just
|
|||
// Where an entry's description starts, and the words that start a list item.
|
||||
const DESCRIPTION_DASH = /^(?:\u2014|\u2013|-|--)$/;
|
||||
const LIST_JOINER = /^(?:and|or|\+|&)$/i;
|
||||
const LIST_ITEM = /^\s*(?:[-*+]|\d+[.)])\s+/;
|
||||
// The list markers a plan line may open with; LIST_ITEM and the 1.9 `File:`
|
||||
// line are both built from it.
|
||||
const LIST_MARKER = String.raw`(?:[-*+]|\d+[.)])`;
|
||||
const LIST_ITEM = new RegExp(String.raw`^\s*${LIST_MARKER}\s+`);
|
||||
const lastSegment = (p) => p.replace(/\/$/, '').split('/').pop();
|
||||
const norm = (p) => p.replace(/\\/g, '/').replace(/^\.\//, '');
|
||||
/** A plan path as the readers hold it: `/` separators, no leading `./`. */
|
||||
export const normPlanPath = (p) => p.replace(/\\/g, '/').replace(/^\.\//, '');
|
||||
const norm = normPlanPath;
|
||||
// A backticked span that is a path, not a URL, an option or a call (`parse()`, `p.parse()`).
|
||||
const tickedPath = (s) => /[./\\]/.test(s) && !/^https?:/.test(s) && !s.startsWith('-') && !/^[\w$.]+\(.*\)$/.test(s);
|
||||
// A plain word without the brackets and punctuation around it, `\` read as `/`.
|
||||
|
|
@ -622,10 +627,10 @@ export function stepTestsOnly(body, version) {
|
|||
|
||||
// A line of the `Test first:` field that names its test files (6.3.1's
|
||||
// selector); global, so read it with matchAll only: one match per `File:`.
|
||||
const TEST_FIRST_FILE_LINE = /\bFile:\**[ \t]*`/gi;
|
||||
const TEST_FIRST_FILE_LINE = /\bFile:\**[ \t]*(?=`)/gi;
|
||||
// From plan_version 1.9: a line that opens with `File:` (after an optional
|
||||
// list marker of the set LIST_ITEM accepts, and `**`), backticked or not.
|
||||
const TEST_FIRST_FILE_LINE_19 = /^\s*(?:(?:[-*+]|\d+[.)])\s+)?(?:\*\*)?File:(?:\*\*)?/i;
|
||||
// list marker (LIST_MARKER) and `**`), backticked or not.
|
||||
const TEST_FIRST_FILE_LINE_19 = new RegExp(String.raw`^\s*(?:${LIST_MARKER}\s+)?(?:\*\*)?File:(?:\*\*)?`, 'i');
|
||||
|
||||
/**
|
||||
* plan_version 1.9: the rest of each `Test first:` `File:` line (after `File:`
|
||||
|
|
@ -692,7 +697,7 @@ export function testFirstFiles(fields, version) {
|
|||
}
|
||||
for (const line of String(fields.get('Test first').text ?? '').split('\n')) {
|
||||
for (const at of line.matchAll(TEST_FIRST_FILE_LINE)) {
|
||||
for (const s of leadingSpans(line.slice(at.index + at[0].length - 1))) {
|
||||
for (const s of leadingSpans(line.slice(at.index + at[0].length))) {
|
||||
if (!(tickedPath(s) || BUILD_FILE.test(s))) continue;
|
||||
const p = norm(s);
|
||||
if (!out.includes(p)) out.push(p);
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@
|
|||
// Reuses: SEVERITY_VALUES / RULE_KEYS / getRule (rule-catalogue.mjs),
|
||||
// computeFindingId (finding-id.mjs, triplet), validateFindings
|
||||
// (findings-schema.mjs). Triplet key format mirrors
|
||||
// scripts/bakeoff-armA-merge.mjs:33; raised_by provenance mirrors
|
||||
// scripts/bakeoff-armA-merge.mjs `dedupByTriplet`; raised_by provenance mirrors
|
||||
// lib/review/plan-review-dedup.mjs.
|
||||
|
||||
import { SEVERITY_VALUES, RULE_KEYS, getRule } from './rule-catalogue.mjs';
|
||||
|
|
@ -262,18 +262,10 @@ export function dedupByTriplet(findings) {
|
|||
}
|
||||
const out = [];
|
||||
for (const group of groups.values()) {
|
||||
// A brief-conflict mark is not carried over from a group member to the
|
||||
// survivor: when the survivor is unmarked, the group stays unmarked and is
|
||||
// judged at its full severity. The survivor is ranked by catalogue tier
|
||||
// first; for a catalogue key the tier always ties inside one triplet (the
|
||||
// rule_key is shared), so a reviewer-agent finding survives over an engine
|
||||
// finding (decision 13, operator 12.10; review bc7f4e23), then the higher
|
||||
// asserted severity among findings of one kind, then the conformance
|
||||
// reviewer, else the first-seen one stays. The survivor is written with the
|
||||
// highest severity any finding in its group asserted, so an engine's mark
|
||||
// never lowers what a reviewer agent asserted, whatever the payload order
|
||||
// (plan 2026-10-11-files-backticks-trial4, ADR "Pass 1 survivor order and
|
||||
// group severity").
|
||||
// Ranked as the JSDoc above says. A brief-conflict mark is not carried over
|
||||
// from a group member to the survivor: when the survivor is unmarked, the
|
||||
// group stays unmarked and is judged at its full severity. For a catalogue
|
||||
// key the tier always ties inside one triplet (the rule_key is shared).
|
||||
const tier = (f) => severityRank(getRule(f.rule_key)?.severity ?? f.severity);
|
||||
let survivor = group[0];
|
||||
for (const f of group.slice(1)) {
|
||||
|
|
|
|||
|
|
@ -90,7 +90,7 @@ export const IMPROVEMENT_CATEGORIES = Object.freeze(new Set(['simplification', '
|
|||
*/
|
||||
export function toReviewerPayload(input, opts = {}) {
|
||||
const errors = [];
|
||||
const spec = Object.hasOwn(ENGINES, String(input?.engine ?? '')) ? ENGINES[input.engine] : null;
|
||||
const spec = Object.hasOwn(ENGINES, input?.engine ?? '') ? ENGINES[input.engine] : null;
|
||||
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) {
|
||||
|
|
|
|||
|
|
@ -38,7 +38,7 @@
|
|||
import { readFileSync, existsSync, realpathSync } from 'node:fs';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { resolve, relative, dirname, isAbsolute, sep } from 'node:path';
|
||||
import { sliceSteps, parseStepFields, entryPaths, testFirstFiles, extractPlanVersion, usesFilesGrammar } from '../parsers/plan-schema.mjs';
|
||||
import { sliceSteps, parseStepFields, entryPaths, testFirstFiles, extractPlanVersion, usesFilesGrammar, normPlanPath as norm } from '../parsers/plan-schema.mjs';
|
||||
import { parseManifest } from '../parsers/manifest-yaml.mjs';
|
||||
import { isMain } from '../util/is-main.mjs';
|
||||
|
||||
|
|
@ -60,8 +60,6 @@ function pathsIn(text) {
|
|||
return out;
|
||||
}
|
||||
|
||||
const norm = p => p.replace(/\\/g, '/').replace(/^\.\//, '');
|
||||
|
||||
/** Every path the plan names: Files, Test first File, manifest expected_paths. */
|
||||
export function plannedPaths(planText) {
|
||||
const out = new Set();
|
||||
|
|
|
|||
|
|
@ -69,7 +69,7 @@ import { tmpdir } from 'node:os';
|
|||
import { dirname, isAbsolute, join, resolve } from 'node:path';
|
||||
import { DEFAULT_TIMEOUT_MS, allowedCommand, runCommand, screenCommand, stoppedBy } from './criteria-runner.mjs';
|
||||
import { commandEnv } from '../util/command-env.mjs';
|
||||
import { extractPlanVersion, parseStepFields, sliceSteps, stepTestsOnlyFromFields, testFirstFiles } from '../parsers/plan-schema.mjs';
|
||||
import { extractPlanVersion, normPlanPath, parseStepFields, sliceSteps, stepTestsOnlyFromFields, testFirstFiles } from '../parsers/plan-schema.mjs';
|
||||
import { isMain } from '../util/is-main.mjs';
|
||||
|
||||
// The one timeout, defined in criteria-runner.mjs (release review of 6.3.0, round 7).
|
||||
|
|
@ -129,32 +129,25 @@ const notRun = (reason, extra = {}) => ({ status: 'not-run', reason, ...extra })
|
|||
// made a true tests-only step look like it built something (release review of
|
||||
// 6.3.0, round 7). A path the step changed outside `Files:` is the
|
||||
// controller's scope finding (trekexecute Sub-step C, Hard rule 2), not this check's.
|
||||
function testsOnly(o) {
|
||||
if (!o.plan) return {};
|
||||
let text;
|
||||
try { text = readFileSync(o.plan, 'utf8'); }
|
||||
catch (e) { return { tests_only: null, tests_only_note: `cannot read the plan: ${e.message}` }; }
|
||||
const step = sliceSteps(text).find((s) => Number(s.n) === Number(o.step));
|
||||
if (!step) return { tests_only: null, tests_only_note: `no "### Step ${o.step}:" in the plan` };
|
||||
return { tests_only: stepTestsOnlyFromFields(parseStepFields(step.body), extractPlanVersion(text)).testsOnly };
|
||||
function testsOnly(step) {
|
||||
if (!step) return {};
|
||||
if (step.error) return { tests_only: null, tests_only_note: step.error };
|
||||
return { tests_only: stepTestsOnlyFromFields(step.fields, step.version).testsOnly };
|
||||
}
|
||||
|
||||
// With `--plan` and `--step`: the step's test files as its `Test first:` File
|
||||
// line names them (testFirstFiles, operator decision 20), read as `testsOnly`
|
||||
// reads the plan. null = the plan or the step could not be read.
|
||||
function planTests(o) {
|
||||
// With `--plan` and `--step`: the step's parsed fields and the plan's version,
|
||||
// read once for both the test list and `testsOnly`. null without `--plan`;
|
||||
// `{error}` when the plan or the step could not be read.
|
||||
function planStep(o) {
|
||||
if (!o.plan) return null;
|
||||
let text;
|
||||
try { text = readFileSync(o.plan, 'utf8'); }
|
||||
catch { return null; }
|
||||
catch (e) { return { error: `cannot read the plan: ${e.message}` }; }
|
||||
const step = sliceSteps(text).find((s) => Number(s.n) === Number(o.step));
|
||||
if (!step) return null;
|
||||
return testFirstFiles(parseStepFields(step.body), extractPlanVersion(text));
|
||||
if (!step) return { error: `no "### Step ${o.step}:" in the plan` };
|
||||
return { fields: parseStepFields(step.body), version: extractPlanVersion(text) };
|
||||
}
|
||||
|
||||
/** A test path as the plan's list holds it: `/` separators, no leading `./`. */
|
||||
const normTest = (p) => String(p).replace(/\\/g, '/').replace(/^\.\//, '');
|
||||
|
||||
// A program that could not start (a spawn error: not installed), or one this
|
||||
// run stopped (the timeout, output past the buffer, a signal), is not a test
|
||||
// verdict at all (criteria-runner `stoppedBy`). With no shell, an exit 126/127
|
||||
|
|
@ -219,15 +212,17 @@ export function proveRed(o) {
|
|||
if (!existsSync(given)) return notRun(`--cwd ${given} does not exist`);
|
||||
const cwd = repoRoot(given);
|
||||
if (!cwd) return notRun(`--cwd ${given} is not inside a git repository`);
|
||||
// The step's own test list (review cf241526): read from the plan when none is
|
||||
// given, and a given list that is another set is refused. An empty or
|
||||
// unreadable plan list changes nothing.
|
||||
const fromPlan = planTests(o);
|
||||
if (fromPlan && fromPlan.length > 0) {
|
||||
if (tests.length === 0) tests = [...fromPlan];
|
||||
// The step's own test list (review cf241526): the step's `Test first:` File
|
||||
// line as testFirstFiles reads it (operator decision 20), taken when none is
|
||||
// given; a given list that is another set is refused. An empty or unreadable
|
||||
// plan list changes nothing.
|
||||
const step = planStep(o);
|
||||
const fromPlan = step?.fields ? testFirstFiles(step.fields, step.version) : [];
|
||||
if (fromPlan.length > 0) {
|
||||
if (tests.length === 0) tests = fromPlan;
|
||||
else {
|
||||
const a = new Set(tests.map(normTest));
|
||||
const b = new Set(fromPlan.map(normTest));
|
||||
const a = new Set(tests.map((t) => normPlanPath(String(t))));
|
||||
const b = new Set(fromPlan);
|
||||
if (a.size !== b.size || [...a].some((t) => !b.has(t))) {
|
||||
return notRun(`tests-mismatch: --tests names ${tests.join(',')}; the plan's Test first File line names ${fromPlan.join(',')}`, { plan_tests: fromPlan });
|
||||
}
|
||||
|
|
@ -270,7 +265,7 @@ export function proveRed(o) {
|
|||
const baseBroken = unrunnable(base);
|
||||
if (baseBroken) return notRun(`at the start point: ${baseBroken}`, { base, result, linked });
|
||||
const out = { base_sha: baseSha, linked, base, result };
|
||||
if (base.exit === 0) return { status: 'rejected', reason: 'not-red-at-base', ...testsOnly(o), ...out };
|
||||
if (base.exit === 0) return { status: 'rejected', reason: 'not-red-at-base', ...testsOnly(step), ...out };
|
||||
if (o.redOnly) return { status: 'proven', reason: 'red at the start point (--red-only: the result was not run)', red_kind: redKind(base), base_sha: baseSha, linked, base };
|
||||
if (result.exit !== 0) return { status: 'rejected', reason: 'not-green-at-result', ...out };
|
||||
|
||||
|
|
|
|||
|
|
@ -52,6 +52,8 @@ const RULE_KEYS = Object.keys(RULE_TIERS);
|
|||
const sevRank = (s) => (SEVERITIES.includes(s) ? SEVERITIES.indexOf(s) : SEVERITIES.length);
|
||||
// The catalogue tier of a rule_key, or null when the key is missing or unknown.
|
||||
const tierOf = (key) => (typeof key === 'string' && Object.prototype.hasOwnProperty.call(RULE_TIERS, key) ? RULE_TIERS[key] : null);
|
||||
// The (file,line,rule_key) triplet a finding is deduped and matched on.
|
||||
const tripletOf = (f) => `${f.file}\u0000${f.line}\u0000${f.rule_key}`;
|
||||
|
||||
// Findings schema — mirrors lib/review/findings-schema.mjs (NW1). The enum on
|
||||
// rule_key enforces the catalogue at the StructuredOutput layer (stronger than
|
||||
|
|
@ -125,7 +127,7 @@ const VERDICT_SCHEMA = {
|
|||
function dedupByTriplet(findings) {
|
||||
const groups = new Map();
|
||||
for (const f of findings) {
|
||||
const key = `${f.file}\u0000${f.line}\u0000${f.rule_key}`;
|
||||
const key = tripletOf(f);
|
||||
if (!groups.has(key)) groups.set(key, []);
|
||||
groups.get(key).push(f);
|
||||
}
|
||||
|
|
@ -278,7 +280,6 @@ const returned = Array.isArray(coord?.findings) ? coord.findings : merged;
|
|||
// move or relabel of an engine finding no longer lowers the verdict.
|
||||
// The raises and put-backs are written into `returned`, the marks kept, so the
|
||||
// rendered review.md carries them.
|
||||
const tripletOf = (f) => `${f.file}\u0000${f.line}\u0000${f.rule_key}`;
|
||||
const counterparts = new Map(merged
|
||||
.filter((m) => m && typeof m === 'object')
|
||||
.map((m) => [tripletOf(m), m]));
|
||||
|
|
@ -312,15 +313,10 @@ for (const f of returned) {
|
|||
}
|
||||
// (c) the put-back (decision 23).
|
||||
const returnedTriplets = new Set(returned.filter((f) => f && typeof f === 'object').map(tripletOf));
|
||||
const putBack = new Map();
|
||||
for (const f of allFindings) {
|
||||
if (!f || typeof f !== 'object' || !f.engine) continue;
|
||||
const key = tripletOf(f);
|
||||
if (returnedTriplets.has(key)) continue;
|
||||
const kept = putBack.get(key);
|
||||
if (!kept || sevRank(f.severity) < sevRank(kept.severity)) putBack.set(key, f);
|
||||
}
|
||||
for (const f of putBack.values()) {
|
||||
// Engine findings only, so dedupByTriplet keeps the higher severity, else the first seen.
|
||||
const putBack = dedupByTriplet(allFindings.filter((f) =>
|
||||
f && typeof f === 'object' && f.engine && !returnedTriplets.has(tripletOf(f))));
|
||||
for (const f of putBack) {
|
||||
const { brief_lowered_from: _lowered, ...copy } = f;
|
||||
returned.push(copy);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -146,6 +146,14 @@ test('HANDOVER-CONTRACTS: the design-part line and invariant 5 say "1.8 or later
|
|||
assert.ok(!bullet.includes('its `Test first:` file,'), 'invariant 7 no longer says "its `Test first:` file"');
|
||||
});
|
||||
|
||||
const trekplan19Block = () => {
|
||||
const p = read('commands/trekplan.md');
|
||||
const h19 = p.indexOf('**plan_version 1.9 additions**');
|
||||
assert.ok(h19 >= 0, 'commands/trekplan.md has the "**plan_version 1.9 additions**" block');
|
||||
const end = p.indexOf('\n>\n> **', h19);
|
||||
return p.slice(h19, end < 0 ? undefined : end);
|
||||
};
|
||||
|
||||
test('/trekplan: the 1.8 additions heading says "1.8 or later", and a separate 1.9 additions block follows it', () => {
|
||||
const p = read('commands/trekplan.md');
|
||||
const h18 = p.indexOf('**plan_version 1.8 or later additions**');
|
||||
|
|
@ -153,9 +161,7 @@ test('/trekplan: the 1.8 additions heading says "1.8 or later", and a separate 1
|
|||
assert.ok(!p.includes('**plan_version 1.8 additions**'), 'the old 1.8-only heading is gone');
|
||||
const h19 = p.indexOf('**plan_version 1.9 additions**');
|
||||
assert.ok(h19 > h18, 'a separate "**plan_version 1.9 additions**" block stands after the 1.8 block');
|
||||
const end = p.indexOf('\n>\n> **', h19);
|
||||
const block = p.slice(h19, end < 0 ? undefined : end);
|
||||
assert.ok(block.includes(ANNOTATION), `the 1.9 block says "${ANNOTATION}"`);
|
||||
assert.ok(trekplan19Block().includes(ANNOTATION), `the 1.9 block says "${ANNOTATION}"`);
|
||||
});
|
||||
|
||||
// Step 12: the docs follow the 1.9 fixes. Operator decision 20 (12.10): the
|
||||
|
|
@ -181,14 +187,6 @@ const invariant7 = () => {
|
|||
return h4.slice(m7.index, m8 ? m7.index + m8.index : undefined);
|
||||
};
|
||||
|
||||
const trekplan19Block = () => {
|
||||
const p = read('commands/trekplan.md');
|
||||
const h19 = p.indexOf('**plan_version 1.9 additions**');
|
||||
assert.ok(h19 >= 0, 'commands/trekplan.md has the "**plan_version 1.9 additions**" block');
|
||||
const end = p.indexOf('\n>\n> **', h19);
|
||||
return p.slice(h19, end < 0 ? undefined : end);
|
||||
};
|
||||
|
||||
const template19Paragraph = () => {
|
||||
const t = read('templates/plan-template.md');
|
||||
const start = t.indexOf('From plan_version 1.9 the rule is the file first on every line');
|
||||
|
|
@ -275,27 +273,7 @@ test('CHANGELOG: parseDoors reads the Unreleased section\'s door lines, none bad
|
|||
`one door line names plan_version 1.9: ${JSON.stringify(d.entries.map((e) => e.text))}`);
|
||||
});
|
||||
|
||||
test('CHANGELOG: the Unreleased section\'s Known issues name the session split', () => {
|
||||
const { heading, body } = firstChangelogSection();
|
||||
assert.equal(heading, UNRELEASED);
|
||||
const lines = body.split('\n');
|
||||
const start = lines.findIndex((l) => /^### Known issues\s*$/.test(l));
|
||||
assert.ok(start >= 0, 'the Unreleased section has a "### Known issues" heading');
|
||||
let end = lines.length;
|
||||
for (let i = start + 1; i < lines.length; i++) {
|
||||
if (/^###? /.test(lines[i])) { end = i; break; }
|
||||
}
|
||||
const known = lines.slice(start + 1, end).join('\n');
|
||||
assert.match(known, /session split/, `Known issues name the session split: ${JSON.stringify(known)}`);
|
||||
});
|
||||
|
||||
// Step 14 of the remediation: the CHANGELOG records the review fixes. The door
|
||||
// line for the Test first files names operator decision 20 (the backticked paths
|
||||
// right after File:), the workflow put-back names operator decision 23, the
|
||||
// engine-provenance entry states the tier rank instead of "on a severity tie",
|
||||
// Known issues name the default path's verdict gate (operator decision 25), and
|
||||
// the intro's measurement names the remediation's Step 13 commit. Word
|
||||
// boundaries keep "operator decision 2026-10-12" from counting as decision 20.
|
||||
// The Unreleased section's `### Known issues` body, up to the next heading.
|
||||
const knownIssues = (body) => {
|
||||
const lines = body.split('\n');
|
||||
const start = lines.findIndex((l) => /^### Known issues\s*$/.test(l));
|
||||
|
|
@ -307,6 +285,20 @@ const knownIssues = (body) => {
|
|||
return lines.slice(start + 1, end).join('\n');
|
||||
};
|
||||
|
||||
test('CHANGELOG: the Unreleased section\'s Known issues name the session split', () => {
|
||||
const { heading, body } = firstChangelogSection();
|
||||
assert.equal(heading, UNRELEASED);
|
||||
const known = knownIssues(body);
|
||||
assert.match(known, /session split/, `Known issues name the session split: ${JSON.stringify(known)}`);
|
||||
});
|
||||
|
||||
// Step 14 of the remediation: the CHANGELOG records the review fixes. The door
|
||||
// line for the Test first files names operator decision 20 (the backticked paths
|
||||
// right after File:), the workflow put-back names operator decision 23, the
|
||||
// engine-provenance entry states the tier rank instead of "on a severity tie",
|
||||
// Known issues name the default path's verdict gate (operator decision 25), and
|
||||
// the intro's measurement names the remediation's Step 13 commit. Word
|
||||
// boundaries keep "operator decision 2026-10-12" from counting as decision 20.
|
||||
test('CHANGELOG: the door lines name operator decisions 20 and 23, and none says "every test file on the Test first File line"', () => {
|
||||
const { heading, body } = firstChangelogSection();
|
||||
assert.equal(heading, UNRELEASED);
|
||||
|
|
|
|||
|
|
@ -53,8 +53,9 @@ function addTest(dir) {
|
|||
writeFileSync(join(dir, 'tests', 'sum.test.mjs'), SUM_TEST);
|
||||
}
|
||||
|
||||
/** The CLI with the child environment the suite requires (tests/helpers/child-env.mjs). */
|
||||
function cli(args) {
|
||||
const r = spawnSync(process.execPath, [CLI, ...args], { encoding: 'utf8' });
|
||||
const r = spawnSync(process.execPath, [CLI, ...args], { encoding: 'utf8', env: childEnv() });
|
||||
return { status: r.status, stdout: r.stdout, stderr: r.stderr };
|
||||
}
|
||||
|
||||
|
|
@ -293,12 +294,6 @@ function planFile(dir, opts = {}, version = '1.8') {
|
|||
return p;
|
||||
}
|
||||
|
||||
/** The CLI with the child environment the suite requires (tests/helpers/child-env.mjs). */
|
||||
function cliPlan(args) {
|
||||
const r = spawnSync(process.execPath, [CLI, ...args], { encoding: 'utf8', env: childEnv() });
|
||||
return { status: r.status, stdout: r.stdout, stderr: r.stderr };
|
||||
}
|
||||
|
||||
test('red proof --plan/--step: a --tests that differs from the step\'s Test first File line is not-run, exit 2 (tests-mismatch)', () => {
|
||||
const { dir, base } = repo('a - b');
|
||||
try {
|
||||
|
|
@ -307,7 +302,7 @@ test('red proof --plan/--step: a --tests that differs from the step\'s Test firs
|
|||
const plan = planFile(dir);
|
||||
for (const given of ['tests/other.test.mjs', 'tests/sum.test.mjs,tests/other.test.mjs']) {
|
||||
for (const mode of [['--red-only'], []]) {
|
||||
const c = cliPlan([...mode, '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--tests', given, '--plan', plan, '--step', '1', '--json']);
|
||||
const c = cli([...mode, '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--tests', given, '--plan', plan, '--step', '1', '--json']);
|
||||
assert.equal(c.status, 2, `${given} ${mode.join(' ')}: ${c.stdout}${c.stderr}`);
|
||||
const out = JSON.parse(c.stdout);
|
||||
assert.equal(out.status, 'not-run', JSON.stringify(out));
|
||||
|
|
@ -328,17 +323,17 @@ test('red proof --plan/--step without --tests: the step\'s Test first file is th
|
|||
try {
|
||||
addTest(dir); // test written, nothing built yet: red at the start point
|
||||
const plan = planFile(dir);
|
||||
const red = cliPlan(['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--plan', plan, '--step', '1', '--json']);
|
||||
const red = cli(['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--plan', plan, '--step', '1', '--json']);
|
||||
assert.equal(red.status, 0, red.stdout + red.stderr);
|
||||
assert.equal(JSON.parse(red.stdout).status, 'proven');
|
||||
// Built: the full run proves red, green and the control from the plan's list alone.
|
||||
writeFileSync(join(dir, 'lib', 'sum.mjs'), 'export function sum(a, b) { return a + b; }\n');
|
||||
const full = cliPlan(['--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--plan', plan, '--step', '1', '--json']);
|
||||
const full = cli(['--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--plan', plan, '--step', '1', '--json']);
|
||||
assert.equal(full.status, 0, full.stdout + full.stderr);
|
||||
assert.equal(JSON.parse(full.stdout).status, 'proven');
|
||||
// A step whose plan list is empty still needs --tests.
|
||||
const none = planFile(dir, { testFirst: false }, '1.7');
|
||||
const empty = cliPlan(['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--plan', none, '--step', '1', '--json']);
|
||||
const empty = cli(['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--plan', none, '--step', '1', '--json']);
|
||||
assert.equal(empty.status, 2, empty.stdout + empty.stderr);
|
||||
assert.match(JSON.parse(empty.stdout).reason, /no test files given/);
|
||||
} finally { rmSync(dir, { recursive: true, force: true }); }
|
||||
|
|
@ -350,8 +345,8 @@ test('red proof --plan/--step: a --tests equal to the plan\'s list (normalised)
|
|||
addTest(dir);
|
||||
const plan = planFile(dir);
|
||||
for (const given of ['tests/sum.test.mjs', './tests/sum.test.mjs']) {
|
||||
const before = cliPlan(['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--tests', given, '--json']);
|
||||
const withPlan = cliPlan(['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--tests', given, '--plan', plan, '--step', '1', '--json']);
|
||||
const before = cli(['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--tests', given, '--json']);
|
||||
const withPlan = cli(['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--tests', given, '--plan', plan, '--step', '1', '--json']);
|
||||
assert.equal(withPlan.status, 0, withPlan.stdout + withPlan.stderr);
|
||||
assert.equal(withPlan.status, before.status);
|
||||
const a = JSON.parse(withPlan.stdout);
|
||||
|
|
@ -371,10 +366,10 @@ test('red proof --plan/--step: a 1.8 File line naming a source in its descriptio
|
|||
// point and the start point would go green (not-red-at-base).
|
||||
writeFileSync(join(dir, 'lib', 'sum.mjs'), 'export function sum(a, b) { return a + b; }\n');
|
||||
const plan = planFile(dir, { testFirstRaw: ' - File: `tests/sum.test.mjs` (existing) - covers `lib/sum.mjs`' });
|
||||
const derived = cliPlan(['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--plan', plan, '--step', '1', '--json']);
|
||||
const derived = cli(['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--plan', plan, '--step', '1', '--json']);
|
||||
assert.equal(derived.status, 0, derived.stdout + derived.stderr);
|
||||
assert.equal(JSON.parse(derived.stdout).status, 'proven');
|
||||
const given = cliPlan(['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--tests', 'tests/sum.test.mjs', '--plan', plan, '--step', '1', '--json']);
|
||||
const given = cli(['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--tests', 'tests/sum.test.mjs', '--plan', plan, '--step', '1', '--json']);
|
||||
assert.equal(given.status, 0, given.stdout + given.stderr);
|
||||
assert.doesNotMatch(JSON.parse(given.stdout).reason, /tests-mismatch/);
|
||||
} finally { rmSync(dir, { recursive: true, force: true }); }
|
||||
|
|
@ -390,12 +385,12 @@ test('red proof --plan/--step: an empty plan list (1.7 without Test first, 1.8 F
|
|||
plans.push(readFileSync(planFile(dir, { testFirst: false }, '1.7'), 'utf8'));
|
||||
plans.push(readFileSync(planFile(dir, { testFirstRaw: ' - File: see `tests/sum.test.mjs` for the test' }, '1.8'), 'utf8'));
|
||||
const args = ['--red-only', '--cwd', dir, '--base', base, '--cmd', TEST_CMD, '--tests', 'tests/other.test.mjs', '--json'];
|
||||
const before = cliPlan(args);
|
||||
const before = cli(args);
|
||||
const b = JSON.parse(before.stdout);
|
||||
for (const content of plans) {
|
||||
const p = join(dir, 'plan.local.md');
|
||||
writeFileSync(p, content);
|
||||
const c = cliPlan([...args, '--plan', p, '--step', '1']);
|
||||
const c = cli([...args, '--plan', p, '--step', '1']);
|
||||
assert.equal(c.status, before.status, c.stdout + c.stderr);
|
||||
const out = JSON.parse(c.stdout);
|
||||
assert.doesNotMatch(out.reason, /tests-mismatch/, JSON.stringify(out));
|
||||
|
|
|
|||
|
|
@ -75,14 +75,6 @@ test('a code file beside the test on the File: line still makes the step not tes
|
|||
assert.equal(m.stepTestsOnly(body).testsOnly, false);
|
||||
});
|
||||
|
||||
test('prose: the executor and the step helper name every file on the Test first File: line', () => {
|
||||
const phrase = 'every file on the `Test first:` `File:` line';
|
||||
for (const rel of [['commands', 'trekexecute.md'], ['agents', 'step-implementer.md']]) {
|
||||
const text = readFileSync(join(ROOT, ...rel), 'utf8');
|
||||
assert.ok(text.includes(phrase), `${rel.join('/')} says "${phrase}"`);
|
||||
}
|
||||
});
|
||||
|
||||
// ---- plan_version 1.9 (plan step 5) ----------------------------------------
|
||||
// From 1.9 a `Test first:` `File:` line is every line that opens with `File:`
|
||||
// (after an optional list marker and `**`), backticked or not, and its rest is
|
||||
|
|
|
|||
|
|
@ -498,12 +498,16 @@ test('decision 13 — an engine payload decides the engine: a /security-review p
|
|||
assertAllOutputLegal(result);
|
||||
});
|
||||
|
||||
// A reviewer MISSING_TEST and a marked /code-review MAJOR on the same triplet, in `order`.
|
||||
function reviewerAndMarkedEngine(order, reviewerExtra = {}) {
|
||||
const reviewerPayload = { reviewer: 'code-correctness-reviewer', findings: [reviewerCopy({ rule_key: 'MISSING_TEST', ...reviewerExtra })] };
|
||||
const enginePayload = { reviewer: 'claude-code-review', findings: [codeReviewDefect({ rule_key: 'MISSING_TEST', contradicts_brief: NON_GOAL })] };
|
||||
return order === 'reviewer first' ? [reviewerPayload, enginePayload] : [enginePayload, reviewerPayload];
|
||||
}
|
||||
|
||||
for (const order of ['reviewer first', 'engine first']) {
|
||||
test(`decision 13 — a tie between a marked /code-review MAJOR and a reviewer MAJOR keeps the reviewer (${order}) → WARN`, () => {
|
||||
const reviewerPayload = { reviewer: 'code-correctness-reviewer', findings: [reviewerCopy({ rule_key: 'MISSING_TEST' })] };
|
||||
const enginePayload = { reviewer: 'claude-code-review', findings: [codeReviewDefect({ rule_key: 'MISSING_TEST', contradicts_brief: NON_GOAL })] };
|
||||
const payloads = order === 'reviewer first' ? [reviewerPayload, enginePayload] : [enginePayload, reviewerPayload];
|
||||
const result = m.runContract(payloads);
|
||||
const result = m.runContract(reviewerAndMarkedEngine(order));
|
||||
assert.equal(result.findings.length, 1);
|
||||
const out = result.findings[0];
|
||||
assert.equal(out.severity, 'MAJOR', 'the reviewer agent asserted MAJOR; the engine mark cannot lower it');
|
||||
|
|
@ -516,15 +520,9 @@ for (const order of ['reviewer first', 'engine first']) {
|
|||
});
|
||||
}
|
||||
|
||||
function minorReviewerAndMarkedEngine(order) {
|
||||
const reviewerPayload = { reviewer: 'code-correctness-reviewer', findings: [reviewerCopy({ rule_key: 'MISSING_TEST', severity: 'MINOR' })] };
|
||||
const enginePayload = { reviewer: 'claude-code-review', findings: [codeReviewDefect({ rule_key: 'MISSING_TEST', contradicts_brief: NON_GOAL })] };
|
||||
return order === 'reviewer first' ? [reviewerPayload, enginePayload] : [enginePayload, reviewerPayload];
|
||||
}
|
||||
|
||||
for (const order of ['reviewer first', 'engine first']) {
|
||||
test(`bc7f4e23 — a marked /code-review MAJOR over a reviewer MINOR on the same triplet keeps the reviewer at MAJOR (${order}) → WARN`, () => {
|
||||
const result = m.runContract(minorReviewerAndMarkedEngine(order));
|
||||
const result = m.runContract(reviewerAndMarkedEngine(order, { severity: 'MINOR' }));
|
||||
assert.equal(result.findings.length, 1);
|
||||
const out = result.findings[0];
|
||||
assert.equal('engine' in out, false, 'the survivor is the reviewer finding');
|
||||
|
|
@ -536,7 +534,7 @@ for (const order of ['reviewer first', 'engine first']) {
|
|||
});
|
||||
|
||||
test(`bc7f4e23 — dedupByTriplet(ingest(...)) keeps the reviewer finding at the group's MAJOR over a marked /code-review MAJOR (${order})`, () => {
|
||||
const { findings } = m.ingest(minorReviewerAndMarkedEngine(order));
|
||||
const { findings } = m.ingest(reviewerAndMarkedEngine(order, { severity: 'MINOR' }));
|
||||
const out = m.dedupByTriplet(findings);
|
||||
assert.equal(out.length, 1);
|
||||
assert.equal('engine' in out[0], false, 'the reviewer-agent finding survives the tier tie');
|
||||
|
|
|
|||
|
|
@ -234,6 +234,7 @@ test('1.9: the red proof\'s tests_only reads Files by the grammar (CLI, temp git
|
|||
// no test. Below 1.9 the step check is read as before (decision 16).
|
||||
const planTestFirst19 = (version, line) =>
|
||||
buildPlan({ version, steps: [{ testFirstRaw: line }, { endToEnd: 'bash tests/e2e.sh' }] });
|
||||
const step1Fields = (text) => schema.parseStepFields(text.split('### Step 1:')[1].split('### Step 2:')[0]);
|
||||
|
||||
test('1.9: a Test first whose File: stands mid-line names no test - PLAN_STEP_NO_TEST_FIRST under --strict, a warning under --soft', () => {
|
||||
for (const line of [
|
||||
|
|
@ -242,7 +243,7 @@ test('1.9: a Test first whose File: stands mid-line names no test - PLAN_STEP_NO
|
|||
]) {
|
||||
const text = planTestFirst19('1.9', line);
|
||||
assert.ok(text.includes(`${line}\n`), 'the builder writes the raw File line');
|
||||
assert.deepEqual(schema.testFirstFiles(schema.parseStepFields(text.split('### Step 1:')[1].split('### Step 2:')[0]), '1.9'), [],
|
||||
assert.deepEqual(schema.testFirstFiles(step1Fields(text), '1.9'), [],
|
||||
'the executor reads no test file here');
|
||||
const r = validatePlanContent(text, { strict: true });
|
||||
assert.equal(r.valid, false, `${line}: ${errCodes(r).join(',')}`);
|
||||
|
|
@ -274,7 +275,6 @@ test('below 1.9: the step check reads a mid-line File: as before', () => {
|
|||
// `File:` line names its test (reviews b4fa1927, 4b6460e5) ----
|
||||
|
||||
const WRAPPED = '`tests/a.test.mjs`,\n `src/a.mjs`';
|
||||
const step1Fields = (text) => schema.parseStepFields(text.split('### Step 1:')[1].split('### Step 2:')[0]);
|
||||
|
||||
test('1.9: a Files list wrapped after a comma names both files and no issue', () => {
|
||||
const r = entryPaths(WRAPPED);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue