From bd79a97df091eec2a2113995b6090dcab3889d3c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 20 Sep 2026 10:26:53 -0400 Subject: [PATCH] fix(#4806): unparseable VERIFICATION.md frontmatter reports a distinct parse error, never status missing (#4896) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#4806): failing-first — unparseable VERIFICATION.md frontmatter is a parse error, not status missing / Field not found * fix(#4806): unparseable VERIFICATION.md frontmatter reports a distinct parse error — never 'missing' or 'Field not found' * test(#4806): census pin 66→67 — cmdFrontmatterGet's unparseable-frontmatter error is a new output({error}) call site * chore(#4806): backfill changeset PR number (4896) --------- Co-authored-by: sim --- .changeset/bold-badgers-march.md | 5 +++++ src/frontmatter.cts | 8 ++++++++ src/verification.cts | 21 ++++++++++++++++++++- tests/frontmatter-cli.test.cjs | 30 ++++++++++++++++++++++++++++++ tests/io.test.cjs | 6 +++--- tests/verification-status.test.cjs | 24 ++++++++++++++++++++++++ 6 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 .changeset/bold-badgers-march.md diff --git a/.changeset/bold-badgers-march.md b/.changeset/bold-badgers-march.md new file mode 100644 index 000000000..ac80a8b7f --- /dev/null +++ b/.changeset/bold-badgers-march.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4896 +--- +**verification status and frontmatter get distinguish unparseable YAML from a missing report** — a VERIFICATION.md whose frontmatter has a YAML syntax error was reported as status "missing" (sending the operator to re-run execute-phase, which cannot fix a YAML typo), and frontmatter get answered "Field not found"; both now report a distinct parse error. (#4806) diff --git a/src/frontmatter.cts b/src/frontmatter.cts index a301d1f63..44c097ffc 100644 --- a/src/frontmatter.cts +++ b/src/frontmatter.cts @@ -1378,6 +1378,14 @@ function cmdFrontmatterGet(cwd: string, filePath: string, field: string | undefi // Pass the resolved path so a truncated file is named in the diagnostic and deduplicated // per file rather than per content digest (#1882, ADR-1411 wiring clause). const fm = extractFrontmatter(content, fullPath); + // #4806: an unparseable frontmatter block is a distinct outcome — the file + // HAS a frontmatter block but its YAML failed to parse. Reporting + // "Field not found" tells the caller the key is absent, which is + // indistinguishable from a file that genuinely lacks it. + if ((fm as unknown as Record)[FRONTMATTER_UNPARSEABLE] === true) { + output({ error: 'Frontmatter is not parseable YAML — fix the syntax error in the frontmatter block', path: filePath }, raw, undefined); + return; + } if (field) { const value = fm[field]; if (value === undefined) { output({ error: 'Field not found', field }, raw, undefined); return; } diff --git a/src/verification.cts b/src/verification.cts index 0d4b2477d..2b93f96eb 100644 --- a/src/verification.cts +++ b/src/verification.cts @@ -49,7 +49,7 @@ import { isContainedIn } from './security.cjs'; const { output, error } = io; const { extractPhaseToken, scopeToPhase } = phaseId; -const { extractFrontmatter } = frontmatterMod; +const { extractFrontmatter, FRONTMATTER_UNPARSEABLE } = frontmatterMod; const { normalizeLineEndings } = coreUtilsMod; const { SCOPE } = planningScopeMod; type Scope = planningScopeMod.Scope; @@ -128,6 +128,15 @@ const VERIFICATION_ROUTING_TABLE: Record = { next_action: 'No verification report found — the verify step never completed. Running execute-phase is safe here: it resumes at the verification gates and does not re-run plans that already have a SUMMARY.md (see #2868).', next_command: 'execute-phase', }, + // #4806: the report EXISTS but its frontmatter is not parseable YAML — + // fundamentally different from "missing" (the verify step DID run; re-running + // execute-phase cannot fix a YAML typo). Consumers treat any non-'passed' + // status as blocking, so this value fails safe while telling the truth. + unparseable: { + status: 'unparseable', + next_action: "The *-VERIFICATION.md frontmatter is not parseable YAML — fix the syntax error in the report itself. Re-running execute-phase cannot fix a YAML typo in an existing report.", + next_command: '', + }, // INTERNAL SENTINEL: constructed when the file has a status value not in // VERIFIER_STATUSES. Never emitted by the verifier. unknown: { @@ -1028,6 +1037,16 @@ function readVerificationStatus( // same root cause as the false-clean class fixed elsewhere in #3707-CR. const content = normalizeLineEndings(fsImpl.readFileSync(filePath, 'utf-8')); fm = extractFrontmatter(content, filePath); + // #4806: an unparseable frontmatter block is NOT "missing" — the file + // exists and verification ran. Report a distinct status so the caller is + // sent to fix the YAML, not to re-run execute-phase. + if ((fm as unknown as Record)[FRONTMATTER_UNPARSEABLE] === true) { + return { + status: 'unparseable', + next_action: "The *-VERIFICATION.md frontmatter is not parseable YAML — fix the syntax error in the report itself. Re-running execute-phase cannot fix a YAML typo in an existing report.", + next_command: '', + }; + } const statusVal = fm['status']; // status is always a scalar string in a well-formed VERIFICATION.md frontmatter; // only accept string values — arrays and objects are not valid status values. diff --git a/tests/frontmatter-cli.test.cjs b/tests/frontmatter-cli.test.cjs index 58ad21eac..2c87a0c2d 100644 --- a/tests/frontmatter-cli.test.cjs +++ b/tests/frontmatter-cli.test.cjs @@ -783,3 +783,33 @@ describe('frontmatter get — truncated vs absent frontmatter (#1882)', () => { assert.strictEqual(r.stderr, '', 'a horizontal rule is valid Markdown, not a truncated file'); }); }); + +// ─── #4806: unparseable frontmatter is a distinct error, not "Field not found" ── + +describe('#4806 frontmatter get — unparseable frontmatter', () => { + test('reports a parse error naming the field, not "Field not found"', () => { + // An invalid backslash escape inside a double-quoted value is a YAML + // SYNTAX error: the file HAS a status key but its frontmatter cannot be + // read. Reporting "Field not found" tells the caller the key is absent — + // indistinguishable from a file that genuinely lacks it. + const file = writeTempFile('---\nstatus: "passed\n---\n\n# Verification Report\n'); + const result = runGsdTools(['frontmatter', 'get', file, '--field', 'status']); + // The verb answers exit-0 JSON with an `error` FIELD (its documented + // error shape) — the assertion is on the error text, not the exit code. + assert.strictEqual(result.success, true, `command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.ok( + (parsed.error || '').includes('not parseable YAML'), + `must report a parse error, got: ${JSON.stringify(parsed)}`, + ); + assert.ok(!parsed.error.includes('Field not found'), 'parse failure must not read as Field not found'); + }); + + test('a well-formed file still returns the field', () => { + const file = writeTempFile('---\nstatus: passed\n---\n\n# V\n'); + const result = runGsdTools(['frontmatter', 'get', file, '--field', 'status']); + assert.ok(result.success, `command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.strictEqual(parsed.status, 'passed'); + }); +}); diff --git a/tests/io.test.cjs b/tests/io.test.cjs index 07aba6a0f..04113ad6b 100644 --- a/tests/io.test.cjs +++ b/tests/io.test.cjs @@ -914,12 +914,12 @@ describe('#3912 A3-A5: output({error}) records DEGRADED — shape-exhaustive plu assert.deepStrictEqual( perFile, { - 'commands.cts': 5, 'frontmatter.cts': 7, 'gsd2-import.cts': 2, 'phase.cts': 4, - 'roadmap.cts': 3, 'state.cts': 27, 'template.cts': 3, 'verify.cts': 8, 'workstream.cts': 7, // +1 #3807: advance-plan's ambiguous-position error; +1 #3784: advance-plan's ambiguous-PLAN-position error (two plan spellings, different numbers) + 'commands.cts': 5, 'frontmatter.cts': 8, 'gsd2-import.cts': 2, 'phase.cts': 4, + 'roadmap.cts': 3, 'state.cts': 27, 'template.cts': 3, 'verify.cts': 8, 'workstream.cts': 7, // +1 #3807: advance-plan's ambiguous-position error; +1 #3784: advance-plan's ambiguous-PLAN-position error (two plan spellings, different numbers); +1 #4806: cmdFrontmatterGet's unparseable-frontmatter error }, `per-file output({error}) census drifted: ${JSON.stringify(perFile)}`, ); - assert.strictEqual(total, 66, `enumerated output({error}) population drifted from the measured 66 (64 + #3807's ambiguous-position error + #3784's ambiguous-plan-position error): got ${total}`); + assert.strictEqual(total, 67, `enumerated output({error}) population drifted from the measured 67 (66 + #4806's cmdFrontmatterGet unparseable-frontmatter error): got ${total}`); }); }); diff --git a/tests/verification-status.test.cjs b/tests/verification-status.test.cjs index 735465cf7..f1fa96b72 100644 --- a/tests/verification-status.test.cjs +++ b/tests/verification-status.test.cjs @@ -3165,3 +3165,27 @@ describe('#2868: verification status CLI drives the execute-phase stranded-phase }); }); } + +// ─── #4806: unparseable VERIFICATION.md frontmatter is a parse error, not "missing" ── + +describe('#4806: unparseable VERIFICATION.md frontmatter reports a parse error, not missing', () => { + test('a VERIFICATION.md whose frontmatter fails to parse reports status unparseable, not missing', () => { + // The file EXISTS and verification ran — "missing" (and its next_command + // re-running execute-phase) is a false statement about the phase. The + // YAML syntax error is in the report, not in the phase's execution. + const dir = mkPhaseDir('unparseable'); + fs.writeFileSync(path.join(dir, '01-foo-VERIFICATION.md'), + '---\nstatus: "passed\n---\n\n# Verification Report\n'); + const result = readVerificationStatus(dir); + assert.equal(result.status, 'unparseable', 'status must be unparseable'); + assert.ok(result.next_action.includes('not parseable YAML'), + 'next_action must name the YAML parse failure'); + }); + + test('a well-formed control file still reports passed (unchanged)', () => { + const dir = mkPhaseDir('control'); + writeVerificationMd(dir, '01-foo-VERIFICATION.md', 'passed'); + const result = readVerificationStatus(dir); + assert.equal(result.status, 'passed'); + }); +});