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

fix(plan): the Files reader gives 6.3.0's answer in the three known regressions

Browse source 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-12 14:21:56 +02:00
commit 6f342a2cdc
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
2 changed files with 103 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

38
lib/parsers/plan-schema.mjs
View file

@ -332,6 +332,25 @@ export function onFailureAction(text) {
*
* A backslash is read as a slash, a leading `./` is dropped. Release review of
* 6.3.0, rounds 1-10.
*
* Three answers 6.3.0 gave and 6.3.1 lost are given back, each in its narrow
* form, and the round 7-10 neighbours of each keep 6.3.1's answer:
*
* - 6.4 / regression A: inside a description, a comma entry that opens with a
* plain file with a slash, or with one followed by a colon or a dash, starts
* a new entry ("`a.test.mjs` - new test, src/a.mjs - the fix" names both).
* That is the new-line test without its "alone on the line" branch
* (opensChunk), so "— mocks fs, child_process.spawnSync and path.join" stays
* description (round 9).
* - 6.4 / regression B: in an entry opened by a backticked path, a plain path
* with a slash after a word of description ("`a.test.mjs` covers the parser
* in src/parse.mjs") counts only where a list starts (after "and", "or", "+"
* or "&"). Right after the backticked path ("`a.test.mjs` src/b.mjs"), and in
* an entry not opened by one (", the CLI entry src/cli.mjs", round 7), it is
* read as before.
* - 6.4 / regression C: a new line that is no list item and holds only a
* backticked capitalised name ("`LICENSE`") ends the description, and the
* name is a file. On the description's own line it stays no file (round 10).
*/
const SEG = '[\\w@.*?()-]';
const PATH_WORD = new RegExp(`^(?:${SEG}+\\/)+${SEG}*$|^[\\w@*?-][\\w@.*?-]*\\.[A-Za-z*][A-Za-z0-9*]*$|^\\.[\\w-][\\w.-]*$`);
@ -365,6 +384,13 @@ const opensEntry = (tokens) => {
const w = plainWord(tokens[0]);
return isFileWord(w) && (w.includes('/') || tokens.length === 1 || tokens[0].endsWith(':') || DESCRIPTION_DASH.test(tokens[1]));
};
// A comma entry inside a description that opens a new entry: opensEntry's test
// without "alone on the line", so "path.join" after a comma stays description
// (6.4 / regression A).
const opensChunk = (tokens) => {
const w = plainWord(tokens[0]);
return isFileWord(w) && (w.includes('/') || tokens[0].endsWith(':') || DESCRIPTION_DASH.test(tokens[1]));
};
export function fieldPaths(text) {
const spans = [];
@ -382,12 +408,21 @@ export function fieldPaths(text) {
const item = LIST_ITEM.exec(line);
const body = item ? line.slice(item[0].length) : line;
if (item || (described && body.trim() !== '' && opensEntry(body.trim().split(/\s+/)))) described = false;
// A new line that holds only a backticked capitalised name ends the
// description (6.4 / regression C).
const alone = /^\u0000(\d+)\u0000$/.exec(body.trim());
if (alone && BARE_NAME.test(spans[Number(alone[1])])) described = false;
for (const raw of body.split(',')) {
const entry = raw.trim().replace(/^[-*]\s+/, '');
if (entry === '') continue;
const tokens = entry.split(/\s+/);
const first = /^\u0000(\d+)\u0000/.exec(entry);
if (first && isTicked[Number(first[1])]) described = false;
if (described && opensChunk(tokens)) described = false;
// An entry opened by a backticked path: after a word of description a
// plain path with a slash counts only where a list starts (6.4 / regression B).
const tickedEntry = Boolean(first && isTicked[Number(first[1])]);
let prose = false;
// A backticked capitalised name alone in its entry, before any
// description, is a file (rounds 6, 10).
if (first && first[0] === entry && !described && BARE_NAME.test(spans[Number(first[1])])) {
@ -410,8 +445,9 @@ export function fieldPaths(text) {
}
if (DESCRIPTION_DASH.test(t)) described = true;
const w = plainWord(t);
if (!isFileWord(w)) prose = true;
if (described || !isFileWord(w)) continue;
if (w.includes('/') || i === 0 || LIST_JOINER.test(tokens[i - 1])) out.push(w);
if ((w.includes('/') && !(tickedEntry && prose)) || i === 0 || LIST_JOINER.test(tokens[i - 1])) out.push(w);
if (t.endsWith(':')) described = true;
}
continue;

66
tests/lib/files-reader-regressions.test.mjs Normal file
View file

@ -0,0 +1,66 @@
// tests/lib/files-reader-regressions.test.mjs
// 6.4 / regressions A, B and C of the `Files:` reader (fieldPaths in
// lib/parsers/plan-schema.mjs): three inputs 6.3.0 read right and 6.3.1 read
// wrong. Each case goes through stepTestsOnly on a 1.8-shaped step and through
// the validator's PLAN_STEP_TESTS_ONLY on a 1.8 plan. The controls are the
// round 7-10 neighbours of each case and the plain path right after a
// backticked one: they pin 6.3.1's answers and stay as they are.
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { buildPlan } from '../helpers/plan-v18-builder.mjs';
import { validatePlanContent } from '../../lib/validators/plan-validator.mjs';
import { stepTestsOnly } from '../../lib/parsers/plan-schema.mjs';
const step = (files) => `- **Files:** ${files}\n- **Changes:** x\n`;
// A 1.8 plan whose step 2 carries `files` as its `Files:` field.
function planWith(files) {
const plan = buildPlan({ steps: [{}, { file: 'src/mod2.mjs', endToEnd: 'bash tests/e2e.sh' }] });
const line = '- **Files:** `src/mod2.mjs`';
assert.ok(plan.includes(line), 'the builder writes step 2\'s Files: line');
return plan.replace(line, `- **Files:** ${files}`);
}
const testsOnlyWarned = (files) => validatePlanContent(planWith(files)).warnings
.some((w) => w.code === 'PLAN_STEP_TESTS_ONLY' && /Step 2\b/.test(w.message));
// [case, Files: text, 6.3.0's paths]
const CASES = [
['A: a comma chunk that opens with a plain file ends the description',
'`tests/a.test.mjs` - new test, src/a.mjs - the fix', ['tests/a.test.mjs', 'src/a.mjs']],
['B: a plain slash path inside a description, with no comma, is no file',
'`tests/a.test.mjs` covers the parser in src/parse.mjs', ['tests/a.test.mjs']],
['C: a backticked bare name alone on a new line is a file',
'`tests/a.test.mjs` - new test\n `LICENSE`', ['tests/a.test.mjs', 'LICENSE']],
];
for (const [name, files, paths] of CASES) {
test(`6.4 / regression ${name}`, () => {
const testsOnly = paths.every((p) => p.startsWith('tests/'));
assert.deepEqual(stepTestsOnly(step(files)), { testsOnly, files: paths }, files);
assert.equal(testsOnlyWarned(files), testsOnly,
`PLAN_STEP_TESTS_ONLY ${testsOnly ? 'fires' : 'stays quiet'} on a 1.8 plan: ${files}`);
});
}
// [control, Files: text, 6.3.1's paths]
const CONTROLS = [
['r7 (comma form of B): a plain slash path in its own comma entry is read',
'`tests/x.test.mjs`, the CLI entry src/cli.mjs', ['tests/x.test.mjs', 'src/cli.mjs']],
['r9 (A\'s neighbour): a description runs on past a comma',
'`tests/a.test.mjs` — mocks fs, child_process.spawnSync and path.join', ['tests/a.test.mjs']],
['r10 (C\'s neighbour): a backticked capitalised name inside a description is no file',
'`tests/a.test.mjs` — covers `parse`, `Parser`, `ParseError`', ['tests/a.test.mjs']],
['B\'s neighbour: a plain path right after the backticked one is read',
'`tests/a.test.mjs` src/b.mjs', ['tests/a.test.mjs', 'src/b.mjs']],
];
for (const [name, files, paths] of CONTROLS) {
test(`6.4 / control ${name}`, () => {
const testsOnly = paths.every((p) => p.startsWith('tests/'));
assert.deepEqual(stepTestsOnly(step(files)), { testsOnly, files: paths }, files);
assert.equal(testsOnlyWarned(files), testsOnly,
`PLAN_STEP_TESTS_ONLY ${testsOnly ? 'fires' : 'stays quiet'} on a 1.8 plan: ${files}`);
});
}