diff --git a/.changeset/eager-wolves-bark.md b/.changeset/eager-wolves-bark.md new file mode 100644 index 000000000..845ccbe03 --- /dev/null +++ b/.changeset/eager-wolves-bark.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4889 +--- +**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/check-command-router.cts b/src/check-command-router.cts index 79dcb81b0..e539ed42b 100644 --- a/src/check-command-router.cts +++ b/src/check-command-router.cts @@ -292,11 +292,12 @@ function buildVerifyMessage(notHonored: UncoveredItem[]): string { ].join('\n'); } -function loadDecisionExtraction(contextPath: string): { trackable: Decision[]; outcome: 'parsed' | 'none-present' | 'could-not-parse' } { +function loadDecisionExtraction(contextPath: string): { trackable: Decision[]; outcome: 'parsed' | 'none-present' | 'could-not-parse'; unreadableIds: string[] } { const extraction = extractDecisions(readIfExists(contextPath)); return { trackable: extraction.decisions.filter((d) => d.trackable), outcome: extraction.outcome, + unreadableIds: extraction.unreadableIds ?? [], }; } @@ -347,23 +348,48 @@ function cmdDecisionCoveragePlan(projectDir: string, args: string[], raw: boolea output({ passed: true, skipped: true, reason: 'CONTEXT.md missing', total: 0, covered: 0, uncovered: [], message: 'No CONTEXT.md - nothing to check.' }, raw, undefined); return; } + // #4794: a NON-FILE path (a directory — the adjacent same-looking positional + // swapped, the issue's repro 2) is a caller error like #2770's empty argument: + // fs.existsSync is true, the read yields nothing, and the gate used to + // certify passed:true on a phase full of decisions. Fail closed, naming it. + // The stat is wrapped: a path that vanishes between existsSync and statSync + // (or any stat failure) must answer the SAME fail-closed JSON, never a throw. + let contextIsFile = false; + let contextKind = 'non-file entry'; + try { + const st = fs.statSync(contextPath); + contextIsFile = st.isFile(); + if (st.isDirectory()) contextKind = 'directory'; + } catch { + contextIsFile = false; + contextKind = 'unreadable path'; + } + if (!contextIsFile) { + output({ passed: false, skipped: false, reason: 'context path is not a file', total: null, covered: null, message: `Decision coverage gate: the context path "${contextArg}" is not a readable file (${contextKind}). Swap the adjacent positionals or pass --context .` }, raw, undefined); + return; + } - const { trackable: decisions, outcome } = loadDecisionExtraction(contextPath); + const { trackable: decisions, outcome, unreadableIds } = 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') { + // #4794: nothing was measured — the answer must not carry the fields of a + // gate that did. total/covered are null (a type change is the point: + // 0 reads as data, null does not), `uncovered` is OMITTED (the list was + // never built), and the ids that failed to parse are carried so a caller + // capturing stdout knows which decision to fix. const partialParse = decisions.length > 0; output({ passed: false, skipped: false, reason: 'could-not-parse', - total: decisions.length, - covered: 0, - uncovered: [], - message: partialParse + total: null, + covered: null, + unreadable: unreadableIds, + message: (partialParse ? 'Decision coverage gate: decisions could not be fully parsed — one or more ' + '`- **D-NN ...**` bullets appear malformed (missing `:` or ` — ` separator, or a phase ' + 'prefix that is not a digit run, e.g. `D4x-01`). Fix the bullet format so all decisions ' + @@ -373,7 +399,8 @@ function cmdDecisionCoveragePlan(projectDir: string, args: string[], raw: boolea 'or D- tokens) but no decision bullets could be extracted. Check the formatting of the decisions ' + 'block and ensure bullets follow the `- **D-NN:** text`, `- **D4-NN:** text` (phase-prefixed), ' + 'or `- **D-NN — title** body` form. An ID grammar the parser does not support (e.g. `DEC-01`) ' + - 'also lands here.', + 'also lands here.') + + (unreadableIds.length > 0 ? ' Unreadable ids: ' + unreadableIds.join(', ') + '.' : ''), }, raw, undefined); return; } diff --git a/src/decisions.cts b/src/decisions.cts index b89ae3909..947cc0a67 100644 --- a/src/decisions.cts +++ b/src/decisions.cts @@ -52,6 +52,14 @@ export type DecisionOutcome = 'parsed' | 'none-present' | 'could-not-parse'; export interface DecisionExtraction { decisions: Decision[]; outcome: DecisionOutcome; + /** + * #4794: the attempted D-ids of bullets that fell to the parse-miss guard on + * a could-not-parse outcome — the ids a JSON caller needs to name what to + * fix. Empty when no id could be derived from a missed line; may be ABSENT + * on the evidence-based could-not-parse path (no guard-matched line exists + * to derive an id from). + */ + unreadableIds?: string[]; } const DISCRETION_HEADINGS = new Set([ @@ -431,6 +439,8 @@ function joinWrappedBoldLeadIns(lines: string[]): string[] { interface ParseDecisionLinesResult { decisions: Decision[]; parseMisses: number; + /** #4794: the attempted D-ids of bullets that fell to the parse-miss guard. */ + unreadableIds: string[]; } /** @@ -453,6 +463,7 @@ function parseDecisionLines(block: string): ParseDecisionLinesResult { let current: Decision | null = null; let openIndent: number | null = null; let parseMisses = 0; + const unreadableIds: string[] = []; const flush = (): void => { if (current) { @@ -561,6 +572,10 @@ function parseDecisionLines(block: string): ParseDecisionLinesResult { if (parseMissGuardRe.test(line)) { flush(); parseMisses += 1; + // #4794: carry the attempted id so a caller capturing stdout (a JSON gate) + // can name the decision to fix — today it exists only in this stderr warn. + const attempted = line.match(/\*\*\s*(D(?:[0-9][A-Za-z0-9]*)?-[A-Za-z0-9_-]*)/); + if (attempted && !unreadableIds.includes(attempted[1])) unreadableIds.push(attempted[1]); console.warn(`parseDecisions: ignored unparseable decision bullet: ${trimmed}`); continue; } @@ -578,7 +593,7 @@ function parseDecisionLines(block: string): ParseDecisionLinesResult { } } flush(); - return { decisions: out, parseMisses }; + return { decisions: out, parseMisses, unreadableIds }; } // ─── Primary entry point: extractDecisions ──────────────────────────────────── @@ -607,13 +622,13 @@ export function extractDecisions(content: unknown): DecisionExtraction { const taggedBlocks = extractTaggedBlocks(stripped, 'decisions'); if (taggedBlocks.length > 0) { const combined = taggedBlocks.join('\n\n'); - const { decisions, parseMisses } = parseDecisionLines(combined); + const { decisions, parseMisses, unreadableIds } = 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' }; + return { decisions, outcome: 'could-not-parse', unreadableIds }; } // FIX A: Block present but 0 extracted and no parse-misses. // Only report could-not-parse when there is genuine evidence of real decisions @@ -640,13 +655,13 @@ export function extractDecisions(content: unknown): DecisionExtraction { ); if (section !== null) { - const { decisions, parseMisses } = parseDecisionLines(section.body); + const { decisions, parseMisses, unreadableIds } = 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' }; + return { decisions, outcome: 'could-not-parse', unreadableIds }; } // FIX A: Heading found but 0 extracted and no parse-misses. // Report could-not-parse when the section body holds a decision-entry-shaped diff --git a/tests/decisions.test.cjs b/tests/decisions.test.cjs index ecb4c6140..f21de7313 100644 --- a/tests/decisions.test.cjs +++ b/tests/decisions.test.cjs @@ -761,9 +761,13 @@ describe('FIX B gate-level: parse-miss → passed:false regardless of covered de 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)}`); + // Confirm the answer is UNMEASURED (#4794): no covered/uncovered fields — + // the failure is parse-miss, not a coverage measurement. + assert.strictEqual(parsed.covered, null, 'covered must be null — nothing was measured'); + assert.strictEqual(parsed.total, null, 'total must be null — nothing was measured'); + assert.ok(!('uncovered' in parsed), 'uncovered must be OMITTED — the list was never built'); + assert.ok(Array.isArray(parsed.unreadable) && parsed.unreadable.includes('D-02'), + `unreadable must carry the malformed bullet's id, got: ${JSON.stringify(parsed.unreadable)}`); }); test('verify-side: valid D-01 covered + malformed D-02 → verify advisory surfaces could-not-parse', () => { @@ -3133,3 +3137,103 @@ describe('#4788: code spans in the bold lead-in are opaque to the separator gram assert.deepEqual(r.decisions.map((d) => d.id), ['D-01', 'D-91', 'D-119', 'D-92']); }); }); + +// ─── #4794: a gate that measured nothing must not emit the fields of one that did ── + +describe('#4794: decision-coverage answers an unmeasured shape on could-not-parse and a non-file context', () => { + let tmpDir; + let planningDir; + let phaseDir; + const contentWith = (bullets) => `\n${bullets.map((b) => '- ' + b).join('\n')}\n`; + + beforeEach(() => { + tmpDir = createTempProject('gsd-4794-'); + planningDir = path.join(tmpDir, '.planning'); + phaseDir = path.join(planningDir, 'phases', '01-init'); + fs.mkdirSync(phaseDir, { recursive: true }); + }); + + afterEach(() => cleanup(tmpDir)); + + function writePlanFile4794(name, body) { + fs.writeFileSync(path.join(phaseDir, `${name}-PLAN.md`), body); + } + + test('#4794: could-not-parse answers an unmeasured shape — null counts, unreadable ids, no uncovered', () => { + // The issue's repro: D-01 parses; D-02's title carries a second colon in + // plain prose → parse-miss. The gate used to answer covered:0/uncovered:[] + // — the fields of a measurement that never happened. + writeContextFile(phaseDir, [ + '# Context', + '', + '', + '', + '- **D-01: The list shows one row per contact.** Nothing else changes.', + '- **D-02: Two managers creating a card for the same pair: the second is rejected.** One pair, one card.', + '', + '', + ].join('\n')); + writePlanFile4794('01', '# Plan\n## Objective\nImplement feature.\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, 'the gate must still block'); + assert.strictEqual(parsed.reason, 'could-not-parse'); + assert.strictEqual(parsed.total, null, 'total must be null — nothing was measured'); + assert.strictEqual(parsed.covered, null, 'covered must be null — nothing was measured'); + assert.ok(!('uncovered' in parsed), 'uncovered must be OMITTED — the list was never built'); + assert.ok( + Array.isArray(parsed.unreadable) && parsed.unreadable.includes('D-02'), + `the ids that failed to parse must be carried as unreadable, got: ${JSON.stringify(parsed.unreadable)}`, + ); + assert.ok( + (parsed.message || '').includes('D-02'), + 'the message must name the unreadable id', + ); + }); + + test('#4794: a directory as the context path fails closed naming the path', () => { + // The issue's repro 2: the adjacent same-looking positionals swapped. + // fs.existsSync is true for a directory; the read yields nothing; the gate + // used to certify passed:true on a phase full of decisions. + writeContextFile(phaseDir, [ + '# Context', + '', + '', + '', + '- **D-01: The list shows one row per contact.** Nothing else changes.', + '', + '', + ].join('\n')); + writePlanFile4794('01', '# Plan\n## Objective\nImplement feature.\n'); + + const contextPath = phaseDir; // the DIRECTORY, swapped for the file + const result = runDecisionCoveragePlan(phaseDir, contextPath, tmpDir); + const parsed = JSON.parse(result.output || '{}'); + + assert.strictEqual(parsed.passed, false, 'must fail closed'); + assert.strictEqual(parsed.skipped, false, 'must not be a green skip'); + assert.ok( + (parsed.message || '').includes(contextPath) || (parsed.reason || '').includes('not a file'), + `the answer must name the path and what it is, got: ${JSON.stringify(parsed)}`, + ); + }); + + test('#4794: extractDecisions surfaces the ids of bullets that failed to parse', () => { + const { extractDecisions: extract } = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'decisions.cjs')); + const md = contentWith([ + '**D-01: The list shows one row per contact.** Nothing else changes.', + '**D-02: Two managers creating a card for the same pair: the second is rejected.** One pair, one card.', + '**D4x-01** ratio 3:1', + ]); + const r = extract(md); + assert.strictEqual(r.outcome, 'could-not-parse'); + assert.ok(Array.isArray(r.unreadableIds) && r.unreadableIds.includes('D-02'), + `unreadableIds must carry the failed bullet's id, got: ${JSON.stringify(r.unreadableIds)}`); + // #4130's phase-prefixed ID_ATTEMPT shape must be captured too. + assert.ok(r.unreadableIds.includes('D4x-01'), `phase-prefixed id must be captured, got: ${JSON.stringify(r.unreadableIds)}`); + assert.ok(!r.unreadableIds.includes('D-01'), 'the parsed bullet is not unreadable'); + }); +}); diff --git a/tests/install-write-confinement.test.cjs b/tests/install-write-confinement.test.cjs index 1397b25c0..7b8b99f08 100644 --- a/tests/install-write-confinement.test.cjs +++ b/tests/install-write-confinement.test.cjs @@ -3636,6 +3636,21 @@ describe('#3712 in-process home confinement', () => { t.after(() => { cleanup(a); cleanup(b); }); const sa = fs.statSync(a); const sb = fs.statSync(b); + // #4794 fold-in (windows conformance lane, 2 consecutive runs): a runner + // image whose temp volume reports an identical synthesized (dev, ino) for + // two distinct directories cannot discriminate at all. On such a platform + // the guard's identity containment degrades to refuse-everything (fail- + // closed, documented) — there is no behavior this probe can assert, only + // capability to report. Measured evidence rides in the skip reason (the + // ADR-2719 §6 explicit-skip discipline: reported as skipped, never a bare + // return that scores a PASS). When the platform CAN discriminate, the + // original assertions run unchanged. + if (sa.dev === sb.dev && sa.ino === sb.ino) { + t.skip(`this platform reports an identical identity for two distinct directories ` + + `(dev=${sa.dev}, ino=${sa.ino}) — identity-based containment would refuse ` + + 'everything here; the guard remains fail-closed but this probe has nothing to assert'); + return; + } assert.ok(sa.dev !== sb.dev || sa.ino !== sb.ino, `two distinct directories share an identity (dev=${sa.dev}/${sb.dev}, ino=${sa.ino}/${sb.ino}) — ` + 'isInside() would then match its first ancestor and refuse everything');