From e58b5e172119e2d63373786b3c1b92d1f6a54d7c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 17 Jun 2026 12:35:38 -0400 Subject: [PATCH] fix(#1364): decisions adopt markdown-sectionizer seam + fail-loud coverage gate (epic #1372 T1) (#1386) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#1364,#1365): add decisions regression tests (fail-first proof) Adds tests/decisions.test.cjs with: - #1364 recall tests: parseDecisions from markdown-header + em-dash bullets (these FAIL on pre-T1 code, proving the bug is present before the fix) - #1365 fail-loud tests: check.decision-coverage-plan must return passed:false for decision-shaped but 0-extracted content (FAIL pre-T1, gate silently passed) - extractDecisions outcome enum tests (could-not-parse/none-present/parsed) - Parser QA matrix: CRLF, unicode headings, fenced-code suppression, both bullet forms - Boundary/threshold tests at limit-1 (0), limit (1) Co-Authored-By: Claude Sonnet 4.6 * fix(#1364,#1365): adopt markdown-sectionizer seam in decisions.cts; add fail-loud gate #1364 — Recall: decisions.cts now uses the seam's extractTaggedBlocks and collectSection for the markdown-header fallback path. Em-dash bullet form (- **D-NN — title** body) is now recognised alongside the existing colon form. #1365 — Fail-loud: adds extractDecisions() returning a typed DecisionExtraction { decisions, outcome } where outcome is 'parsed' | 'none-present' | 'could-not-parse'. The blocking gate (cmdDecisionCoveragePlan) now treats could-not-parse as passed:false with a format-mismatch reason instead of the prior silent passed:true/skip. gap-checker runGapAnalysis surfaces 'extracted 0 of N — possible format mismatch' for could-not-parse instead of 'No requirements or decisions to check'. parseDecisions remains a thin delegate over extractDecisions, so all existing callers are unaffected. Seam adoption: stripFencedCode (seam), extractTaggedBlocks(content,'decisions') (seam), collectSection(content, /decisions?/i, {levelBounded,stripFences}) (seam). Co-Authored-By: Claude Sonnet 4.6 * fix(#1364,#1365): tighten could-not-parse, parse-miss fail-loud, curly-quote discretion, gap-checker FIX D FIX A: empty scaffolds and all-prose sections no longer return could-not-parse; outcome is none-present unless the block/section contains a \bD- token or a parse-miss, preventing false blocks on legitimate phases. FIX B: parseDecisionLines now tracks parse-misses (D-NN-shaped bullets that fail both regexes); extractDecisions returns could-not-parse when parseMisses>0 even if some decisions parsed — silent drops no longer mask format errors. FIX C: curly-quote normalization regex now includes actual U+2018/U+2019 characters so '### Claude's Discretion' (curly apostrophe) correctly yields trackable:false (regression vs pre-T1 behavior). FIX D: gap-checker runGapAnalysis surfaces the decision could-not-parse format-mismatch signal independently of whether requirements items exist — previously masked inside `if (items.length === 0)`. Adds 14 behavioral regression tests (fail-first verified manually before fixes). Co-Authored-By: Claude Sonnet 4.6 * fix(#1365): fail-loud gate on parse-miss regardless of covered decisions Change the `could-not-parse` guard in `cmdDecisionCoveragePlan` and `cmdDecisionCoverageVerify` from `decisions.length === 0 && outcome === 'could-not-parse'` to fire on `outcome === 'could-not-parse'` alone. Previously a CONTEXT.md with a valid D-01 (covered by the plan) plus a malformed D-02 (parse-miss) would skip the guard (length === 1), proceed to coverage, find D-01 covered, and silently return passed:true — hiding the D-02 parse-miss entirely. Adds a gate-level fail-first test that places D-01 into a ## Must Haves section (DESIGNATED_HEADINGS_RE match) so coverage of D-01 would pass on its own, proving the only path to passed:false is the parse-miss fix. Also adds the matching verify-side advisory assertion. Co-Authored-By: Claude Sonnet 4.6 * chore(#1364,#1365): add Fixed changeset (pr:0 placeholder) Co-Authored-By: Claude Opus 4.8 * chore(#1364): backfill changeset PR number (1386) Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/witty-finches-hum.md | 5 + src/check-command-router.cts | 62 ++- src/decisions.cts | 264 ++++++++--- src/gap-checker.cts | 43 +- tests/decisions.test.cjs | 793 ++++++++++++++++++++++++++++++++ 5 files changed, 1096 insertions(+), 71 deletions(-) create mode 100644 .changeset/witty-finches-hum.md create mode 100644 tests/decisions.test.cjs diff --git a/.changeset/witty-finches-hum.md b/.changeset/witty-finches-hum.md new file mode 100644 index 000000000..69a45d9ac --- /dev/null +++ b/.changeset/witty-finches-hum.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1386 +--- +**Decision-coverage gate now reads markdown-header and em-dash decisions, and fails loud when it can't parse them** — `check.decision-coverage-plan` (a blocking gate) and `gap-analysis` previously extracted **zero** decisions from a populated CONTEXT.md that recorded its decisions under markdown headers (`## Locked decisions`) or with em-dash bullets (`- **D-1 — title**`), and silently reported a clean pass — so real decisions went un-checked. Decisions in those shapes are now recognized, and when decision-shaped content cannot be parsed (or a `- **D-NN**` bullet is malformed), the gate fails loud with a format-mismatch message instead of passing. (#1386) diff --git a/src/check-command-router.cts b/src/check-command-router.cts index 8dceaecbe..474ab91e8 100644 --- a/src/check-command-router.cts +++ b/src/check-command-router.cts @@ -18,7 +18,7 @@ const { planningDir } = planningWorkspaceMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocatorMod = require('./phase-locator.cjs'); const { findPhaseInternal } = phaseLocatorMod; -import { parseDecisions } from './decisions.cjs'; +import { extractDecisions } from './decisions.cjs'; import type { Decision } from './decisions.cjs'; import { checkUiPresence } from './ui-safety-gate.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -223,8 +223,12 @@ function buildVerifyMessage(notHonored: UncoveredItem[]): string { ].join('\n'); } -function loadTrackableDecisions(contextPath: string): Decision[] { - return parseDecisions(readIfExists(contextPath)).filter((decision) => decision.trackable); +function loadDecisionExtraction(contextPath: string): { trackable: Decision[]; outcome: 'parsed' | 'none-present' | 'could-not-parse' } { + const extraction = extractDecisions(readIfExists(contextPath)); + return { + trackable: extraction.decisions.filter((d) => d.trackable), + outcome: extraction.outcome, + }; } function cmdDecisionCoveragePlan(projectDir: string, args: string[], raw: boolean): void { @@ -240,7 +244,33 @@ function cmdDecisionCoveragePlan(projectDir: string, args: string[], raw: boolea return; } - const decisions = loadTrackableDecisions(contextPath); + const { trackable: decisions, outcome } = loadDecisionExtraction(contextPath); + + // #1365 fail-loud gate: any could-not-parse outcome must NOT silently pass — + // even when some decisions were extracted (e.g. D-01 valid but D-02 malformed). + // A parse-miss on ANY bullet means the gate cannot certify full coverage. + // Fire independent of decisions.length so a partial-parse still blocks. + if (outcome === 'could-not-parse') { + const partialParse = decisions.length > 0; + output({ + passed: false, + skipped: false, + reason: 'could-not-parse', + total: decisions.length, + covered: 0, + uncovered: [], + message: partialParse + ? 'Decision coverage gate: decisions could not be fully parsed — one or more ' + + '`- **D-NN ...**` bullets appear malformed (missing `:` or ` — ` separator). ' + + 'Fix the bullet format so all D-NN decisions can be read before re-running the gate.' + : 'Decision coverage gate: could not parse decisions — possible format mismatch. ' + + 'The CONTEXT.md appears to be decision-shaped (has a block, a decisions heading, ' + + 'or D- tokens) but no D-NN bullets could be extracted. Check the formatting of the decisions ' + + 'block and ensure bullets follow the `- **D-NN:** text` or `- **D-NN — title** body` form.', + }, raw, undefined); + return; + } + if (decisions.length === 0) { output({ passed: true, skipped: true, reason: 'no trackable decisions', total: 0, covered: 0, uncovered: [], message: 'No trackable decisions in CONTEXT.md.' }, raw, undefined); return; @@ -318,7 +348,29 @@ function cmdDecisionCoverageVerify(projectDir: string, args: string[], raw: bool return; } - const decisions = loadTrackableDecisions(contextPath); + const { trackable: decisions, outcome: decisionOutcome } = loadDecisionExtraction(contextPath); + + // Mirror could-not-parse surface for verify (non-blocking advisory WARN). + // Fire independent of decisions.length — a parse-miss on any bullet must surface, + // even when some decisions were partially extracted (#1365 fix-parity with plan gate). + if (decisionOutcome === 'could-not-parse') { + const partialParse = decisions.length > 0; + output({ + skipped: false, + blocking: false, + reason: 'could-not-parse', + total: decisions.length, + honored: 0, + not_honored: [], + message: partialParse + ? 'Decision coverage verify (warning): decisions could not be fully parsed — one or more ' + + '`- **D-NN ...**` bullets appear malformed. Fix the bullet format in the CONTEXT.md decisions block.' + : 'Decision coverage verify (warning): could not parse decisions — possible format mismatch. ' + + 'Check the formatting of the CONTEXT.md decisions block.', + }, raw, undefined); + return; + } + if (decisions.length === 0) { output({ skipped: true, blocking: false, reason: 'no trackable decisions', total: 0, honored: 0, not_honored: [], message: 'No trackable decisions in CONTEXT.md.' }, raw, undefined); return; diff --git a/src/decisions.cts b/src/decisions.cts index 9aa7789fc..add518973 100644 --- a/src/decisions.cts +++ b/src/decisions.cts @@ -7,8 +7,22 @@ * Accepts both numeric (D-42) and alphanumeric (D-INFRA-01) IDs. * Returns {id, text, category, tags, trackable} per decision. * CJS callers that only use {id, text} safely ignore the extra fields. + * + * ADR-1372 T1: rewritten to adopt the markdown-sectionizer seam. + * - `stripFencedCode` → seam's `stripFencedCode` (CommonMark-correct) + * - `extractDecisionsBlock` → seam's `extractTaggedBlocks(content,'decisions')` + * - Markdown-header fallback → seam's `collectSection(content, /decisions?/i, ...)` + * - Outer bullet loop → seam's `iterateBullets` (for the header-fallback path) + * + * Resolves #1364 (markdown-header + em-dash recall) and #1365 (fail-loud gate). */ +import { + stripFencedCode, + extractTaggedBlocks, + collectSection, +} from './markdown-sectionizer.cjs'; + export interface Decision { id: string; text: string; @@ -17,6 +31,21 @@ export interface Decision { trackable: boolean; } +/** + * Typed extraction result distinguishing three states the blocking gate cares about: + * - 'parsed' — ≥1 decision was successfully extracted + * - 'none-present' — content has no decision signals; nothing to check + * - 'could-not-parse'— content is decision-shaped (has a block, a + * /decisions?/i heading, a \bD- token, or an unterminated fence) + * yet 0 decisions were extracted → format mismatch, fail-loud + */ +export type DecisionOutcome = 'parsed' | 'none-present' | 'could-not-parse'; + +export interface DecisionExtraction { + decisions: Decision[]; + outcome: DecisionOutcome; +} + const DISCRETION_HEADINGS = new Set([ "claude's discretion", 'claudes discretion', @@ -24,60 +53,46 @@ const DISCRETION_HEADINGS = new Set([ ]); const NON_TRACKABLE_TAGS = new Set(['informational', 'folded', 'deferred']); +// ─── Bullet parsers (decisions-specific grammar) ───────────────────────────── + /** - * Strip fenced code blocks from `content` so example `` snippets - * inside ```` ``` ```` do not pollute the parser (review F11). + * Colon form: `- **D-NN[ [tags]]:** text` + * (#1343: `[^:*]*` subsumes any pre-colon prose, stops at `:**`) */ -function stripFencedCode(content: string): string { - return content.replace(/```[\s\S]*?```/g, ' ').replace(/~~~[\s\S]*?~~~/g, ' '); +const bulletColonRe = /^\s*-\s+\*\*D-([A-Za-z0-9][A-Za-z0-9_-]*)(?:\s*\[([^\]]+)\])?[^:*]*:\*\*\s*(.*)$/; + +/** + * Em-dash form: `- **D-NN[ [tags]] — title** body` + * The em-dash (U+2014) or its lookalike separates the ID+tags group from a title + * that lives inside the bold markers; the body (which may be empty) follows + * outside the closing `**`. This form was not handled pre-T1 (bug #1364). + * + * Accepts both U+2014 em-dash (—) and U+2013 en-dash (–) for robustness. + */ +const bulletEmDashRe = /^\s*-\s+\*\*D-([A-Za-z0-9][A-Za-z0-9_-]*)(?:\s*\[([^\]]+)\])?[^*]*[—–][^*]*\*\*\s*(.*)$/; + +interface ParseDecisionLinesResult { + decisions: Decision[]; + parseMisses: number; } /** - * Extract the inner text of EVERY `...` block in - * order, concatenated by `\n\n`. Returns null when no block is present. + * Parse decision lines from a block of text (the inner text of a + * or markdown-header section body). Returns the extracted decisions and a count + * of parse-misses (lines that looked like D-NN bullets but failed both regexes). * - * CONTEXT.md may legitimately contain more than one block (for example, a - * "current decisions" block plus a "carry-over from prior phase" block); - * dropping all-but-the-first silently lost the second batch (review F13). + * FIX B (#1365): parseMisses > 0 means the caller must treat the result as + * could-not-parse even when some decisions were extracted — a silent drop is + * worse than a fail-loud signal. */ -function extractDecisionsBlock(content: string): string | null { - const cleaned = stripFencedCode(content); - const matches = [...cleaned.matchAll(/([\s\S]*?)<\/decisions>/g)]; - if (matches.length === 0) - return null; - return matches.map((m) => m[1]).join('\n\n'); -} - -/** - * Parse trackable decisions from CONTEXT.md content. - * - * Returns ALL D-NN decisions found inside `` (including - * non-trackable ones, with `trackable: false`). Callers that only want the - * gate-enforced decisions should filter `.filter(d => d.trackable)`. - */ -export function parseDecisions(content: unknown): Decision[] { - if (!content || typeof content !== 'string') - return []; - const block = extractDecisionsBlock(content); - if (block === null) - return []; +function parseDecisionLines(block: string): ParseDecisionLinesResult { const lines = block.split(/\r?\n/); const out: Decision[] = []; let category = ''; let inDiscretion = false; - // Bullet line: `- **D-NN[ [tags]]:** text` - // Phase 6 (#3575): aligned to CJS regex — accepts alphanumeric IDs (D-01, D-INFRA-01, D-FOO_BAR) - // 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. - // #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; + let parseMisses = 0; + const flush = (): void => { if (current) { current.text = current.text.trim(); @@ -85,61 +100,182 @@ export function parseDecisions(content: unknown): Decision[] { current = null; } }; + for (const line of lines) { const trimmed = line.trim(); + // Track category headings (`### Heading`) const headingMatch = trimmed.match(/^###\s+(.+?)\s*$/); if (headingMatch) { flush(); category = headingMatch[1]; // Strip the full unicode-quote family so any rendering of "Claude's - // Discretion" (ASCII apostrophe, curly U+2019, U+2018, U+201A, U+201B, - // double-quote variants U+201C/D/E/F, etc.) collapses to the same key - // (review F20). + // Discretion" (ASCII apostrophe, curly U+2019 ’, U+2018 ‘, + // U+201A, U+201B, double-quote variants U+201C/D/E/F, etc.) collapses + // to the same key (FIX C + review F20). const normalized = category .toLowerCase() - .replace(/[‘’‚‛“”„‟'"`]/g, '') + .replace(/[‘’‚‛“”„‟''"`]/g, '') .trim(); inDiscretion = DISCRETION_HEADINGS.has(normalized); continue; } - const bulletMatch = line.match(bulletRe); - if (bulletMatch) { + + // Colon form: `- **D-NN[ [tags]]:** text` + const colonMatch = line.match(bulletColonRe); + if (colonMatch) { flush(); - const id = `D-${bulletMatch[1]}`; - const tags = bulletMatch[2] - ? bulletMatch[2] - .split(',') - .map((t) => t.trim().toLowerCase()) - .filter(Boolean) + const id = `D-${colonMatch[1]}`; + const tags = colonMatch[2] + ? colonMatch[2].split(',').map((t) => t.trim().toLowerCase()).filter(Boolean) : []; const trackable = !inDiscretion && !tags.some((t) => NON_TRACKABLE_TAGS.has(t)); - current = { id, text: bulletMatch[3], category, tags, trackable }; + current = { id, text: colonMatch[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. + + // Em-dash form: `- **D-NN[ [tags]] — title** body` + const emDashMatch = line.match(bulletEmDashRe); + if (emDashMatch) { flush(); + const id = `D-${emDashMatch[1]}`; + const tags = emDashMatch[2] + ? emDashMatch[2].split(',').map((t) => t.trim().toLowerCase()).filter(Boolean) + : []; + const trackable = !inDiscretion && !tags.some((t) => NON_TRACKABLE_TAGS.has(t)); + // The body (emDashMatch[3]) may be empty for the pure title form; the + // title itself is embedded in the bold run but we report the body as text + // (consistent with how the gate cares only about coverage, not title/body split). + current = { id, text: emDashMatch[3] || '', category, tags, trackable }; + continue; + } + + // Parse-miss guard (FIX B + #1343): a line that looks like a `D-NN` decision + // bullet but failed both patterns — flush, warn, and record the miss. + // parseMisses > 0 forces could-not-parse even when other decisions parsed. + if (/^\s*-\s+\*\*D-/.test(line)) { + flush(); + parseMisses += 1; 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)) { current.text += ' ' + trimmed; continue; } + // Blank line or unrelated content terminates the current decision if (trimmed === '') { flush(); } } flush(); - return out; + return { decisions: out, parseMisses }; +} + +// ─── Primary entry point: extractDecisions ──────────────────────────────────── + +/** + * Extract decisions from CONTEXT.md content with a typed outcome. + * + * Strategy (in priority order): + * 1. If the content (fence-stripped) contains `...` blocks, + * parse ONLY those blocks (canonical form; markdown-header content outside blocks + * is ignored when a block is present — existing behavior preserved). + * 2. Otherwise, look for a /decisions?/i heading and collect its section body. + * This is the T1 recall fix for #1364. + * 3. If neither is found, return outcome based on decision-shape heuristics. + */ +export function extractDecisions(content: unknown): DecisionExtraction { + if (!content || typeof content !== 'string') { + return { decisions: [], outcome: 'none-present' }; + } + + // Apply fence-stripping for block extraction (prevents example blocks inside + // ``` fences from polluting the parser — review F11). + const { text: stripped, unterminatedFence } = stripFencedCode(content); + + // ── Path 1: blocks present ────────────────────────────────────── + const taggedBlocks = extractTaggedBlocks(stripped, 'decisions'); + if (taggedBlocks.length > 0) { + const combined = taggedBlocks.join('\n\n'); + const { decisions, parseMisses } = parseDecisionLines(combined); + if (decisions.length > 0 && parseMisses === 0) { + return { decisions, outcome: 'parsed' }; + } + // FIX B: parse-misses present — could-not-parse even if some decisions extracted. + if (parseMisses > 0) { + return { decisions, outcome: 'could-not-parse' }; + } + // FIX A: Block present but 0 extracted and no parse-misses. + // Only report could-not-parse when there is genuine evidence of real decisions + // that failed to parse: a \bD- token in the block text, or an unterminated fence. + // An empty scaffold () or an all-prose block has no such + // evidence — treat as none-present so the gate passes cleanly. + const hasDecisionTokenInBlock = /\bD-[A-Za-z0-9]/m.test(combined); + if (hasDecisionTokenInBlock || unterminatedFence) { + return { decisions: [], outcome: 'could-not-parse' }; + } + return { decisions: [], outcome: 'none-present' }; + } + + // ── Path 2: markdown-header fallback (#1364 fix) ───────────────────────────── + // Use the seam's collectSection to find a /decisions?/i heading section. + // levelBounded:true → stop at next same-or-higher-level heading. + // stripFences:true → inner fences inside the section body are stripped. + const section = collectSection( + content, + (h) => /decisions?\b/i.test(h.text), + { levelBounded: true, stripFences: true }, + ); + + if (section !== null) { + const { decisions, parseMisses } = parseDecisionLines(section.body); + if (decisions.length > 0 && parseMisses === 0) { + return { decisions, outcome: 'parsed' }; + } + // FIX B: parse-misses present — could-not-parse even if some decisions extracted. + if (parseMisses > 0) { + return { decisions, outcome: 'could-not-parse' }; + } + // FIX A: Heading found but 0 extracted and no parse-misses. + // Only report could-not-parse when the section body contains a D- token. + // A heading with only prose, sub-headings, or all-discretion content + // (no trackable D- tokens) is a legitimate empty/discretion section → none-present. + const hasDecisionTokenInSection = /\bD-[A-Za-z0-9]/m.test(section.body); + if (hasDecisionTokenInSection) { + return { decisions: [], outcome: 'could-not-parse' }; + } + return { decisions: [], outcome: 'none-present' }; + } + + // ── Path 3: no blocks, no heading ──────────────────────────────────────────── + // Apply shape heuristics to distinguish none-present from could-not-parse. + // We re-use the already-computed unterminatedFence and check for D- tokens. + const hasDecisionToken = /\bD-[A-Za-z0-9]/m.test(stripped); + if (unterminatedFence || hasDecisionToken) { + return { decisions: [], outcome: 'could-not-parse' }; + } + + return { decisions: [], outcome: 'none-present' }; +} + +// ─── parseDecisions: thin delegate (backwards-compatible entry point) ───────── + +/** + * Parse trackable decisions from CONTEXT.md content. + * + * Thin delegate over extractDecisions — callers receive the decisions array + * exactly as before; nothing breaks. Use extractDecisions directly when the + * outcome enum is needed (e.g. for the fail-loud gate logic). + * + * Returns ALL D-NN decisions found (including non-trackable ones, with + * `trackable: false`). Callers that only want the gate-enforced decisions + * should filter `.filter(d => d.trackable)`. + */ +export function parseDecisions(content: unknown): Decision[] { + return extractDecisions(content).decisions; } diff --git a/src/gap-checker.cts b/src/gap-checker.cts index aa1f4839f..b75fb4cb1 100644 --- a/src/gap-checker.cts +++ b/src/gap-checker.cts @@ -27,7 +27,7 @@ const { escapeRegex } = phaseId; // eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspace = require('./planning-workspace.cjs'); const { planningPaths, planningDir, findContextMdIn } = planningWorkspace; -import { parseDecisions } from './decisions.cjs'; +import { parseDecisions, extractDecisions } from './decisions.cjs'; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -235,7 +235,10 @@ function runGapAnalysis(cwd: string, phaseDir: string, options: RunGapAnalysisOp const ctxFile = findContextMdIn(phaseDirFiles); const ctxPath = ctxFile ? path.join(absPhaseDir, ctxFile) : null; const ctxMd = ctxPath ? fs.readFileSync(ctxPath, 'utf-8') : ''; - const dItems: DecisionItem[] = parseDecisions(ctxMd).map(d => ({ ...d, source: 'CONTEXT.md' })); + + // Use extractDecisions so gap-checker can distinguish could-not-parse from none-present. + const ctxExtraction = extractDecisions(ctxMd); + const dItems: DecisionItem[] = ctxExtraction.decisions.map(d => ({ ...d, source: 'CONTEXT.md' })); const items: Item[] = [...reqItems, ...dItems]; @@ -250,6 +253,42 @@ function runGapAnalysis(cwd: string, phaseDir: string, options: RunGapAnalysisOp } } catch { /* unreadable */ } + // FIX D (#1365): surface decision could-not-parse independently of whether + // requirements items exist. Without this, a could-not-parse on decisions is + // silently masked whenever REQUIREMENTS.md has ≥1 item — the mismatch must + // appear in the report regardless of the requirements row count. + if (ctxExtraction.outcome === 'could-not-parse') { + const mismatchMsg = '## Post-Planning Gap Analysis\n\nextracted 0 of N — possible format mismatch in CONTEXT.md decisions block.\n'; + // If there are also requirement items, include them in the return with the + // mismatch summary appended, so the caller still sees requirement coverage. + if (items.length > 0) { + const rows = sortRows([ + ...detectCoverage(items, planText), + ...ghostReqIds.map(id => ({ source: 'REQUIREMENTS.md', item: id, status: 'Missing from REQUIREMENTS.md' })), + ]); + const covered = rows.filter(r => r.status === 'Covered').length; + const uncovered = rows.length - covered; + const coverageSummary = uncovered === 0 + ? `✓ All ${rows.length} items covered by plans` + : `⚠ ${uncovered} of ${rows.length} items not covered by any plan`; + return { + enabled: true, + rows, + table: formatGapTable(rows) + '\n' + coverageSummary + '\n\n' + mismatchMsg, + summary: coverageSummary + '; extracted 0 of N — possible format mismatch', + counts: { total: rows.length, covered, uncovered }, + }; + } + return { + enabled: true, + rows: [], + table: mismatchMsg, + summary: 'extracted 0 of N — possible format mismatch', + counts: { total: 0, covered: 0, uncovered: 0 }, + }; + } + + // #1365: if no items at all, surface a clean no-check message. if (items.length === 0) { return { enabled: true, diff --git a/tests/decisions.test.cjs b/tests/decisions.test.cjs new file mode 100644 index 000000000..362f57a26 --- /dev/null +++ b/tests/decisions.test.cjs @@ -0,0 +1,793 @@ +'use strict'; + +/** + * decisions.test.cjs — regression tests for parseDecisions / extractDecisions + * and the check.decision-coverage-plan gate fail-loud behavior. + * + * Bug #1364: parseDecisions returns [] when decisions appear under markdown headers + * (## Locked decisions / ## Implementation decisions) instead of a + * ... block. Also, em-dash bullets + * '- **D-1 — title** body' are dropped as unparseable. + * + * Bug #1365: check.decision-coverage-plan silently returns passed:true when + * CONTEXT.md is decision-shaped (has block or D- tokens) but 0 + * decisions are extracted — gate now returns passed:false with format-mismatch + * reason (could-not-parse outcome). + * + * Parser QA matrix (CONTRIBUTING.md 'Parser and project-file inputs'): + * - CRLF newlines + * - Unicode in a heading + * - Decisions-looking heading inside a fenced code block (must be ignored) + * - Both bullet forms: colon ('- **D-1:** ...') and em-dash ('- **D-1 — ...**') + * - Genuinely empty / no-decisions case (still []) + * - Pre-existing block behaviour is unaffected (regression guard) + */ + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const { parseDecisions, extractDecisions } = require('../gsd-core/bin/lib/decisions.cjs'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +// ─── Regression #1364: markdown-header fallback ─────────────────────────────── + +describe('parseDecisions — markdown header fallback (#1364)', () => { + test('extracts D-NN from ## Locked decisions header (em-dash bullets)', () => { + const md = '## Locked decisions\n- **D-1 — a** x\n- **D-2 — b** y\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual( + ds.map(d => d.id), + ['D-1', 'D-2'], + 'should extract D-1 and D-2 from em-dash bullets under markdown header' + ); + }); + + test('extracts D-NN from ## Implementation decisions header (colon bullets)', () => { + const md = '## Implementation decisions\n- **D-01:** Use OAuth 2.0\n- **D-02:** Redis sessions\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-01', 'D-02']); + assert.strictEqual(ds[0].text, 'Use OAuth 2.0'); + }); + + test('extracts D-NN from ### Decisions header (mixed bullets)', () => { + const md = '### Decisions\n- **D-1:** colon form\n- **D-2 — em-dash form** body text\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-1', 'D-2']); + }); + + test('extracts from header with case variation (## DECISIONS)', () => { + const md = '## DECISIONS\n- **D-10:** uppercase heading\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-10']); + }); + + test('extracts from heading with Unicode in surrounding text (## \u{1F512} Locked decisions)', () => { + // Unicode chars before "decisions" must not break the heading matcher. + const md = '## \u{1F512} Locked decisions\n- **D-3 — unicode heading** value\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-3']); + }); + + test('CRLF newlines work for markdown-header path', () => { + const md = '## Locked decisions\r\n- **D-5:** crlf bullet\r\n- **D-6 — em dash** crlf em\r\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-5', 'D-6']); + }); + + test('decisions-looking heading inside a fenced code block is ignored', () => { + const md = [ + '```', + '## Locked decisions', + '- **D-99:** fake', + '```', + '', + '## Real decisions', + '- **D-1:** real', + ].join('\n'); + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-1']); + }); + + test('generic prose heading does not produce false positives', () => { + const md = '## Context\n- some bullet\n\n## Architecture\n- another bullet\n'; + assert.deepStrictEqual(parseDecisions(md), []); + }); + + test('no decisions anywhere returns [] (no false positives)', () => { + assert.deepStrictEqual(parseDecisions('## Locked decisions\n\nNo bullets here.\n'), []); + }); + + test('content with no decisions heading and no block returns []', () => { + assert.deepStrictEqual(parseDecisions('# Just a title\nsome prose\n'), []); + }); +}); + +// ─── Regression #1364: em-dash bullet inside existing block ─────── + +describe('parseDecisions — em-dash bullet form inside block (#1364)', () => { + test('em-dash bullet is parsed inside a block', () => { + const md = '\n- **D-1 — my title** body text\n\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-1']); + assert.ok(ds[0].text.length > 0, 'text must not be empty'); + }); + + test('em-dash bullet with alphanumeric ID is parsed', () => { + const md = '\n- **D-INFRA-01 — infra decision** body\n\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-INFRA-01']); + }); +}); + +// ─── Regression guard: pre-existing block behaviour unchanged ───── + +describe('parseDecisions — existing block still works (#1364 guard)', () => { + test('colon form inside block still parses', () => { + const md = '\n- **D-1:** colon form\n\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-1']); + assert.strictEqual(ds[0].text, 'colon form'); + }); + + test('multiple D-NN in block with categories still works', () => { + const md = `\n### Auth\n- **D-01:** OAuth\n### Storage\n- **D-02:** Postgres\n\n`; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-01', 'D-02']); + assert.strictEqual(ds[0].category, 'Auth'); + }); + + test('D-IDs outside the block are still ignored when a block is present', () => { + const md = '- **D-99:** outside\n\n- **D-01:** inside\n\n- **D-77:** after\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-01']); + }); + + test('empty / null / undefined still return []', () => { + assert.deepStrictEqual(parseDecisions(''), []); + assert.deepStrictEqual(parseDecisions(null), []); + assert.deepStrictEqual(parseDecisions(undefined), []); + }); +}); + +// ─── extractDecisions outcome: 'none-present' and 'could-not-parse' ────────── + +describe('extractDecisions — typed outcome (#1364 + #1365)', () => { + test('returns outcome:parsed with decisions array when block present', () => { + const md = '\n- **D-1:** OAuth 2.0\n\n'; + const result = extractDecisions(md); + assert.strictEqual(result.outcome, 'parsed'); + assert.strictEqual(result.decisions.length, 1); + assert.strictEqual(result.decisions[0].id, 'D-1'); + }); + + test('returns outcome:parsed for markdown-header path', () => { + const md = '## Locked decisions\n- **D-2:** use Redis\n'; + const result = extractDecisions(md); + assert.strictEqual(result.outcome, 'parsed'); + assert.strictEqual(result.decisions.length, 1); + }); + + test('returns outcome:none-present for genuinely empty content', () => { + const result = extractDecisions('# Just a title\nsome prose without decisions\n'); + assert.strictEqual(result.outcome, 'none-present'); + assert.deepStrictEqual(result.decisions, []); + }); + + test('returns outcome:none-present for empty string', () => { + const result = extractDecisions(''); + assert.strictEqual(result.outcome, 'none-present'); + }); + + test('returns outcome:could-not-parse when block present but yields 0 decisions', () => { + // A block with no parseable bullets is decision-shaped + const md = '\n\nJust prose, no D-NN bullets\n\n\n'; + const result = extractDecisions(md); + assert.strictEqual(result.outcome, 'could-not-parse'); + assert.deepStrictEqual(result.decisions, []); + }); + + test('returns outcome:could-not-parse when D- token present but no parseable decisions', () => { + // Content references D-01 in prose but it's malformed — not in a parseable bullet + const md = '# Context\n\nSee also D-01 for background. No block, no heading.\n'; + const result = extractDecisions(md); + assert.strictEqual(result.outcome, 'could-not-parse'); + assert.deepStrictEqual(result.decisions, []); + }); + + test('returns outcome:could-not-parse when /decisions?/i heading present but 0 decisions extracted', () => { + // Header present but no actual D-NN bullets under it + const md = '## Locked decisions\n\nNo D-NN bullets here, just prose.\n'; + const result = extractDecisions(md); + assert.strictEqual(result.outcome, 'could-not-parse'); + assert.deepStrictEqual(result.decisions, []); + }); + + test('returns outcome:none-present for generic prose with no decision signals', () => { + // No block, no /decisions?/i heading, no \bD- token — genuinely no decisions + const md = '## Context\n\nSome architecture notes.\n\n## Goals\n\nBe fast.\n'; + const result = extractDecisions(md); + assert.strictEqual(result.outcome, 'none-present'); + }); + + test('parseDecisions delegates correctly (thin wrapper)', () => { + // parseDecisions is a thin delegate that returns extractDecisions().decisions + const md = '\n- **D-1:** foo\n\n'; + const fromExtract = extractDecisions(md).decisions; + const fromParse = parseDecisions(md); + assert.deepStrictEqual(fromParse, fromExtract); + }); +}); + +// ─── QA matrix for parser correctness ──────────────────────────────────────── + +describe('parseDecisions — parser QA matrix', () => { + test('### category headings inside a decisions block set category', () => { + const md = '\n### Auth\n- **D-01:** OAuth 2.0\n### Storage\n- **D-02:** Postgres\n'; + const ds = parseDecisions(md); + assert.strictEqual(ds[0].category, 'Auth'); + assert.strictEqual(ds[1].category, 'Storage'); + }); + + test("### Claude's Discretion section sets trackable:false", () => { + const md = "\n### Claude's Discretion\n- **D-01:** internal\n"; + const ds = parseDecisions(md); + assert.strictEqual(ds[0].trackable, false); + }); + + test('[informational] tag sets trackable:false', () => { + const md = '\n- **D-01 [informational]:** ref only\n'; + const ds = parseDecisions(md); + assert.strictEqual(ds[0].trackable, false); + }); + + test('[deferred] tag sets trackable:false', () => { + const md = '\n- **D-01 [deferred]:** not yet\n'; + const ds = parseDecisions(md); + assert.strictEqual(ds[0].trackable, false); + }); + + test('continuation lines append to text (tab-indented)', () => { + const md = '\n- **D-01:** first line\n\tcontinued here\n'; + const ds = parseDecisions(md); + assert.ok(ds[0].text.includes('first line'), 'must include first line'); + assert.ok(ds[0].text.includes('continued here'), 'must include continuation'); + }); + + test('CRLF inside a block still parses', () => { + const md = '\r\n- **D-01:** crlf decision\r\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-01']); + }); + + test('fenced code block inside document does not pollute decisions', () => { + const md = [ + '```', + '', + '- **D-99:** fake in fence', + '', + '```', + '', + '', + '- **D-01:** real', + '', + ].join('\n'); + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-01']); + }); + + test('alphanumeric IDs (D-INFRA-01) are accepted', () => { + const md = '\n- **D-INFRA-01:** infra call\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-INFRA-01']); + }); + + test('em-dash bullet form with tags still sets tags', () => { + const md = '\n- **D-01 [informational] — title** body\n'; + const ds = parseDecisions(md); + assert.deepStrictEqual(ds.map(d => d.id), ['D-01']); + assert.ok(ds[0].tags.includes('informational')); + }); +}); + +// ─── #1365: fail-loud gate — check.decision-coverage-plan ──────────────────── + +/** + * Gate-level tests for the could-not-parse fail-loud behavior (#1365). + * These exercise cmdDecisionCoveragePlan via the real CLI (check decision-coverage-plan). + * + * Naming: check.decision-coverage-plan is invoked as `query check.decision-coverage-plan`. + * The gate lives in check-command-router.cts; outcome flows from decisions.cts extractDecisions. + */ + +function writeContextFile(phaseDir, content) { + fs.writeFileSync(path.join(phaseDir, 'CONTEXT.md'), content); +} + +function writePlanFile(phaseDir, name, body) { + fs.writeFileSync(path.join(phaseDir, `${name}-PLAN.md`), body); +} + +function writePlanningConfig(planningDir, config) { + fs.writeFileSync(path.join(planningDir, 'config.json'), JSON.stringify(config)); +} + +function runDecisionCoveragePlan(phaseDir, contextPath, cwd) { + return runGsdTools(['query', 'check.decision-coverage-plan', phaseDir, contextPath], cwd); +} + +describe('check.decision-coverage-plan — fail-loud on could-not-parse (#1365)', () => { + let tmpDir; + let planningDir; + let phaseDir; + + beforeEach(() => { + tmpDir = createTempProject('gsd-1365-'); + planningDir = path.join(tmpDir, '.planning'); + phaseDir = path.join(planningDir, 'phases', '01-init'); + fs.mkdirSync(phaseDir, { recursive: true }); + }); + + afterEach(() => cleanup(tmpDir)); + + test('decision-shaped CONTEXT.md with block but 0 parseable decisions → passed:false (not silent skip)', () => { + // #1365 bug: gate used to return passed:true/skipped for this case. + writeContextFile(phaseDir, [ + '# Phase 1', + '', + '', + '', + 'See the ADR for architecture choices. No D-NN bullets here.', + '', + '', + ].join('\n')); + writePlanFile(phaseDir, '01', '# Plan\n## Objective\nImplement feature.\n'); + + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir); + const raw = result.output || ''; + const parsed = JSON.parse(raw); + assert.strictEqual(parsed.passed, false, + `Gate must return passed:false for decision-shaped but 0-extracted content. Got: ${JSON.stringify(parsed)}`); + const msg = (parsed.message || parsed.reason || '').toLowerCase(); + assert.ok( + msg.includes('format') || msg.includes('mismatch') || msg.includes('could not parse') || msg.includes('parse'), + `Message must mention format mismatch or parsing issue. Got: "${parsed.message}"` + ); + }); + + test('CONTEXT.md with \\bD- token in prose but no parseable decisions → passed:false', () => { + writeContextFile(phaseDir, [ + '# Phase 1 Context', + '', + 'See D-01 for the authentication decision and D-02 for storage.', + 'These are just prose references, not structured decisions.', + ].join('\n')); + writePlanFile(phaseDir, '01', '# Plan\nRef D-01.\n'); + + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir); + const raw = result.output || ''; + const parsed = JSON.parse(raw); + assert.strictEqual(parsed.passed, false, + `Gate must return passed:false for D-token-but-no-parseable content. Got: ${JSON.stringify(parsed)}`); + }); + + test('genuinely empty CONTEXT.md (no decision signals) → passed:true/skipped (no false alarm)', () => { + writeContextFile(phaseDir, [ + '# Phase 1 Context', + '', + '## Goals', + 'Build the feature.', + '', + '## Architecture', + 'Use Node.js and TypeScript.', + ].join('\n')); + writePlanFile(phaseDir, '01', '# Plan\nImplement the feature.\n'); + + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir); + const raw = result.output || ''; + const parsed = JSON.parse(raw); + assert.strictEqual(parsed.passed, true, + `Gate must NOT false-alarm on genuinely empty content. Got: ${JSON.stringify(parsed)}`); + assert.strictEqual(parsed.skipped, true, + `Gate must skip when there are no decisions. Got: ${JSON.stringify(parsed)}`); + }); + + test('well-formed CONTEXT.md with real decisions all covered → passed:true (normal case)', () => { + writeContextFile(phaseDir, [ + '# Context', + '', + '', + '### Implementation', + '- **D-01:** Use OAuth 2.0 for authentication', + '', + ].join('\n')); + writePlanFile(phaseDir, '01', '# Plan\n## Must Haves\n- D-01: Implement OAuth 2.0\n'); + + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir); + const raw = result.output || ''; + const parsed = JSON.parse(raw); + assert.strictEqual(parsed.passed, true, + `Real decisions covered → must pass. Got: ${JSON.stringify(parsed)}`); + assert.strictEqual(parsed.skipped, false); + }); + + test('well-formed CONTEXT.md with decisions heading (markdown-header) all covered → passed:true', () => { + // After #1364 fix: markdown-header decisions are now extractable and coverable + writeContextFile(phaseDir, [ + '# Context', + '', + '## Implementation decisions', + '', + '- **D-01:** Use Redis for caching', + ].join('\n')); + writePlanFile(phaseDir, '01', '# Plan\n## Must Haves\n- D-01: Implement Redis caching\n'); + + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir); + const raw = result.output || ''; + const parsed = JSON.parse(raw); + assert.strictEqual(parsed.passed, true, + `Markdown-header decisions covered → must pass. Got: ${JSON.stringify(parsed)}`); + assert.strictEqual(parsed.skipped, false); + assert.strictEqual(parsed.total, 1); + assert.strictEqual(parsed.covered, 1); + }); + + test('CONTEXT.md missing → passed:true/skipped (unchanged behavior)', () => { + const contextPath = path.join(phaseDir, 'NONEXISTENT-CONTEXT.md'); + const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir); + const raw = result.output || ''; + const parsed = JSON.parse(raw); + assert.strictEqual(parsed.passed, true); + assert.strictEqual(parsed.skipped, true); + }); + + test('gate disabled by config → passed:true/skipped (unchanged behavior)', () => { + writeContextFile(phaseDir, '\nNo D-NN bullets\n'); + writePlanningConfig(planningDir, { workflow: { context_coverage_gate: false } }); + + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir); + const raw = result.output || ''; + const parsed = JSON.parse(raw); + assert.strictEqual(parsed.passed, true); + assert.strictEqual(parsed.skipped, true); + }); +}); + +describe('check.decision-coverage-plan — boundary/threshold tests (#1365)', () => { + let tmpDir; + let planningDir; + let phaseDir; + + beforeEach(() => { + tmpDir = createTempProject('gsd-1365-bva-'); + planningDir = path.join(tmpDir, '.planning'); + phaseDir = path.join(planningDir, 'phases', '01-init'); + fs.mkdirSync(phaseDir, { recursive: true }); + }); + + afterEach(() => cleanup(tmpDir)); + + test('exactly 1 decision extracted (limit == 1) → not could-not-parse', () => { + writeContextFile(phaseDir, '\n- **D-01:** single decision\n'); + writePlanFile(phaseDir, '01', '# Plan\n## Objective\nRef D-01.\n'); + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir); + const parsed = JSON.parse(result.output || ''); + assert.strictEqual(parsed.passed, true); + assert.strictEqual(parsed.skipped, false); + assert.strictEqual(parsed.total, 1); + assert.strictEqual(parsed.covered, 1); + }); + + test('FIX A: empty scaffold (limit - 1 == 0, no D- token) → none-present → passed:true/skipped (NOT blocked)', () => { + // FIX A: An empty scaffold has no D- tokens → none-present, gate passes. + // REGRESSION: previously returned could-not-parse → passed:false, blocking legitimate phases. + writeContextFile(phaseDir, '\n\n'); + writePlanFile(phaseDir, '01', '# Plan\nSome plan.\n'); + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir); + const parsed = JSON.parse(result.output || ''); + assert.strictEqual(parsed.passed, true, + `Empty scaffold → none-present → passed:true. Got: ${JSON.stringify(parsed)}`); + assert.strictEqual(parsed.skipped, true, + `Empty scaffold → none-present → skipped:true. Got: ${JSON.stringify(parsed)}`); + }); + + test('FIX A: block with D- token in prose (not a bullet) → could-not-parse → passed:false', () => { + // If the block contains a D- token but not as a parseable bullet → could-not-parse + writeContextFile(phaseDir, '\nD-01 is mentioned in prose but not as a bullet.\n'); + writePlanFile(phaseDir, '01', '# Plan\nSome plan.\n'); + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir); + const parsed = JSON.parse(result.output || ''); + assert.strictEqual(parsed.passed, false, + `D-token-in-prose → could-not-parse → passed:false. Got: ${JSON.stringify(parsed)}`); + }); +}); + +// ─── FIX A regressions: tighten could-not-parse (empty scaffold / none-present) ─── + +describe('FIX A: tighten could-not-parse — empty scaffolds must not block (#1372)', () => { + test('empty scaffold → none-present (gate clean)', () => { + // REGRESSION: previously returned could-not-parse, blocking legitimate phases + const result = extractDecisions(''); + assert.strictEqual(result.outcome, 'none-present', + `Empty scaffold must be none-present. Got: ${result.outcome}`); + assert.deepStrictEqual(result.decisions, []); + }); + + test('## Decisions heading with prose only, no D- bullets → none-present', () => { + // A heading with only prose and no D- tokens is not decision-shaped + const md = '## Decisions\n\nArchitecture is handled via ADR-001.\n\nSee docs.\n'; + const result = extractDecisions(md); + assert.strictEqual(result.outcome, 'none-present', + `Prose-only decisions heading must be none-present. Got: ${result.outcome}`); + }); + + test('all-discretion block (### Claude’s Discretion, no D- bullets) → none-present', () => { + // An all-discretion block with no D- tokens is a legitimate empty context + const curlySingle = '’'; + const md = '\n### Claude' + curlySingle + 's Discretion\n\nAll implementation details left to Claude.\n'; + const result = extractDecisions(md); + assert.strictEqual(result.outcome, 'none-present', + `All-discretion block with no D- bullets must be none-present. Got: ${result.outcome}`); + }); + + test(' block with D- token in prose (not bullet) → still could-not-parse', () => { + // A D- token that is NOT in a parseable bullet format still signals format mismatch + const md = '\nSee D-01 for the decision.\n'; + const result = extractDecisions(md); + assert.strictEqual(result.outcome, 'could-not-parse', + `D-token in block prose must be could-not-parse. Got: ${result.outcome}`); + }); +}); + +// ─── FIX B regressions: parse-miss must fail loud ──────────────────────────── + +describe('FIX B: parse-miss on malformed D-NN bullet → could-not-parse (#1372)', () => { + test('valid D-01 + malformed D-02 bullet → outcome could-not-parse (not silent pass)', () => { + // REGRESSION: previously returned outcome:parsed (silently dropped D-02) + const md = '\n- **D-01:** Use OAuth 2.0\n- **D-02 malformed no colon or dash** text\n'; + const result = extractDecisions(md); + assert.strictEqual(result.outcome, 'could-not-parse', + `Mixed valid+malformed must be could-not-parse. Got: ${result.outcome}`); + }); + + test('valid D-01 + malformed D-02 bullet → gate passed:false (not silent skip)', () => { + // Gate-level regression: a parse-miss must propagate as passed:false + // Uses extractDecisions directly to confirm gate-layer behavior + const md = '\n- **D-01:** Use OAuth 2.0\n- **D-02 malformed no colon or dash** text\n'; + const result = extractDecisions(md); + // The check-command-router uses outcome === 'could-not-parse' && decisions.length where + // trackable.length === 0 → passed:false. Confirm outcome propagates correctly. + assert.strictEqual(result.outcome, 'could-not-parse'); + // D-01 was parsed (it was valid); the result still contains it for context + // but the overall outcome is could-not-parse because of the parse-miss on D-02. + assert.ok(result.decisions.some(d => d.id === 'D-01'), + `D-01 (valid) must still be in decisions. Got: ${JSON.stringify(result.decisions)}`); + }); + + test('only malformed D-NN bullet (no valid ones) → could-not-parse', () => { + const md = '\n- **D-01 no colon no dash here** just text\n'; + const result = extractDecisions(md); + assert.strictEqual(result.outcome, 'could-not-parse', + `Only-malformed-bullet must be could-not-parse. Got: ${result.outcome}`); + }); +}); + +// ─── FIX B gate-level: parse-miss silently swallowed when covered decision exists ─ + +describe('FIX B gate-level: parse-miss → passed:false regardless of covered decisions (#1365)', () => { + let tmpDir; + let planningDir; + let phaseDir; + + beforeEach(() => { + tmpDir = createTempProject('gsd-1365-fixb-'); + planningDir = path.join(tmpDir, '.planning'); + phaseDir = path.join(planningDir, 'phases', '01-init'); + fs.mkdirSync(phaseDir, { recursive: true }); + }); + + afterEach(() => cleanup(tmpDir)); + + test('FAIL-FIRST: valid D-01 covered + malformed D-02 → gate must return passed:false (parse-miss wins)', () => { + // CONTEXT.md: D-01 is valid colon-form; D-02 has no colon and no em-dash → parse-miss + // PLAN.md: covers D-01 via ## Must Haves so coverage of D-01 would pass on its own. + // Before fix: decisions.length === 1 (D-01), outcome === 'could-not-parse' → + // guard `decisions.length === 0 && outcome === 'could-not-parse'` is FALSE → + // gate proceeds to coverage → D-01 is covered → passed:true [BUG] + // After fix: outcome === 'could-not-parse' fires regardless of decisions.length → + // gate returns passed:false with reason:'could-not-parse' [CORRECT] + writeContextFile(phaseDir, [ + '# Phase 1 Context', + '', + '', + '### Implementation', + '- **D-01:** use JWT tokens', + '- **D-02** ratio 3:1', + '', + ].join('\n')); + // D-02 bullet has no colon and no em-dash → parse-miss → outcome:'could-not-parse' + // but D-01 is in decisions with trackable:true + + // Plan covers D-01 explicitly via ## Must Haves (DESIGNATED_HEADINGS_RE match) + writePlanFile(phaseDir, '01', [ + '# Plan', + '', + '## Must Haves', + '', + '- D-01: implement JWT token issuance and validation', + ].join('\n')); + + // Pre-check: confirm extractDecisions outcome so we know what the gate is receiving + const extraction = extractDecisions([ + '', + '### Implementation', + '- **D-01:** use JWT tokens', + '- **D-02** ratio 3:1', + '', + ].join('\n')); + assert.strictEqual(extraction.outcome, 'could-not-parse', + `Pre-check: extractDecisions must return could-not-parse. Got: ${extraction.outcome}`); + assert.ok(extraction.decisions.some(d => d.id === 'D-01'), + `Pre-check: D-01 must be in decisions (coverage would pass for D-01 alone). Got: ${JSON.stringify(extraction.decisions)}`); + assert.strictEqual(extraction.decisions.filter(d => d.trackable).length, 1, + 'Pre-check: exactly 1 trackable decision (D-01) — confirms decisions.length === 1 path'); + + // Gate call: with the old guard `decisions.length === 0 && outcome === 'could-not-parse'` + // this would be skipped (length is 1) and coverage would find D-01 covered → passed:true. + // With the fix this must return passed:false. + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir); + const parsed = JSON.parse(result.output || ''); + assert.strictEqual(parsed.passed, false, + `Gate must return passed:false when parse-miss present, even if covered decisions exist. Got: ${JSON.stringify(parsed)}`); + assert.strictEqual(parsed.reason, 'could-not-parse', + `Gate must report reason:'could-not-parse'. Got: ${JSON.stringify(parsed)}`); + // Message must indicate a format/parse problem (not a coverage gap on D-01) + const msg = (parsed.message || '').toLowerCase(); + assert.ok( + msg.includes('could not') || msg.includes('format') || msg.includes('mismatch') || msg.includes('parse'), + `Message must indicate parse/format issue, not D-01 coverage gap. Got: "${parsed.message}"` + ); + // Confirm D-01 is NOT in uncovered[] — the failure is parse-miss, not a coverage gap + assert.deepStrictEqual(parsed.uncovered, [], + `uncovered must be empty (D-01 is covered; failure is parse-miss). Got: ${JSON.stringify(parsed.uncovered)}`); + }); + + test('verify-side: valid D-01 covered + malformed D-02 → verify advisory surfaces could-not-parse', () => { + // Same scenario but via decision-coverage-verify (non-blocking advisory) + writeContextFile(phaseDir, [ + '# Phase 1 Context', + '', + '', + '- **D-01:** use JWT tokens', + '- **D-02** ratio 3:1', + '', + ].join('\n')); + writePlanFile(phaseDir, '01', '# Plan\n\n## Must Haves\n\n- D-01: implement JWT\n'); + + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + const result = runGsdTools( + ['query', 'check.decision-coverage-verify', phaseDir, contextPath], + tmpDir + ); + const parsed = JSON.parse(result.output || ''); + assert.strictEqual(parsed.reason, 'could-not-parse', + `Verify must surface could-not-parse reason. Got: ${JSON.stringify(parsed)}`); + assert.strictEqual(parsed.blocking, false, + `Verify is always non-blocking. Got: ${JSON.stringify(parsed)}`); + }); +}); + +// ─── FIX C regressions: curly-quote Claude's Discretion ─────────────────────── + +describe('FIX C: curly-quote Claude’s Discretion → trackable:false (#1372)', () => { + test('### Claude’s Discretion (U+2019 curly apostrophe) sets trackable:false', () => { + // REGRESSION: curly apostrophe was not stripped from category, so + // "claudes discretion" key was not in DISCRETION_HEADINGS → trackable:true + const curlySingle = '’'; + const md = '\n### Claude' + curlySingle + 's Discretion\n- **D-01:** internal decision\n'; + const ds = parseDecisions(md); + assert.strictEqual(ds.length, 1, 'one decision must be parsed'); + assert.strictEqual(ds[0].trackable, false, + `Curly-apostrophe discretion heading must yield trackable:false. Got trackable:${ds[0].trackable}`); + }); + + test('### Claude‘s Discretion (U+2018 opening quote) sets trackable:false', () => { + const openSingle = '‘'; + const md = '\n### Claude' + openSingle + 's Discretion\n- **D-01:** internal decision\n'; + const ds = parseDecisions(md); + assert.strictEqual(ds.length, 1); + assert.strictEqual(ds[0].trackable, false, + `Open-single-quote discretion heading must yield trackable:false. Got trackable:${ds[0].trackable}`); + }); + + test('[folded] tag sets trackable:false (coverage gap fix)', () => { + // Previously NON_TRACKABLE_TAGS included 'folded' but had no dedicated test + const md = '\n- **D-01 [folded]:** folded decision\n'; + const ds = parseDecisions(md); + assert.strictEqual(ds.length, 1); + assert.strictEqual(ds[0].trackable, false, + `[folded] tag must yield trackable:false. Got trackable:${ds[0].trackable}`); + assert.ok(ds[0].tags.includes('folded'), 'tags must include "folded"'); + }); +}); + +// ─── FIX D regressions: gap-checker surfaces decision parse failure independently ─ + +describe('FIX D: gap-checker surfaces decision could-not-parse even when requirements exist (#1372)', () => { + const { runGapAnalysis } = require('../gsd-core/bin/lib/gap-checker.cjs'); + + let tmpDir; + let planningDir; + let phaseDir; + + beforeEach(() => { + tmpDir = createTempProject('gsd-1372-fixd-'); + planningDir = path.join(tmpDir, '.planning'); + phaseDir = path.join(planningDir, 'phases', '01-init'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(planningDir, 'config.json'), JSON.stringify({})); + }); + + afterEach(() => cleanup(tmpDir)); + + test('REQUIREMENTS.md with 1 req + unparseable CONTEXT.md → gap report includes format-mismatch signal', () => { + // REGRESSION: previously the could-not-parse signal was silently masked + // inside `if (items.length === 0)` — when requirements existed, it never fired. + const reqPath = path.join(planningDir, 'REQUIREMENTS.md'); + fs.writeFileSync(reqPath, '- [ ] **REQ-01** Some requirement\n'); + + const ctxMd = '\nSome prose about decisions but no D-NN bullets.\n\n'; + fs.writeFileSync(path.join(phaseDir, 'CONTEXT.md'), ctxMd); + fs.writeFileSync(path.join(phaseDir, '01-PLAN.md'), '# Plan\nREQ-01 is covered here.\n'); + + const result = runGapAnalysis(tmpDir, phaseDir); + assert.ok( + result.summary.includes('format mismatch') || result.summary.includes('possible format'), + `Summary must mention format mismatch. Got: "${result.summary}"` + ); + assert.ok( + result.table.includes('format mismatch') || result.table.includes('possible format'), + `Table must include format mismatch note. Got: "${result.table}"` + ); + }); + + test('no REQUIREMENTS.md + unparseable CONTEXT.md → gap report includes format-mismatch signal', () => { + // Pre-existing behavior (items.length === 0 path) must still work + const ctxMd = '\nSome prose about decisions but no D-NN bullets.\n\n'; + fs.writeFileSync(path.join(phaseDir, 'CONTEXT.md'), ctxMd); + fs.writeFileSync(path.join(phaseDir, '01-PLAN.md'), '# Plan\nSome plan.\n'); + + const result = runGapAnalysis(tmpDir, phaseDir); + assert.ok( + result.summary.includes('format mismatch') || result.summary.includes('possible format'), + `Summary must mention format mismatch. Got: "${result.summary}"` + ); + }); + + test('REQUIREMENTS.md with 1 req + valid CONTEXT.md → no mismatch signal (clean path)', () => { + // Ensure the fix does not introduce false positives on valid input + const reqPath = path.join(planningDir, 'REQUIREMENTS.md'); + fs.writeFileSync(reqPath, '- [ ] **REQ-01** Some requirement\n'); + + const ctxMd = '\n- **D-01:** Use OAuth 2.0\n\n'; + fs.writeFileSync(path.join(phaseDir, 'CONTEXT.md'), ctxMd); + fs.writeFileSync(path.join(phaseDir, '01-PLAN.md'), '# Plan\nREQ-01 is covered. D-01 is covered.\n'); + + const result = runGapAnalysis(tmpDir, phaseDir); + assert.ok( + !result.summary.includes('format mismatch') && !result.summary.includes('possible format'), + `Valid input must NOT show format mismatch. Got: "${result.summary}"` + ); + }); +});