From e2d879f6815446527b5e0fbb5c8f67c891d9f3f1 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 20 Sep 2026 08:45:54 -0400 Subject: [PATCH] fix(#4802): audit acknowledge refuses targets whose frontmatter fails to parse (#4895) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#4802): failing-first — acknowledge must refuse an unparseable-frontmatter target instead of splicing over it * fix(#4802): audit acknowledge refuses targets whose frontmatter fails to parse — never splices the marker-marked object over the file * chore(#4802): backfill changeset PR number (4895) --------- Co-authored-by: sim --- .changeset/eager-wolves-bark.md | 2 +- src/audit.cts | 16 ++++++- tests/audit-command-cutover.test.cjs | 66 ++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/.changeset/eager-wolves-bark.md b/.changeset/eager-wolves-bark.md index 845ccbe03..c895b8808 100644 --- a/.changeset/eager-wolves-bark.md +++ b/.changeset/eager-wolves-bark.md @@ -1,5 +1,5 @@ --- type: Fixed -pr: 4889 +pr: 4895 --- **check.decision-coverage-plan stops answering an unmeasured shape** — on could-not-parse it reported covered: 0 / uncovered: [] (fields of a measurement that never happened) and a directory passed as the context path certified passed: true; the gate now answers covered: null / total: null with the unreadable decision ids (and omits uncovered), and a non-file context path fails closed naming the path. (#4794) diff --git a/src/audit.cts b/src/audit.cts index 58497534b..847790068 100644 --- a/src/audit.cts +++ b/src/audit.cts @@ -30,7 +30,7 @@ import frontmatter = require('./frontmatter.cjs'); // does not require this module, so the edge is acyclic. // eslint-disable-next-line @typescript-eslint/no-require-imports import commandsModule = require('./commands.cjs'); -const { extractFrontmatter, spliceFrontmatter } = frontmatter; +const { extractFrontmatter, FRONTMATTER_UNPARSEABLE, spliceFrontmatter } = frontmatter; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); const { PHASE_NUMBER_TOKEN_SOURCE, scopeToPhase } = phaseIdMod; @@ -1711,6 +1711,13 @@ function cmdAuditAcknowledge(cwd: string, args: string[], raw: boolean): void { const content = fs.readFileSync(safeFilePath, 'utf-8'); const fm = extractFrontmatter(content, safeFilePath); + // #4802: an unparseable frontmatter block is NOT an empty one — splicing + // the marker-marked object over the file would discard every field the + // author actually wrote. Refuse and name the file (the write-path + // counterpart of the read-side FRONTMATTER_UNPARSEABLE contract). + if ((fm as unknown as Record)[FRONTMATTER_UNPARSEABLE] === true) { + ioError(`refusing to acknowledge — the frontmatter of "${file as string}" is not parseable YAML (splicing would discard every other frontmatter field); fix the YAML syntax error first, then re-run`); + } // Mixed-frame fix (security review, 4th instance on this branch): the // splice above and below stays keyed to RAW `content` (raw byte offsets // must not shift), but `scanUatGaps`/`scanContextQuestions` now derive @@ -1853,6 +1860,13 @@ function cmdAuditAcknowledge(cwd: string, args: string[], raw: boolean): void { const presenceOnly = category === 'todos'; const fm = createIfMissing ? fmForCreate : extractFrontmatter(fs.readFileSync(safeFilePath, 'utf-8'), safeFilePath); + // #4802: same unparseable-frontmatter refusal as the phase-scoped branch — + // createIfMissing never reaches this extract (it only fires when the file is + // absent), so an existing file with broken YAML refuses instead of splicing + // a near-empty object over every field the author wrote. + if (!createIfMissing && (fm as unknown as Record)[FRONTMATTER_UNPARSEABLE] === true) { + ioError(`refusing to acknowledge — the frontmatter of "${safeFilePath}" is not parseable YAML (splicing would discard every other frontmatter field); fix the YAML syntax error first, then re-run`); + } fm.audit_acknowledged = presenceOnly ? { ...markerBase } : { ...markerBase, [snapshotKey]: currentValue }; const newContent = createIfMissing ? spliceFrontmatter('', fm) diff --git a/tests/audit-command-cutover.test.cjs b/tests/audit-command-cutover.test.cjs index b2ac019af..1f50d6718 100644 --- a/tests/audit-command-cutover.test.cjs +++ b/tests/audit-command-cutover.test.cjs @@ -2673,3 +2673,69 @@ describe('bug #950: quick-task SUMMARY must carry status: complete', () => { }); }); } + +// ─── #4802: an unparseable frontmatter block must not be spliced over ────── + +describe('#4802: acknowledge refuses targets whose frontmatter fails to parse', () => { + const fs = require('node:fs'); + const path = require('node:path'); + let tmpDir; + + beforeEach(() => { tmpDir = createTempProject('gsd-4802-'); }); + afterEach(() => { cleanup(tmpDir); }); + + function planningPath(...segs) { + return path.join(tmpDir, '.planning', ...segs); + } + + // A real YAML SYNTAX error (the issue's second shape: an invalid backslash + // escape inside a double-quoted value). Note the issue's first shape + // (unescaped colons inside a double-quoted value) is actually LEGAL YAML — + // double-quoted scalars may contain colons — so the fixture uses a shape + // that genuinely fails to parse. Empirically verified against the built + // frontmatter.cjs: this content returns an object marked + // FRONTMATTER_UNPARSEABLE. + const UNPARSEABLE_FM = [ + '---', + 'status: complete', + 'ref: "bad\\q escape"', + 'key-decisions:', + ' - decision one', + '---', + ].join('\n'); + + function ack(tmpDir, args) { + return runGsdTools(['audit-open', 'acknowledge', ...args, '--json'], tmpDir); + } + + test('threads: an unparseable-frontmatter target is refused, file byte-identical', () => { + const threadsDir = planningPath('threads'); + fs.mkdirSync(threadsDir, { recursive: true }); + const filePath = path.join(threadsDir, 'broken-yaml.md'); + const before = UNPARSEABLE_FM + '\n# Thread\n'; + fs.writeFileSync(filePath, before, 'utf-8'); + + const result = ack(tmpDir, ['--category', 'threads', '--slug', 'broken-yaml', '--milestone', 'v1.0']); + assert.ok(!result.success, `acknowledge must refuse; stdout: ${result.output}\nstderr: ${result.error}`); + assert.ok( + (result.error || '').includes('not parseable YAML') && (result.error || '').includes('broken-yaml.md'), + `the refusal must name the file and the unparseable frontmatter; stderr: ${result.error}`, + ); + assert.strictEqual(fs.readFileSync(filePath, 'utf-8'), before, + 'the file must be byte-identical — no splice may discard frontmatter'); + }); + + test('uat_gaps (phase-scoped): an unparseable-frontmatter target is refused, file byte-identical', () => { + const ctxDir = planningPath('phases', '01-init'); + fs.mkdirSync(ctxDir, { recursive: true }); + const filePath = path.join(ctxDir, 'CONTEXT.md'); + const before = UNPARSEABLE_FM + '\n# Context\n'; + fs.writeFileSync(filePath, before, 'utf-8'); + fs.writeFileSync(path.join(ctxDir, '01-01-PLAN.md'), '# Plan\n'); + + const result = ack(tmpDir, ['--category', 'uat_gaps', '--phase', '01', '--file', 'CONTEXT.md', '--milestone', 'v1.0']); + assert.ok(!result.success, `acknowledge must refuse; stdout: ${result.output}\nstderr: ${result.error}`); + assert.strictEqual(fs.readFileSync(filePath, 'utf-8'), before, + 'the file must be byte-identical — no splice may discard frontmatter'); + }); +});