fix(plan): a 1.9 wrapped line keeps its paths only after a comma or in an empty entry

Operator decision 29 (14.10); review findings d21ff81e and eaeb398e.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-14 10:29:35 +02:00
commit b70f242e53
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
2 changed files with 115 additions and 21 deletions

View file

@ -477,12 +477,15 @@ export function fieldPaths(text) {
// //
// - The entries are the field's first non-empty line and each list-item line // - The entries are the field's first non-empty line and each list-item line
// (`-`, `*`, `+`, `1.`, `1)`). Any other non-empty line continues the entry // (`-`, `*`, `+`, `1.`, `1)`). Any other non-empty line continues the entry
// above it and is appended to that entry's text with one space before the // above it: when the entry so far ends with a comma it is appended with one
// entry is read, so a list wrapped after a comma ("`tests/a.test.mjs`," and // space before the entry is read, so a list wrapped after a comma
// "`src/a.mjs`" on the next line) names both files, a wrapped description // ("`tests/a.test.mjs`," and "`src/a.mjs`" on the next line) names both
// line stays description, and an empty first line before a nested list is // files; otherwise it and every later wrapped line of that entry are
// no entry. An issue carries the line number of the entry's first line // description and never read (operator decision 29, 14.10; reviews
// (review `b4fa1927`). // `d21ff81e`, `eaeb398e`). An entry with nothing on its own line (a `File:`
// line with nothing after it) takes its first wrapped line. An empty first
// line before a nested list is no entry. An issue carries the line number
// of the entry's first line (review `b4fa1927`).
// - An entry's files are the backticked spans it opens with, separated only by // - An entry's files are the backticked spans it opens with, separated only by
// commas or whitespace. Everything from the first other text on is // commas or whitespace. Everything from the first other text on is
// description and is never read ("`a.test.mjs` - covers `src/parse.mjs`" // description and is never read ("`a.test.mjs` - covers `src/parse.mjs`"
@ -494,9 +497,14 @@ export function fieldPaths(text) {
// //
// - A `Test first:` `File:` line is a line that opens with `File:` (or // - A `Test first:` `File:` line is a line that opens with `File:` (or
// `**File:**`) after an optional list marker of the same set (`-`, `*`, `+`, // `**File:**`) after an optional list marker of the same set (`-`, `*`, `+`,
// `1.`, `1)`); its rest is one entry under this grammar. The non-list lines // `1.`, `1)`); its rest is one entry under this grammar. Any other
// right after it are appended to it with one space, as above; any list item // non-empty line right after it continues it by the same rule: when the
// (a `Verifies:` or `Pattern:` line too) ends it, and a non-list line after // entry so far ends with a comma it is appended with one space, so a list
// wrapped after a comma names both files; otherwise it and every later
// wrapped line of that entry are description and never read (operator
// decision 29, 14.10; reviews `d21ff81e`, `eaeb398e`). A `File:` line with
// nothing after it takes its first wrapped line. Any list item (a
// `Verifies:` or `Pattern:` line too) ends it, and a non-list line after
// such an item belongs to that item, never to the `File:` entry. A line that // such an item belongs to that item, never to the `File:` entry. A line that
// opens with `File:` always starts a new entry; a `File:` mid-line names no // opens with `File:` always starts a new entry; a `File:` mid-line names no
// test (review `4b6460e5`). // test (review `4b6460e5`).
@ -540,6 +548,26 @@ function leadingSpans(text) {
return out; return out;
} }
/**
* Whether a wrapped line (a non-empty line that is no list item) joins the
* 1.9 entry above it (operator decision 29, 14.10; reviews `d21ff81e`,
* `eaeb398e`). An entry with nothing on its own line (a `File:` line with
* nothing after it) takes its first wrapped line; an entry whose text so far
* ends with a comma gets the line appended with one space, so a list wrapped
* after a comma names both files. Otherwise the line is description: null,
* and the caller closes the entry, so every later wrapped line of it is
* description too.
* @param {string} text - the entry so far
* @param {string} line - the wrapped line
* @returns {string|null} the entry's new text, or null when the line is description
*/
function wrapLine(text, line) {
const so = String(text ?? '');
if (so.trim() === '') return line.trim();
if (so.trimEnd().endsWith(',')) return `${so} ${line.trim()}`.trim();
return null;
}
/** /**
* The files a 1.9 `Files:` field names, by the grammar above. * The files a 1.9 `Files:` field names, by the grammar above.
* @param {string} text - the field's text (parseStepFields(body).get('Files').text) * @param {string} text - the field's text (parseStepFields(body).get('Files').text)
@ -554,13 +582,19 @@ export function entryPaths(text) {
String(text ?? '').split('\n').forEach((line, i) => { String(text ?? '').split('\n').forEach((line, i) => {
const item = LIST_ITEM.exec(line); const item = LIST_ITEM.exec(line);
const rest = line.trim(); const rest = line.trim();
if (item) entries.push({ line: i + 1, entry: line.slice(item[0].length).trim() }); if (item) entries.push({ line: i + 1, entry: line.slice(item[0].length).trim(), closed: false });
else if (rest === '') return; else if (rest === '') return;
else if (entries.length === 0) entries.push({ line: i + 1, entry: rest }); else if (entries.length === 0) entries.push({ line: i + 1, entry: rest, closed: false });
else { else {
// A wrapped continuation line belongs to the entry above it. // A wrapped line joins the entry above it only after a comma, or when
// the entry is empty (wrapLine); otherwise it closes the entry, and it
// and every later wrapped line of that entry are description, never read
// (operator decision 29).
const last = entries[entries.length - 1]; const last = entries[entries.length - 1];
last.entry = `${last.entry} ${rest}`.trim(); if (last.closed) return;
const joined = wrapLine(last.entry, rest);
if (joined === null) last.closed = true;
else last.entry = joined;
} }
}); });
if (entries.length === 0) { if (entries.length === 0) {
@ -631,10 +665,15 @@ const TEST_FIRST_FILE_LINE_19 = /^\s*(?:(?:[-*+]|\d+[.)])\s+)?(?:\*\*)?File:(?:\
/** /**
* plan_version 1.9: the rest of each `Test first:` `File:` line (after `File:` * plan_version 1.9: the rest of each `Test first:` `File:` line (after `File:`
* and any `**`), with the non-list lines right after it appended (one space * and any `**`), one entry per `File:` line, in order. Any other non-empty
* each), one entry per `File:` line, in order. Any list item ends the entry; * line right after it continues the entry above it: when the entry so far
* a line that opens with `File:` starts a new one. The one selector both * ends with a comma it is appended with one space, so a list wrapped after a
* testFirstFiles and the plan validator's grammar check read. * comma names both files; otherwise it and every later wrapped line of that
* entry are description and never read (operator decision 29, 14.10; reviews
* `d21ff81e`, `eaeb398e`). An entry with nothing on its own line (a `File:`
* line with nothing after it) takes its first wrapped line. Any list item ends
* the entry; a line that opens with `File:` starts a new one. The one
* selector both testFirstFiles and the plan validator's grammar check read.
* @param {Map<string, {text: string}>} fields - parseStepFields(body) * @param {Map<string, {text: string}>} fields - parseStepFields(body)
* @returns {string[]} * @returns {string[]}
*/ */
@ -651,7 +690,9 @@ export function testFirstFileEntries(fields) {
} else if (LIST_ITEM.test(line) || line.trim() === '') { } else if (LIST_ITEM.test(line) || line.trim() === '') {
open = false; open = false;
} else if (open) { } else if (open) {
out[out.length - 1] = `${out[out.length - 1]} ${line.trim()}`.trim(); const joined = wrapLine(out[out.length - 1], line);
if (joined === null) open = false;
else out[out.length - 1] = joined;
} }
} }
return out; return out;
@ -673,9 +714,13 @@ export function testFirstFileEntries(fields) {
* *
* From 1.9 a line counts when it opens with `File:` (testFirstFileEntries), * From 1.9 a line counts when it opens with `File:` (testFirstFileEntries),
* backticked or not, after an optional list marker (`1.` and `1)` included); * backticked or not, after an optional list marker (`1.` and `1)` included);
* its rest, with its wrapped continuation lines, is one entry read by * its rest is one entry read by entryPaths: the leading backticked paths are
* entryPaths: the leading backticked paths are the test files, the rest is * the test files, the rest is description. Any other non-empty line right
* description. * after it continues the entry: when the entry so far ends with a comma it is
* appended with one space, so a list wrapped after a comma names both files;
* otherwise it and every later wrapped line of that entry are description and
* never read (operator decision 29, 14.10; reviews `d21ff81e`, `eaeb398e`). A
* `File:` line with nothing after it takes its first wrapped line.
* *
* Normalised, in order, deduplicated. A `Verifies:` line without a `File:` is * Normalised, in order, deduplicated. A `Verifies:` line without a `File:` is
* never read. * never read.

View file

@ -339,6 +339,55 @@ test('1.9: two unmarked File lines in a row give both tests', () => {
assert.deepEqual(schema.testFirstFiles(step1Fields(text), '1.9'), ['tests/mod1.test.mjs', 'tests/mod1b.test.mjs']); assert.deepEqual(schema.testFirstFiles(step1Fields(text), '1.9'), ['tests/mod1.test.mjs', 'tests/mod1b.test.mjs']);
}); });
// ---- Step 2 (second remediation): a 1.9 wrapped line keeps its paths only
// after a comma, or in an entry with nothing on its own line (operator
// decision 29, 14.10; reviews d21ff81e, eaeb398e) ----
test('1.9: a line wrapped onto a backticked call after an entry with no comma is description - no issue', () => {
const r = entryPaths('`tests/a.test.mjs`\n `parse()` reads the input');
assert.deepEqual(r.files, ['tests/a.test.mjs']);
assert.deepEqual(r.issues, []);
});
test('1.9: a line wrapped onto a backticked option after an entry with no comma is description - no issue, and the validator agrees', () => {
const pair = '`src/a.mjs`\n `--strict` makes it an error';
const r = entryPaths(pair);
assert.deepEqual(r.files, ['src/a.mjs']);
assert.deepEqual(r.issues, []);
const v = validatePlanContent(plan19(pair), { strict: true });
assert.deepEqual(pathIssues(v.errors), [], JSON.stringify(v.errors));
});
test('1.9: once a wrapped line is description, a comma at its end does not open the entry again', () => {
const r = entryPaths('`src/a.mjs`\n `src/b.mjs`,\n `src/c.mjs`');
assert.deepEqual(r.files, ['src/a.mjs']);
assert.deepEqual(r.issues, []);
});
test('1.9: a File line wrapped onto a backticked source path gives only the test, and the step is not tests-only', () => {
const file = ' - File: `tests/mod1.test.mjs`\n `src/mod1.mjs` is the parser under test';
const text = planTestFirst19('1.9', file);
assert.ok(text.includes(`${file}\n`), 'the builder writes the raw File line');
assert.deepEqual(schema.testFirstFiles(step1Fields(text), '1.9'), ['tests/mod1.test.mjs']);
const body = `- **Files:** \`tests/mod1.test.mjs\`, \`src/mod1.mjs\`\n- **Changes:** x\n- **Test first:**\n${file}\n - Verifies: x\n`;
assert.deepEqual(schema.stepTestsOnly(body, '1.9'), { testsOnly: false, files: ['tests/mod1.test.mjs', 'src/mod1.mjs'] });
});
test('1.9: a File line wrapped onto a backticked call gives no PLAN_STEP_PATH_NOT_A_PATH under --strict', () => {
const text = planTestFirst19('1.9', ' - File: `tests/mod1.test.mjs`\n `parse()` reads the input');
const r = validatePlanContent(text, { strict: true });
assert.equal(r.errors.find((e) => e.code === 'PLAN_STEP_PATH_NOT_A_PATH'), undefined, JSON.stringify(r.errors));
assert.deepEqual(schema.testFirstFiles(step1Fields(text), '1.9'), ['tests/mod1.test.mjs']);
});
test('1.9 controls: an empty File line takes its first wrapped line, a comma keeps the list, a comma after no path still refuses', () => {
const empty = planTestFirst19('1.9', ' - File:\n `tests/mod1.test.mjs` (new)');
assert.deepEqual(schema.testFirstFiles(step1Fields(empty), '1.9'), ['tests/mod1.test.mjs']);
assert.deepEqual(entryPaths(WRAPPED).files, ['tests/a.test.mjs', 'src/a.mjs']);
const bad = entryPaths('- `src/a.mjs`\n- the CLI,\n `src/b.mjs`');
assert.deepEqual(bad.issues.map((i) => [i.code, i.line]), [['PLAN_STEP_PATH_NOT_FIRST', 2]]);
});
// ---- Step 4 (trial 4): a 1.9 plan refuses a placeholder and a mixed-case bare // ---- Step 4 (trial 4): a 1.9 plan refuses a placeholder and a mixed-case bare
// name as a file (operator decision 22, 13.10; review 33f37f53) ---- // name as a file (operator decision 22, 13.10; review 33f37f53) ----