diff --git a/.changeset/1343-decision-coverage-false-pass.md b/.changeset/1343-decision-coverage-false-pass.md new file mode 100644 index 000000000..5f43d049f --- /dev/null +++ b/.changeset/1343-decision-coverage-false-pass.md @@ -0,0 +1,6 @@ +--- +type: Fixed +pr: 1358 +--- + +**`check.decision-coverage-plan` no longer reports a false pass when a `D-NN` decision header has text before the colon** — `parseDecisions` previously dropped any `- **D-NN …:**` bullet whose header contained a `(parenthetical)`, em-dash, or other prose before the `:**`, silently narrowing the trackable set so the blocking coverage gate green-lit a phase whose dropped decisions were never checked. The parser now tolerates a freeform run before the colon (preserving `[bracket]` tags) and warns on any `D-NN` bullet it still cannot parse instead of dropping it. (#1343) diff --git a/src/decisions.cts b/src/decisions.cts index 117834a52..9aa7789fc 100644 --- a/src/decisions.cts +++ b/src/decisions.cts @@ -70,7 +70,13 @@ export function parseDecisions(content: unknown): Decision[] { // in addition to numeric-only IDs (D-42). The first character after `D-` must // be alphanumeric, so malformed shapes like `D--foo` or `D-_bar` are rejected. // CJS callers consume {id, text} and ignore the optional extras. - const bulletRe = /^\s*-\s+\*\*D-([A-Za-z0-9][A-Za-z0-9_-]*)(?:\s*\[([^\]]+)\])?\s*:\*\*\s*(.*)$/; + // #1343: `[^:*]*` replaces the old `\s*` before `:**` so that a freeform run + // such as `(parenthetical)`, an em-dash, or other prose between the optional + // bracket-tag group and the closing `:**` is tolerated rather than silently + // dropping the whole decision. `[^:*]*` subsumes plain whitespace and stops + // correctly at `:**`. Capture groups 1 (id), 2 (bracket tags), 3 (text) are + // unchanged. + const bulletRe = /^\s*-\s+\*\*D-([A-Za-z0-9][A-Za-z0-9_-]*)(?:\s*\[([^\]]+)\])?[^:*]*:\*\*\s*(.*)$/; let current: Decision | null = null; const flush = (): void => { if (current) { @@ -111,6 +117,18 @@ export function parseDecisions(content: unknown): Decision[] { current = { id, text: bulletMatch[3], category, tags, trackable }; continue; } + // Parse-miss guard (#1343): a line that looks like a `D-NN` decision bullet + // but failed `bulletRe` (e.g. a `:` or `*` inside the pre-colon run) must NOT + // be silently dropped — a narrowed trackable set lets a blocking coverage gate + // report a false pass. Surface it loudly instead. + if (/^\s*-\s+\*\*D-/.test(line)) { + // A malformed D-bullet still starts a (failed) new decision, so it ends the + // previous one — flush before warning so a following continuation line cannot + // be mis-appended to the prior valid decision. + flush(); + console.warn(`parseDecisions: ignored unparseable decision bullet: ${trimmed}`); + continue; + } // Continuation line for current decision (indented with space OR tab, // non-bullet, non-empty) — tab indentation must work too (review F12). if (current && trimmed !== '' && !trimmed.startsWith('-') && /^[ \t]/.test(line)) { diff --git a/tests/post-planning-gaps-2493.test.cjs b/tests/post-planning-gaps-2493.test.cjs index b4665d1ae..deeb0a35d 100644 --- a/tests/post-planning-gaps-2493.test.cjs +++ b/tests/post-planning-gaps-2493.test.cjs @@ -30,6 +30,7 @@ const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); const REPO_ROOT = path.join(__dirname, '..'); const PLAN_PHASE_PATH = path.join(REPO_ROOT, 'gsd-core', 'workflows', 'plan-phase.md'); +const { parseDecisions } = require('../gsd-core/bin/lib/decisions.cjs'); // ─── Workflow file structure ────────────────────────────────────────────────── @@ -82,8 +83,6 @@ describe('plan-phase.md Step 13e insertion (#2493)', () => { // ─── Decisions parser ──────────────────────────────────────────────────────── describe('decisions.cjs parser (shared with #2492)', () => { - const { parseDecisions } = require('../gsd-core/bin/lib/decisions.cjs'); - test('extracts D-NN entries from a block', () => { const md = ` @@ -446,3 +445,181 @@ describe('workflow.post_planning_gaps config (#2493)', () => { assert.strictEqual(config.post_planning_gaps, true); }); }); + +// ─── #1343 regression suite ────────────────────────────────────────────────── + +// helper shared across #1343 cases +function wrapDecisions(body) { + return `\n## Decisions\n\n${body}\n\n`; +} + +describe('#1343 — parseDecisions tolerates freeform text before the colon (regressions)', () => { + // ── 1. Parenthetical before colon now parses ───────────────────────────── + + test('all three ids extracted when one bullet has a parenthetical before :**', () => { + const md = wrapDecisions( + '- **D-01:** a\n' + + '- **D-02 (note before colon):** b\n' + + '- **D-03 [robust]:** c\n' + ); + const ds = parseDecisions(md); + assert.deepStrictEqual( + ds.map(d => d.id), + ['D-01', 'D-02', 'D-03'], + 'D-02 with parenthetical must not be dropped' + ); + assert.strictEqual(ds.length, 3); + }); + + test('text is preserved for parenthetical bullet', () => { + const md = wrapDecisions('- **D-02 (note before colon):** b\n'); + const ds = parseDecisions(md); + assert.strictEqual(ds[0].id, 'D-02'); + assert.strictEqual(ds[0].text, 'b'); + }); + + // ── 2. Bracket tags still captured + drive trackable ──────────────────── + + test('[informational] tag makes trackable:false', () => { + const md = wrapDecisions( + '- **D-04 [informational]:** x\n' + + '- **D-05:** y\n' + ); + const ds = parseDecisions(md); + const d04 = ds.find(d => d.id === 'D-04'); + const d05 = ds.find(d => d.id === 'D-05'); + assert.ok(d04, 'D-04 must be present'); + assert.ok(d04.tags.includes('informational'), 'D-04 tags must include informational'); + assert.strictEqual(d04.trackable, false, 'D-04 must be non-trackable'); + assert.ok(d05, 'D-05 must be present'); + assert.strictEqual(d05.trackable, true, 'D-05 must be trackable'); + }); + + // ── 3. Bracket + parenthetical together ───────────────────────────────── + + test('D-06 [robust] (note) parses correctly', () => { + const md = wrapDecisions('- **D-06 [robust] (note):** z\n'); + const ds = parseDecisions(md); + assert.strictEqual(ds.length, 1); + assert.strictEqual(ds[0].id, 'D-06'); + assert.ok(ds[0].tags.includes('robust'), 'tags must include robust'); + assert.strictEqual(ds[0].text, 'z'); + }); + + // ── 4. Parse-miss WARN floor ───────────────────────────────────────────── + + test('genuinely unparseable bullet (colon inside pre-colon run) is excluded and warns', () => { + // `D-07 ratio 3:1` has a colon in the pre-colon run; after [^:*]* matches up + // to the first colon, the `:**` anchor fails → bulletRe does not match → falls + // through to the parse-miss guard, which must warn and skip. + const md = wrapDecisions('- **D-07 ratio 3:1:** w\n'); + + const warnMessages = []; + const origWarn = console.warn; + try { + console.warn = (...args) => { + warnMessages.push(args.join(' ')); + }; + const ds = parseDecisions(md); + assert.strictEqual(ds.length, 0, 'unparseable bullet must be excluded from results'); + } finally { + console.warn = origWarn; + } + + assert.ok( + warnMessages.some(m => m.includes('D-07')), + `expected a console.warn mentioning D-07, got: ${JSON.stringify(warnMessages)}` + ); + }); + + // ── Test A — malformed D-bullet must not corrupt previous decision's text ─ + + test('D-02 malformed flush: D-01 text stays clean, continuation does not attach', () => { + // D-02 has a colon inside the pre-colon run ("ratio 3:1"), so bulletRe rejects + // it and the parse-miss guard fires. Before this fix the guard skipped WITHOUT + // flushing, leaving current=D-01; the following indented continuation line was + // then mis-appended to D-01's text. + const md = wrapDecisions( + '- **D-01:** first decision\n' + + '- **D-02 ratio 3:1:** malformed (unparseable, has colon in pre-colon run)\n' + + ' indented continuation that must NOT attach to D-01\n' + + '- **D-03:** third decision\n' + ); + + const warnMessages = []; + const origWarn = console.warn; + try { + console.warn = (...args) => { warnMessages.push(args.join(' ')); }; + const ds = parseDecisions(md); + + assert.deepStrictEqual( + ds.map(d => d.id), + ['D-01', 'D-03'], + 'only D-01 and D-03 should be present (D-02 dropped)' + ); + + const d01 = ds.find(d => d.id === 'D-01'); + assert.ok(d01, 'D-01 must be present'); + assert.strictEqual( + d01.text, + 'first decision', + 'D-01 text must be exactly "first decision", not polluted by continuation' + ); + assert.ok(!d01.text.includes('indented'), 'D-01 text must NOT contain "indented"'); + + const d03 = ds.find(d => d.id === 'D-03'); + assert.ok(d03, 'D-03 must be present'); + } finally { + console.warn = origWarn; + } + + assert.ok( + warnMessages.some(m => m.includes('D-02')), + `expected a console.warn mentioning D-02, got: ${JSON.stringify(warnMessages)}` + ); + }); + + // ── Test B — malformed/unterminated bracket tag: safe tagless trackable ── + + test('D-09 [informational: (missing ]) yields tags=[] and trackable=true', () => { + // "- **D-09 [informational:** body" has no closing `]` so the optional bracket + // group in bulletRe does not match, leaving tags=[]. The decision is still + // captured because the ID and `:**` are intact. + // + // This is the intentional SAFE DIRECTION for the coverage gate: a tagless + // trackable decision can only make the gate STRICTER (it counts toward required + // decisions), never produce a false pass. An alternative that silently turned + // the decision non-trackable could allow a gate bypass. + const md = wrapDecisions('- **D-09 [informational:** body\n'); + const ds = parseDecisions(md); + assert.strictEqual(ds.length, 1, 'D-09 should be present'); + assert.strictEqual(ds[0].id, 'D-09'); + assert.deepStrictEqual(ds[0].tags, [], 'tags must be empty (unclosed bracket not parsed)'); + assert.strictEqual(ds[0].trackable, true, 'trackable must be true (no non-trackable tag matched)'); + }); + + // ── 5. Non-regression: plain bullet + continuation ─────────────────────── + + test('D-08 with continuation line parses correctly', () => { + const md = wrapDecisions( + '- **D-08:** ok\n' + + ' continuation text here\n' + ); + const ds = parseDecisions(md); + assert.strictEqual(ds.length, 1); + assert.strictEqual(ds[0].id, 'D-08'); + assert.ok(ds[0].text.includes('ok'), 'text must include bullet text'); + assert.ok(ds[0].text.includes('continuation'), 'text must include continuation'); + }); + + test('existing numeric and bracket forms unchanged', () => { + const md = wrapDecisions( + '- **D-42:** numeric id\n' + + '- **D-INFRA-01 [deferred]:** alphanumeric id\n' + ); + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-42', 'D-INFRA-01']); + assert.ok(ds[1].tags.includes('deferred')); + assert.strictEqual(ds[1].trackable, false); + }); +});