From 822934c901021956b9eccd50399a5193957be5a2 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 20 Sep 2026 02:53:58 -0400 Subject: [PATCH] fix(#4794): decision-coverage answers an unmeasured shape on could-not-parse; a non-file context path fails closed (#4889) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#4794): failing-first — could-not-parse must answer an unmeasured shape; a directory context path fails closed * fix(#4794): could-not-parse answers an unmeasured shape (null counts, unreadable ids, no uncovered); a non-file context path fails closed * chore(#4794): backfill changeset PR number (4889) * test(#4794): skip the directory-identity probe when the platform cannot discriminate (windows runner volume collapse, measured) Two consecutive windows conformance runs failed the probe with measured identical (dev, ino) for two distinct mkdtemp directories (dev=3606225537, ino=9007199255243448 for both) — a runner-volume property, not a regression in the guard. On such a platform the guard's identity containment degrades to refuse-everything (fail-closed, documented); the probe asserts capability, so the honest response is an explicit t.skip carrying the measurement (ADR-2719 §6), not a red lane for every PR. --------- Co-authored-by: sim --- .changeset/eager-wolves-bark.md | 5 ++ src/check-command-router.cts | 41 +++++++-- src/decisions.cts | 25 ++++-- tests/decisions.test.cjs | 110 ++++++++++++++++++++++- tests/install-write-confinement.test.cjs | 15 ++++ 5 files changed, 181 insertions(+), 15 deletions(-) create mode 100644 .changeset/eager-wolves-bark.md 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');