From fd64389616107b7f47fd184e4ebe0717bfbbbd1b Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 3 Aug 2026 15:21:30 -0400 Subject: [PATCH] fix(#2703): strip GSD-2 frontmatter with the canonical parser (#3027) * test(#2703): failing-first coverage for CRLF frontmatter strip in SUMMARY.md Drives the exported buildPlanningArtifacts seam. Rows for CRLF/LF parity, stacked blocks and a leading BOM fail against the current hand-rolled regex; the negative-space rows pin behavior that must not change. Co-Authored-By: Claude Opus 5 * fix(#2703): strip GSD-2 frontmatter with the canonical parser buildSummaryMd matched the closing delimiter with a hardcoded bare \n, so a CRLF-authored task summary never matched and fell through to the raw-passthrough branch. The function then prepended its own block, emitting a SUMMARY.md with two stacked frontmatter blocks and no warning. Delegates to stripFrontmatter from frontmatter.cts -- the canonical, line-ending tolerant primitive this repo already deduplicated once (#2143) -- instead of adding another hand-rolled variant. Co-Authored-By: Claude Opus 5 * fix(#2703): strip only the first frontmatter block in gsd2 import Adversarial review caught a regression in the first cut: stripFrontmatter loops by design, so a summary body opening with a thematic-break-delimited section (--- / heading / ---) had that section silently deleted. The old pre-#2703 regex preserved it, so shipping the loop would have traded one silent corruption for another. Adds an explicit { once } option to the canonical primitive -- default behavior and the two existing callers are unchanged -- and has buildSummaryMd opt in. A GSD-2 summary is an arbitrary user document, not a GSD artifact with a known doubling failure mode, so a second block there is body content. This also makes the acceptance criterion exact: CRLF now produces the same result LF already produced, rather than a new result for both. Co-Authored-By: Claude Opus 5 * chore(#2703): backfill changeset pr number Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- .changeset/noble-jays-leap.md | 5 ++ src/frontmatter.cts | 19 +++-- src/gsd2-import.cts | 22 ++++- tests/frontmatter.unit.test.cjs | 52 ++++++++++++ tests/gsd2-import.test.cjs | 141 ++++++++++++++++++++++++++++++++ 5 files changed, 230 insertions(+), 9 deletions(-) create mode 100644 .changeset/noble-jays-leap.md diff --git a/.changeset/noble-jays-leap.md b/.changeset/noble-jays-leap.md new file mode 100644 index 000000000..6a5e0abe2 --- /dev/null +++ b/.changeset/noble-jays-leap.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3027 +--- +**GSD-2 import no longer duplicates frontmatter in the generated SUMMARY.md** — importing a GSD-2 project whose task summaries were authored with CRLF line endings emitted the original GSD-2 frontmatter a second time, as body text, below the new one. Stripping now goes through the canonical line-ending-tolerant parser. (#2703) diff --git a/src/frontmatter.cts b/src/frontmatter.cts index ebbd69414..539f77755 100644 --- a/src/frontmatter.cts +++ b/src/frontmatter.cts @@ -642,22 +642,29 @@ const FRONTMATTER_SCHEMAS: Record { assert.ok(msg.includes('Edit the file directly'), msg); }); }); + +// ─── stripFrontmatter ───────────────────────────────────────────────────────── + +describe('stripFrontmatter', () => { + const stacked = ['---', 'a: 1', '---', '---', 'b: 2', '---', '', 'Real body.'].join('\n'); + + test('strips a single block', () => { + assert.strictEqual(stripFrontmatter(['---', 'a: 1', '---', '', 'Body.'].join('\n')), 'Body.'); + }); + + test('is CRLF-tolerant', () => { + const crlf = ['---', 'a: 1', '---', '', 'Body.'].join('\r\n'); + assert.strictEqual(stripFrontmatter(crlf), 'Body.'); + }); + + test('defaults to stripping every stacked block (corruption recovery)', () => { + assert.strictEqual(stripFrontmatter(stacked), 'Real body.'); + }); + + test('an omitted options argument keeps the greedy default', () => { + // Back-compat: state.cts and state-transition.cts call this with one arg. + assert.strictEqual(stripFrontmatter(stacked, {}), 'Real body.'); + }); + + test('once: true stops after the first block', () => { + assert.strictEqual( + stripFrontmatter(stacked, { once: true }), + ['---', 'b: 2', '---', '', 'Real body.'].join('\n'), + ); + }); + + test('once: false is the greedy default', () => { + assert.strictEqual(stripFrontmatter(stacked, { once: false }), 'Real body.'); + }); + + test('returns content unchanged when there is no frontmatter', () => { + const plain = ['Just prose.', '', 'More prose.'].join('\n'); + assert.strictEqual(stripFrontmatter(plain), plain); + assert.strictEqual(stripFrontmatter(plain, { once: true }), plain); + }); + + test('leaves an unterminated block alone under both modes', () => { + const unterminated = ['---', 'a: 1', 'b: 2'].join('\n'); + assert.strictEqual(stripFrontmatter(unterminated), unterminated); + assert.strictEqual(stripFrontmatter(unterminated, { once: true }), unterminated); + }); + + test('empty string round-trips', () => { + assert.strictEqual(stripFrontmatter(''), ''); + }); +}); diff --git a/tests/gsd2-import.test.cjs b/tests/gsd2-import.test.cjs index e7d33b55b..7cd04bbb7 100644 --- a/tests/gsd2-import.test.cjs +++ b/tests/gsd2-import.test.cjs @@ -9,6 +9,7 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const { createTempDir, cleanup, runGsdTools } = require('./helpers.cjs'); +const fc = require('./helpers/fast-check-setup.cjs'); const { parseSlicesFromRoadmap, @@ -578,3 +579,143 @@ describe('gsd-tools from-gsd2 CLI', () => { assert.ok(!fs.existsSync(path.join(tmpDir, '.planning', 'phases', '02-auth-system', '02-01-SUMMARY.md'))); }); }); + +// ─── SUMMARY.md frontmatter stripping (#2703) ────────────────────────────── + +/** + * The emitted artifact under test. `buildSummaryMd` is module-private; + * `buildPlanningArtifacts` is its only caller and is the shape `cmdFromGsd2` + * drives in production, so every row below asserts through that seam rather + * than against the private function. + */ +const SUMMARY_KEY = 'phases/01-setup/01-01-SUMMARY.md'; + +function emitSummary(summary) { + return buildPlanningArtifacts({ + projectContent: '# P\n', + requirements: null, + milestones: [{ + id: 'M001', + title: 'Foundation', + slices: [{ + done: true, + id: 'S01', + title: 'Setup', + tasks: [{ done: true, id: 'T01', title: 'Init', description: '', mustHaves: [], summary }], + }], + }], + }).get(SUMMARY_KEY); +} + +/** Re-encode an LF document with CRLF line endings. */ +const crlf = (s) => s.replace(/\n/g, '\r\n'); + +/** The whole document `buildSummaryMd` is expected to emit for a given body. */ +const expectedDoc = (body) => ['---', 'phase: "01"', 'plan: "01"', '---', '', body, ''].join('\n'); + +/** + * Assert that `summary` — and its CRLF re-encoding — both emit the document + * built from `body`. CRLF/LF parity is the invariant this whole block exists + * to protect, so every row asserts it the same way rather than restating the + * pair by hand. + */ +function assertBothEncodings(summary, body) { + assert.strictEqual(emitSummary(summary), expectedDoc(body)); + assert.strictEqual(emitSummary(crlf(summary)), expectedDoc(crlf(body))); +} + +describe('buildSummaryMd frontmatter stripping (#2703)', () => { + test('strips GSD-2 frontmatter identically under CRLF and LF (#2703)', () => { + const summary = ['---', 'task: T01', 'status: done', '---', '', 'The task body.'].join('\n'); + // The bug: the CRLF emission carried a second, unstripped frontmatter block. + assert.strictEqual(emitSummary(crlf(summary)), emitSummary(summary)); + assertBothEncodings(summary, 'The task body.'); + }); + + test('strips a single frontmatter block', () => { + assertBothEncodings(['---', 'task: T01', '---', '', 'Body one.'].join('\n'), 'Body one.'); + }); + + test('passes through a summary that has no frontmatter', () => { + // Body line endings are passed through untouched — stripping frontmatter + // must not silently re-encode the author's prose. + const summary = ['Just prose.', '', 'No frontmatter here.'].join('\n'); + assertBothEncodings(summary, summary); + }); + + test('emits no SUMMARY.md at all for an empty summary', () => { + // buildPlanningArtifacts guards on `task.done && task.summary`, so an empty + // summary produces no artifact rather than a default-bodied one. + assert.strictEqual(emitSummary(''), undefined); + }); + + test('falls back to the migration default for a whitespace-only summary', () => { + assert.strictEqual(emitSummary(' \n \n'), expectedDoc('Task completed (migrated from GSD-2).')); + }); + + test('preserves a lone thematic break that opens the body', () => { + const summary = ['---', 'task: T01', '---', '', '---', '', 'Body after a rule.'].join('\n'); + assertBothEncodings(summary, ['---', '', 'Body after a rule.'].join('\n')); + }); + + test('does not eat a thematic-break-delimited section that opens the body', () => { + // Regression guard. The canonical stripper's DEFAULT greedy loop deletes + // `Some Heading` outright, because `---` / text / `---` is lexically a + // second frontmatter block. buildSummaryMd therefore passes `once: true`. + // Caught by adversarial review of the first cut of this fix; the old + // pre-#2703 regex preserved this content, so eating it would have been a + // silent regression shipped alongside the CRLF fix. + const summary = ['---', 'task: T01', '---', '---', 'Some Heading', '---', '', 'Body content below.'].join('\n'); + assertBothEncodings(summary, ['---', 'Some Heading', '---', '', 'Body content below.'].join('\n')); + }); + + test('strips only the first block when two frontmatter-shaped blocks lead', () => { + // Same `once` semantics stated for the YAML-shaped case: a GSD-2 summary is + // an arbitrary user document, so a second block is body content, not a + // corrupt duplicate header to be recovered from. + const summary = ['---', 'a: 1', '---', '---', 'b: 2', '---', '', 'Real body.'].join('\n'); + assertBothEncodings(summary, ['---', 'b: 2', '---', '', 'Real body.'].join('\n')); + }); + + test('does not strip a --- that appears mid-body', () => { + const summary = ['Intro prose.', '', '---', '', 'More prose.'].join('\n'); + assertBothEncodings(summary, summary); + }); + + test('leaves an unterminated frontmatter block as body text', () => { + const summary = ['---', 'task: T01', 'status: done'].join('\n'); + assertBothEncodings(summary, summary); + }); + + test('strips frontmatter behind a leading BOM', () => { + const summary = '' + ['---', 'task: T01', '---', '', 'Body.'].join('\n'); + assertBothEncodings(summary, 'Body.'); + }); + + test('property: CRLF and LF summaries emit the same SUMMARY.md (#2703)', () => { + // Body lines are drawn from a charset with no `-`, so a generated body can + // never accidentally form a second frontmatter block. + const yamlKey = fc.stringMatching(/^[a-z][a-z0-9_]{0,10}$/); + const yamlValue = fc.stringMatching(/^[a-zA-Z0-9 ._]{1,20}$/); + const bodyLine = fc.stringMatching(/^[a-zA-Z0-9 ._]{1,30}$/); + + fc.assert( + fc.property( + fc.array(fc.tuple(yamlKey, yamlValue), { minLength: 1, maxLength: 4 }), + fc.array(bodyLine, { minLength: 1, maxLength: 4 }), + (pairs, bodyLines) => { + const doc = [ + '---', + ...pairs.map(([k, v]) => `${k}: ${v}`), + '---', + '', + ...bodyLines, + ].join('\n'); + const fromLf = emitSummary(doc); + const fromCrlf = emitSummary(crlf(doc)); + assert.strictEqual(fromCrlf.replace(/\r\n/g, '\n'), fromLf); + }, + ), + ); + }); +});