diff --git a/.changeset/gallant-eagles-zip.md b/.changeset/gallant-eagles-zip.md new file mode 100644 index 000000000..b770fc6c3 --- /dev/null +++ b/.changeset/gallant-eagles-zip.md @@ -0,0 +1,9 @@ +--- +type: Fixed +pr: 3903 +--- +**UAT rows separated only by a lone carriage return were silently dropped from the audit-uat scan.** A `VERIFICATION.md` or `deferred-items.md` written with lone-CR line endings rendered normally to a human reader, but reported zero outstanding items to the audit, hiding real human-verification and deferred-work entries. Both file types now surface their rows exactly as their LF/CRLF equivalents do. + +**Planning-inspect now surfaces UAT rows separated only by a lone carriage return.** The same lone-CR line-ending gap also hid rows from planning-inspect's own UAT reporting; a row that previously vanished from `uat.unresolved` now appears there too, matching its LF/CRLF equivalents. + +**A UAT row whose `result:` line had trailing text containing a Unicode line or paragraph separator (U+2028/U+2029) is no longer dropped.** A column-0 `result:` line whose text after the token happened to contain one of these separators previously failed to parse at all, silently discarding an outstanding row; it now parses the same as its plain-line equivalent. (#3707) diff --git a/.changeset/merry-koalas-chatter.md b/.changeset/merry-koalas-chatter.md new file mode 100644 index 000000000..ca5ce7dae --- /dev/null +++ b/.changeset/merry-koalas-chatter.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3903 +--- +**A phase with an unreadable UAT row no longer reports an affirmative milestone completion percentage.** Previously, one specific unreadable class — UAT rows hidden inside a closed code fence — was exempted from degrading a phase's fold, so a milestone could still publish a completion percentage over work nobody could actually see. Every class of unreadable UAT content now withholds the milestone's percentages the same way. The per-phase signal is unchanged: a phase's own `uat.scope` already reported "truncated" for this case and still does. (#3707) diff --git a/src/audit.cts b/src/audit.cts index 71b08ea89..71c7544c2 100644 --- a/src/audit.cts +++ b/src/audit.cts @@ -18,6 +18,9 @@ import { platformReadSync } from './shell-command-projection.cjs'; import { collectSection } from './markdown-sectionizer.cjs'; import { splitLines } from './text-lines.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports +import coreUtils = require('./core-utils.cjs'); +const { normalizeLineEndings } = coreUtils; +// eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspace = require('./planning-workspace.cjs'); const { planningDir, quickDirFrom } = planningWorkspace; // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -449,8 +452,16 @@ function scanDebugSessions(planDir: string): ScanOutcome { continue; } - const content = platformReadSync(safeFilePath); - if (content === null) continue; + // #3078-CR MEDIUM 2 (security review follow-up): normalize a lone-CR + // document at this read boundary, same seam as `src/uat.cts`'s + // `readNormalizedDocument` — `platformReadSync` performs no line-ending + // normalization itself, and extractFrontmatter/status-derivation below + // degrade a lone-CR file's frontmatter to `unknown`, which every scan + // in this module treats as "not open" (fail-open, the permissive + // direction) rather than a real parse gap. + const rawContent = platformReadSync(safeFilePath); + if (rawContent === null) continue; + const content = normalizeLineEndings(rawContent); const fm = extractFrontmatter(content, safeFilePath); const status = ((fm.status as string) || 'unknown').toLowerCase(); @@ -570,10 +581,14 @@ function scanQuickTasks(planDir: string): ScanOutcome { } catch { continue; } - const content = platformReadSync(safeSum); - if (content === null) { + // #3078-CR MEDIUM 2: same normalize-at-read-boundary fix as the other + // scans in this module — see the comment above `scanDebugSessions`'s + // read. + const rawContent = platformReadSync(safeSum); + if (rawContent === null) { status = 'unreadable'; } else { + const content = normalizeLineEndings(rawContent); fm = extractFrontmatter(content, safeSum); status = ((fm.status as string) || 'unknown').toLowerCase(); } @@ -648,8 +663,16 @@ function scanThreads(planDir: string): ScanOutcome { continue; } - const content = platformReadSync(safeFilePath); - if (content === null) continue; + // #3078-CR MEDIUM 2 (security review follow-up): normalize a lone-CR + // document at this read boundary, same seam as `src/uat.cts`'s + // `readNormalizedDocument` — `platformReadSync` performs no line-ending + // normalization itself, and extractFrontmatter/status-derivation below + // degrade a lone-CR file's frontmatter to `unknown`, which every scan + // in this module treats as "not open" (fail-open, the permissive + // direction) rather than a real parse gap. + const rawContent = platformReadSync(safeFilePath); + if (rawContent === null) continue; + const content = normalizeLineEndings(rawContent); const fm = extractFrontmatter(content, safeFilePath); const status = deriveThreadStatus(fm, content); @@ -723,8 +746,16 @@ function scanTodos(planDir: string): ScanOutcome { continue; } - const content = platformReadSync(safeFilePath); - if (content === null) continue; + // #3078-CR MEDIUM 2 (security review follow-up): normalize a lone-CR + // document at this read boundary, same seam as `src/uat.cts`'s + // `readNormalizedDocument` — `platformReadSync` performs no line-ending + // normalization itself, and extractFrontmatter/status-derivation below + // degrade a lone-CR file's frontmatter to `unknown`, which every scan + // in this module treats as "not open" (fail-open, the permissive + // direction) rather than a real parse gap. + const rawContent = platformReadSync(safeFilePath); + if (rawContent === null) continue; + const content = normalizeLineEndings(rawContent); const fm = extractFrontmatter(content, safeFilePath); @@ -796,8 +827,16 @@ function scanSeeds(planDir: string): ScanOutcome { continue; } - const content = platformReadSync(safeFilePath); - if (content === null) continue; + // #3078-CR MEDIUM 2 (security review follow-up): normalize a lone-CR + // document at this read boundary, same seam as `src/uat.cts`'s + // `readNormalizedDocument` — `platformReadSync` performs no line-ending + // normalization itself, and extractFrontmatter/status-derivation below + // degrade a lone-CR file's frontmatter to `unknown`, which every scan + // in this module treats as "not open" (fail-open, the permissive + // direction) rather than a real parse gap. + const rawContent = platformReadSync(safeFilePath); + if (rawContent === null) continue; + const content = normalizeLineEndings(rawContent); const fm = extractFrontmatter(content, safeFilePath); const status = ((fm.status as string) || 'dormant').toLowerCase(); @@ -970,8 +1009,16 @@ function scanUatGaps(planDir: string, cwd: string): ScanOutcome { continue; } - const content = platformReadSync(safeFilePath); - if (content === null) continue; + // #3078-CR MEDIUM 2 (security review follow-up): normalize a lone-CR + // document at this read boundary, same seam as `src/uat.cts`'s + // `readNormalizedDocument` — `platformReadSync` performs no line-ending + // normalization itself, and extractFrontmatter/status-derivation below + // degrade a lone-CR file's frontmatter to `unknown`, which every scan + // in this module treats as "not open" (fail-open, the permissive + // direction) rather than a real parse gap. + const rawContent = platformReadSync(safeFilePath); + if (rawContent === null) continue; + const content = normalizeLineEndings(rawContent); const fm = extractFrontmatter(content, safeFilePath); const status = ((fm.status as string) || 'unknown').toLowerCase(); @@ -1045,8 +1092,16 @@ function scanVerificationGaps(planDir: string, cwd: string): ScanOutcome 0) { diagnostics.push({ @@ -914,7 +928,7 @@ function buildUatRows( detail: `UAT document has ${headingsSeen} test block(s) with no parseable result; unresolved is not a complete answer.`, }); scope = SCOPE.TRUNCATED; - if (headingsSeen > shortfallBlocks) foldScope = SCOPE.TRUNCATED; + foldScope = SCOPE.TRUNCATED; } } return { items, scope, foldScope }; @@ -1226,13 +1240,15 @@ function buildPlanningInspect(cwd: string): Record { const phaseId = token ? token[1] : null; const { goal, dependencies } = buildPhaseGoalAndDependencies(cwd, roadmapDoc, phaseId, phase.dir, diagnostics); - // `uat.foldScope`, NOT `uat.scope` (#3078 round-8). The two differ for - // exactly one case: a UAT document whose ONLY parse gap is the - // fence-suppression shortfall, `src/uat.cts`'s documented ACCEPTED - // OVER-REPORT class. That still reports honestly on the row itself - // (`uat.scope === "truncated"` plus the `uat_unreadable` diagnostic below), - // but it must not raise `phase_scope_degraded` and must not withhold the - // milestone's percentages — see `buildUatRows` for the full rationale. + // `uat.foldScope`, NOT `uat.scope` (#3078 round-8). The two currently + // agree at every call site — see `buildUatRows` for why the field is + // still kept separate — so folding either one here produces the same + // result today. `foldScope` is used because it is the field with teeth: + // it is what `worstScope` folds into the phase's overall scope, and a + // non-COMPLETE result here raises `phase_scope_degraded` and, via + // `makeFraction`, withholds the milestone's percentages. A phase whose + // UAT document could not be fully read must not contribute an + // affirmative completion to the milestone. const folded = worstScope(phase.scope, plans.scope, uat.foldScope, goal.scope, dependencies.scope); if (folded !== SCOPE.COMPLETE) { diagnostics.push({ diff --git a/src/uat-predicate.cts b/src/uat-predicate.cts index 0b6c4f952..b260f465e 100644 --- a/src/uat-predicate.cts +++ b/src/uat-predicate.cts @@ -25,6 +25,9 @@ const { readVerificationStatus } = verification; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); const { scopeToPhase } = phaseIdMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import coreUtils = require('./core-utils.cjs'); +const { normalizeLineEndings } = coreUtils; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -156,34 +159,64 @@ function analyzeMarkdown(raw: string): { unterminatedFence: boolean; unterminate function parseUatResultItems(cleanContent: string): Array<{ test: number; name: string; result: string }> { const items: Array<{ test: number; name: string; result: string }> = []; - // Find all ### N. Name headings (line-anchored) - const headingPattern = /^###\s*(\d+)\.\s*(.+)$/gm; - const headings: Array<{ index: number; test: number; name: string }> = []; - let hMatch: RegExpExecArray | null; - while ((hMatch = headingPattern.exec(cleanContent)) !== null) { - headings.push({ - index: hMatch.index + hMatch[0].length, - test: parseInt(hMatch[1], 10), - name: hMatch[2].trim(), - }); + // Find all ### N. Name headings. + // #3078-CR MEDIUM (security review follow-up): STRUCTURE and ATTRIBUTION + // need different split frames. This is a STRUCTURE scan — finding where a + // heading block begins — and there is no attribution distinction to + // preserve, so split on any of `\n`, U+2028, U+2029: a heading delimited by + // an exotic line separator (origin/next's `/m`-anchored scan found these; + // a naive `split('\n')`-only port silently stopped finding them, making the + // gate MORE permissive than origin/next) is found exactly like a + // `\n`-delimited one. Contrast the `result:` scan below, which is an + // ATTRIBUTION scan and must NOT do this. + const HEADING_LINE_RE = /^###\s*(\d+)\.\s*(.+)$/; + const headings: Array<{ index: number; lineStart: number; test: number; name: string }> = []; + { + // All three separators are exactly one UTF-16 code unit, so the + // `line.length + 1` offset arithmetic below stays valid regardless of + // which separator terminated a given line. + const lines = cleanContent.split(/[\n\u2028\u2029]/); + let offset = 0; + for (const line of lines) { + const hMatch = line.match(HEADING_LINE_RE); + if (hMatch) { + headings.push({ + index: offset + hMatch[0].length, + lineStart: offset, + test: parseInt(hMatch[1], 10), + name: hMatch[2].trim(), + }); + } + offset += line.length + 1; // +1 for the separator consumed by split + } } for (let i = 0; i < headings.length; i++) { const h = headings[i]; const blockStart = h.index; - // More precise: find next heading's position in the original string - // We'll slice from current heading end to the position just before next heading's "###" - const nextHeadingMatch = i + 1 < headings.length - ? cleanContent.lastIndexOf('\n###', headings[i + 1].index) - : -1; - const blockContent = nextHeadingMatch >= blockStart - ? cleanContent.slice(blockStart, nextHeadingMatch) + // A block spans until the START of the next heading's line (tracked + // directly from the same split-frame scan above), not a re-search for a + // literal '\n###' over unsplit text -- the latter would silently miss a + // next heading delimited by U+2028/U+2029 instead of '\n' and swallow + // every subsequent block into this one. + const blockContent = i + 1 < headings.length + ? cleanContent.slice(blockStart, headings[i + 1].lineStart) : cleanContent.slice(blockStart); - // Column-0 anchored result line: /^result:[ \t]*\[?([\w-]+)\]?/mi + // Column-0 result line, split-then-match (#3078-CR MEDIUM — same fix as + // the heading scan above): test each already-split line individually + // against a single-line (no `/m` anchor) pattern instead of anchoring + // over unsplit `blockContent`, so a `result:`-shaped line reachable only + // via a U+2028/U+2029 separator inside an `expected: |` scalar body can + // never register as a fake column-0 match. FIRST MATCH WINS — no + // ambiguity counting, matching src/uat.cts's contract. // Uses [ \t]* (not \s*) so the captured value must sit on the SAME line as result:. // A result: key with the value on a subsequent line yields no match → 'missing' (blocker). - const resultMatch = /^result:[ \t]*\[?([\w-]+)\]?/mi.exec(blockContent); + const RESULT_LINE_RE = /^result:[ \t]*\[?([\w-]+)\]?/i; + const resultMatch = blockContent + .split('\n') + .map((line) => line.match(RESULT_LINE_RE)) + .find((m): m is RegExpMatchArray => m !== null) ?? null; if (resultMatch) { items.push({ test: h.test, @@ -265,7 +298,11 @@ function evaluateUatPassed( const uatFilePath = path.join(phaseFullDir, file); let raw = ''; try { - raw = fs.readFileSync(uatFilePath, 'utf-8'); + // #3078-CR MEDIUM: normalize line endings at the read boundary — the + // same seam src/uat.cts and src/verification.cts route through — so a + // lone-CR *-UAT.md is not read as one unbroken line by the column-0 + // scans below. + raw = normalizeLineEndings(fs.readFileSync(uatFilePath, 'utf-8')); } catch { blockers.push(`${file}: could not read file`); continue; @@ -317,7 +354,8 @@ function evaluateUatPassed( const verificationFilePath = path.join(phaseFullDir, file); let raw = ''; try { - raw = fs.readFileSync(verificationFilePath, 'utf-8'); + // #3078-CR MEDIUM: same read-boundary normalization as the UAT loop above. + raw = normalizeLineEndings(fs.readFileSync(verificationFilePath, 'utf-8')); } catch { blockers.push(`${file}: could not read verification file`); continue; diff --git a/src/uat.cts b/src/uat.cts index 093de9240..89c468136 100644 --- a/src/uat.cts +++ b/src/uat.cts @@ -22,7 +22,7 @@ import markdownTable = require('./markdown-table.cjs'); const { splitTableRow, isDelimiterRow } = markdownTable; // eslint-disable-next-line @typescript-eslint/no-require-imports import coreUtils = require('./core-utils.cjs'); -const { toPosixPath } = coreUtils; +const { toPosixPath, normalizeLineEndings } = coreUtils; // eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspace = require('./planning-workspace.cjs'); const { planningDir } = planningWorkspace; @@ -114,6 +114,25 @@ function selectPhaseUatFiles(files: string[], phaseDirName: string): string[] { return scopeToPhase(files.filter((f) => f.includes('-UAT') && f.endsWith('.md')), phaseDirName); } +/** + * The ONE read boundary for every document `cmdAuditUat` scans off disk + * (#3707-CR follow-up MAJOR). Wraps `fs.readFileSync` + + * `normalizeLineEndings` in a single seam so a lone-CR-separated + * `*-UAT.md`, `*-VERIFICATION.md`, or `deferred-items.md` is normalized BY + * CONSTRUCTION before it reaches ANY downstream parser — current + * (`parseUatItemsWithStats`, `parseVerificationItems`, `parseDeferredItems`) + * or future. Fixing this per-parser was the original (#3707-CR) MEDIUM fix's + * mistake: two of the four ingresses in this function were normalized by + * editing their own parsers directly, and the other two (VERIFICATION, + * deferred-items.md) were missed precisely because nothing forced a new call + * site to remember the step. Routing every read through this function + * removes that failure mode: a parser added later needs no line-ending logic + * of its own, because the text it receives is already normalized. + */ +function readNormalizedDocument(filePath: string): string { + return normalizeLineEndings(fs.readFileSync(filePath, 'utf-8')); +} + function cmdAuditUat(cwd: string, raw: boolean): void { const phasesDir = path.join(planningDir(cwd), 'phases'); const hasActivePhases = fs.existsSync(phasesDir); @@ -174,7 +193,7 @@ function cmdAuditUat(cwd: string, raw: boolean): void { // the reason scopeToPhase has no unfiltered fallback. for (const file of selectPhaseUatFiles(files, dir)) { const uatFilePath = path.join(phaseDir, file); - const content = fs.readFileSync(uatFilePath, 'utf-8'); + const content = readNormalizedDocument(uatFilePath); const { items, headingsSeen } = parseUatItemsWithStats(content); const status = (extractFrontmatter(content, uatFilePath).status as string || 'unknown'); // `parse_gap` means the file contained `### N.` test blocks that @@ -235,7 +254,7 @@ function cmdAuditUat(cwd: string, raw: boolean): void { // for the same reason as the UAT loop above. for (const file of scopeToPhase(files.filter(f => f.includes('-VERIFICATION') && f.endsWith('.md')), dir)) { const verificationFilePath = path.join(phaseDir, file); - const content = fs.readFileSync(verificationFilePath, 'utf-8'); + const content = readNormalizedDocument(verificationFilePath); const status = extractFrontmatter(content, verificationFilePath).status as string || 'unknown'; if (status === 'human_needed' || status === 'gaps_found') { const items = parseVerificationItems(content, status, verificationFilePath); @@ -263,7 +282,7 @@ function cmdAuditUat(cwd: string, raw: boolean): void { // required. const deferredFile = 'deferred-items.md'; if (files.includes(deferredFile)) { - const content = fs.readFileSync(path.join(phaseDir, deferredFile), 'utf-8'); + const content = readNormalizedDocument(path.join(phaseDir, deferredFile)); const items = parseDeferredItems(content); if (items.length > 0) { results.push({ @@ -364,6 +383,13 @@ function cmdRenderCheckpoint(cwd: string, options: { file?: string } = {}, raw: // ─── parseCurrentTest ───────────────────────────────────────────────────────── function parseCurrentTest(content: string): CurrentTest { + // #3707-CR: this is the render-checkpoint path's own independent ingress + // into `tokenizeHeadings` (via the `parseFirstPendingTest` fallback below), + // separate from `parseUatItemsWithStats`'s. Normalize here too, ONCE, so a + // lone-CR document cannot hide its first pending row from this path either + // — see `normalizeLineEndings` for why. + content = normalizeLineEndings(content); + // Use the seam to locate the ## Current Test section (ADR-1372 T5). // HTML-comment stripping within the section body is UAT-specific, so we keep // the comment removal caller-side after extracting the body. @@ -1220,10 +1246,21 @@ function countUnattributedIndentedRows(surface: string): number { * Reported separately so a consumer that must decide whether to WITHHOLD a * derived number — as opposed to merely REPORT the gap — can tell "a row I * definitely could not read" from "a row I possibly mis-counted". - * `src/planning-inspect.cts`'s `buildUatRows` is that consumer; `cmdAuditUat` - * is not, and still gates `parse_gap` on the total. + * + * #3707-CR: `src/planning-inspect.cts`'s `buildUatRows` does NOT destructure + * this field (verified — it and `cmdAuditUat` both consume only `items` and + * `headingsSeen`), correcting an earlier stated instruction that it did. + * `shortfallBlocks` currently has NO production consumer outside this + * function's own computation. It is retained on the return value anyway, + * deliberately, as part of this function's published stats contract — tests + * assert on the full `{ items, headingsSeen, shortfallBlocks }` shape, and + * dropping a returned field is a wider, unrelated change than a line-ending + * fix warrants. A future consumer that needs to distinguish an + * accepted-over-report shortfall from the rest of `headingsSeen` (the + * original design intent above) can still do so. */ function parseUatItemsWithStats(content: string): { items: UatItem[]; headingsSeen: number; shortfallBlocks: number } { + content = normalizeLineEndings(content); const items: UatItem[] = []; let headingsSeen = 0; let shortfallBlocks = 0; @@ -1422,7 +1459,45 @@ function parseUatItemsWithStats(content: string): { items: UatItem[]; headingsSe // doing so previously changed `categorizeItem`'s classification for // shapes origin/next categorized differently (an unpinned behavior // change, not something the blocker required). - const resultLineMatch = fenceStrippedBlock.match(/^result:\s*\[?(\w+)\]?.*$/im); + // #3078-CR defect A fix, split-then-match scan: the previous `.match()` + // against `/^result:.../im` ran a MULTILINE regex anchor directly over + // unsplit block text. ECMA-262's LineTerminator set for `^`/`$` under + // `/m` includes U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR, but + // `content.split('\n')` and this module's own heading tokenizer do NOT + // treat either as a boundary. A `result:`-shaped line inside an + // `expected: |` scalar body, sitting immediately after one of these + // separators instead of an ordinary character, was therefore read as a + // genuine line start by the regex engine even though it is not + // `\n`-delimited from anything — it is exactly as much "one line" to + // every other consumer as the ordinary-character control case. + // Splitting on `\n` FIRST and testing each already-split line against a + // single-line (`/im`-anchor-free) pattern fixes this: a line is never + // split by U+2028/U+2029 (`String.prototype.split` matches only its + // literal separator argument, never the wider ECMA-262 LineTerminator + // set), so a `result:`-shaped line reachable only via one of those + // separators can never register as its own split line — the split view + // and the regex view are back in agreement, by construction, exactly the + // way `splitLines` module is documented to be immune to the sibling `\r` + // bug. + // + // FIRST MATCH WINS (byte-identical to origin/next otherwise): a block + // with more than one column-0 `result:` line resolves to the FIRST one + // encountered, same as the pre-existing `.match()` behaviour without + // `/g` — this is deliberately NOT an ambiguity/parse-gap case (that + // variant was tried and reverted: its boundary-truncation heuristic + // mistook an indented `### N.` living inside a legitimate block scalar + // for a heading boundary, corrupting every scalar/indent guard in this + // module — see tests/uat.test.cjs's #3078 scalar guard family). + // Trailing text is matched with `[^]*` rather than `.*` (final review + // MINOR 1): `.` never matches U+2028/U+2029, so a column-0 `result:` + // line whose trailing text contains one of those separators would + // otherwise never reach `$`, and the whole line would fail to match — + // an unpinned regression against origin/next, which parses it. + const RESULT_LINE_RE = /^result:\s*\[?(\w+)\]?[^]*$/i; + const resultLineMatch = fenceStrippedBlock + .split('\n') + .map((line) => line.match(RESULT_LINE_RE)) + .find((m): m is RegExpMatchArray => m !== null); if (!resultLineMatch) { headingsSeen += 1; continue; diff --git a/src/verification.cts b/src/verification.cts index 3a2c04a98..510da63dd 100644 --- a/src/verification.cts +++ b/src/verification.cts @@ -37,6 +37,8 @@ import phaseId = require('./phase-id.cjs'); import frontmatterMod = require('./frontmatter.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports -- plan-scan.cjs is an export= CommonJS module import scanPhasePlans = require('./plan-scan.cjs'); +// eslint-disable-next-line @typescript-eslint/no-require-imports -- core-utils.cjs is an export= CommonJS module +import coreUtilsMod = require('./core-utils.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-scope.cjs is an export= CommonJS module import planningScopeMod = require('./planning-scope.cjs'); import { execGit } from './shell-command-projection.cjs'; @@ -45,6 +47,7 @@ import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; const { output, error } = io; const { extractPhaseToken, scopeToPhase } = phaseId; const { extractFrontmatter } = frontmatterMod; +const { normalizeLineEndings } = coreUtilsMod; const { SCOPE } = planningScopeMod; type Scope = planningScopeMod.Scope; @@ -650,7 +653,17 @@ function readVerificationStatus( const filePath = path.join(phaseDir, verificationFile); let rawStatus: string | null = null; try { - const content = fsImpl.readFileSync(filePath, 'utf-8'); + // #3707-CR follow-up MINOR 1: normalize line endings at this read + // boundary — this function's own `readFileSync` is the equivalent seam + // `planning.inspect`'s `buildUatRows`/`readDocument` route through for + // UAT/REQUIREMENTS documents, but `readVerificationStatus` had no such + // normalization of its own. A lone-CR VERIFICATION.md's `---\r...\r---` + // frontmatter fence never matched `extractFrontmatter`'s byte-0 + // `---\n`/`---\r\n` check, so `status: passed` was read as absent and + // this function reported 'missing' — under-reporting a completed + // verification as if the step never ran, the fail-safe direction but the + // same root cause as the false-clean class fixed elsewhere in #3707-CR. + const content = normalizeLineEndings(fsImpl.readFileSync(filePath, 'utf-8')); const fm = extractFrontmatter(content, filePath); const statusVal = fm['status']; // status is always a scalar string in a well-formed VERIFICATION.md frontmatter; diff --git a/tests/audit-command-cutover.test.cjs b/tests/audit-command-cutover.test.cjs index c560e4795..5803fa023 100644 --- a/tests/audit-command-cutover.test.cjs +++ b/tests/audit-command-cutover.test.cjs @@ -2409,5 +2409,95 @@ describe('bug #950: quick-task SUMMARY must carry status: complete', () => { const result = ack(tmpDir, ['--category', 'uat_gaps', '--milestone', 'v1.0']); // no --phase/--file assert.equal(result.success, false, 'missing --phase/--file must be refused'); }); + + // ── mixed-frame fix (security review, #3078-CR follow-up): the writer's + // snapshot value and the scanners' recomputed value must share ONE + // frame (both normalized) even though the writer's SPLICE stays raw. A + // lone-CR artifact discriminates this: normalizing shifts every byte + // offset, so if the splice used normalized text it would corrupt the + // file, and if the snapshot used raw text it would never match the + // scanner's normalized recomputation — acknowledge would silently never + // suppress. An LF control proves the round trip isn't accidentally + // broken for the common case while fixing the CR case. ────────────── + + test('mixed-frame fix: context_questions acknowledge on a lone-CR artifact actually suppresses the item', () => { + // A lone-CR CONTEXT.md is the clean discriminator: `deriveOpenQuestions` + // splits the `## Open Questions` body on `\n` — raw lone-CR content has + // NO `\n` at all, so pre-fix the writer's raw-content digest is + // computed over a ZERO-question set (sha256 of ''), which can never + // equal what the scanner (reading normalized content) recomputes — the + // acknowledge is a silent no-op. This file carries no frontmatter + // fence, so it is not entangled with the separate, pre-existing + // splice-vs-lone-CR-fence limitation a UAT/VERIFICATION file with an + // EXISTING lone-CR frontmatter block would hit. + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '01-CONTEXT.md'); + fs.writeFileSync(filePath, '# Context\r\r## Open Questions\r\r- Which backend?\r- What about auth?\r'); + + const before = audit(tmpDir); + assert.equal(before.counts.context_questions, 1, 'lone-CR CONTEXT file must be parsed as having open questions before acknowledge'); + + const result = ack(tmpDir, ['--category', 'context_questions', '--phase', '01', '--file', '01-CONTEXT.md', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + assert.notEqual( + JSON.parse(result.output).questions_digest, + 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855', + 'the recorded digest must reflect the REAL 2-question set, not sha256 of an empty set (the pre-fix raw-content bug)', + ); + + const after = audit(tmpDir); + assert.equal(after.counts.context_questions, 0, 'lone-CR context_questions item must be SUPPRESSED, not resurface as a silent no-op'); + assert.equal(after.acknowledged.context_questions, 1); + assert.deepEqual( + (after.items.context_questions || []).filter((i) => !i.scan_error).map((i) => i.file), + [], + 'the specific acknowledged file must be gone from the open items, by identity — not just a smaller count', + ); + }); + + test('mixed-frame fix control: context_questions acknowledge on an LF artifact still suppresses the item (round trip not broken by the CR fix)', () => { + const phaseDir = planningPath('phases', '02-beta'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '02-CONTEXT.md'); + fs.writeFileSync(filePath, '# Context\n\n## Open Questions\n\n- Which backend?\n- What about auth?\n'); + + const before = audit(tmpDir); + assert.equal(before.counts.context_questions, 1); + + const result = ack(tmpDir, ['--category', 'context_questions', '--phase', '02', '--file', '02-CONTEXT.md', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const after = audit(tmpDir); + assert.equal(after.counts.context_questions, 0, 'LF context_questions item must still be suppressed after the fix'); + assert.equal(after.acknowledged.context_questions, 1); + assert.deepEqual( + (after.items.context_questions || []).filter((i) => !i.scan_error).map((i) => i.file), + [], + 'the specific acknowledged file must be gone from the open items, by identity', + ); + assert.ok(!fs.readFileSync(filePath, 'utf-8').includes('\r'), 'LF file stays LF — the fix must not introduce CR bytes on the common path'); + }); + + test('mixed-frame fix compatibility: a HAND-AUTHORED pre-existing LF acknowledgment marker (never touched by this change\'s writer) is still recognised', () => { + const phaseDir = planningPath('phases', '03-gamma'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '03-UAT.md'); + // The marker's `gap_snapshot` value is computed BY HAND here, per + // `deriveUatGapSnapshotValue`'s documented shape + // (`${status}::scenarios=${openScenarioCount}`) — NOT produced by + // calling the writer — so this exercises the scanner's READ side in + // isolation against a marker that predates this change entirely. This + // content has zero `result: pending`/`[pending]` matches, so the open + // scenario count is 0. + fs.writeFileSync( + filePath, + '---\nstatus: gaps_found\naudit_acknowledged:\n milestone: v1.0\n at: "2026-08-15"\n gap_snapshot: "gaps_found::scenarios=0"\n---\n# UAT\n\n## Gaps\n\n- truth: "something broke"\n status: open\n', + ); + + const after = audit(tmpDir); + assert.equal(after.counts.uat_gaps, 0, 'a pre-existing LF acknowledgment marker must still suppress the item after the mixed-frame fix'); + assert.equal(after.acknowledged.uat_gaps, 1); + }); }); } diff --git a/tests/planning-inspect.test.cjs b/tests/planning-inspect.test.cjs index 214da28aa..6d6e04f77 100644 --- a/tests/planning-inspect.test.cjs +++ b/tests/planning-inspect.test.cjs @@ -1226,26 +1226,73 @@ describe('planning inspect — evidence kept separate, never folded', () => { }); } - // ─── #3078 round-8: a shortfall is REPORTED, it does not WITHHOLD ────────── + // ─── #3078 round-8 REVERTED (fix/3707-fold-shield-revert): the shield is + // GONE — a fence-suppression shortfall now degrades the fold exactly like + // every other UAT parse gap. `phase.uat.scope` (this phase's own reported + // evidence quality) and `phase.scope` (the `worstScope` fold that gates + // `phase_scope_degraded` and — via `progress.*` — the milestone's + // percentages) remain two SEPARATE fields with two separate meanings, but + // as of this revert they no longer disagree on a shortfall-only gap: both + // report degraded evidence. `src/uat.cts` still counts `shortfallBlocks` as + // its own documented ACCEPTED-OVER-REPORT subset of `headingsSeen`, but + // `buildUatRows` no longer reads that subset when deciding `foldScope` — see + // `src/planning-inspect.cts`'s `buildUatRows` for the current (unconditional) + // rule: `headingsSeen > 0` alone sets BOTH `scope` and `foldScope` to + // TRUNCATED, with no `headingsSeen > shortfallBlocks` comparison left to + // exempt anything. // - // `phase.uat.scope` (what this phase's UAT evidence is worth) and - // `phase.scope` (the `worstScope` fold, which gates `phase_scope_degraded` - // and — via `progress.*` — the milestone's percentages) are separate - // decisions. The fence-suppression shortfall is `src/uat.cts`'s documented - // ACCEPTED OVER-REPORT class: a closed-fence documentation sample with - // literal digits is indistinguishable from a straddled row. A COMPLETED - // phase is terminal, so letting that class withhold percentages would make a - // paragraph of prose suppress the project's numbers in every future audit - // forever. + // Boundary matrix (`headingsSeen`, `shortfallBlocks`): + // (0, 0) -> no diagnostic, fold COMPLETE — unchanged + // (1, 1) -> fold TRUNCATED — THE CHANGE (below) + // (1, 0) -> fold TRUNCATED — unchanged + // (2, 1) -> fold TRUNCATED — unchanged - test('shortfallAloneReportsTheGapWithoutDegradingThePhaseOrWithholdingThePercentage', (t) => { + test('shortfallAloneDegradesTheFoldAndWithholdsThePercentage', (t) => { const tmpDir = createTempProject(); t.after(() => cleanup(tmpDir)); const phaseDir = declarePhase(tmpDir, '1', 'Foo'); writeVerification(phaseDir, '1', 'passed'); // A `## Notes` section documenting the row format inside a CLOSED fence — // literal digits, so `TEST_HEADING_LINE_RE` counts it and the shortfall - // fires. This is prose, not an outstanding row. + // scan fires: headingsSeen === 1, shortfallBlocks === 1 (the boundary + // case the removed `headingsSeen > shortfallBlocks` comparison used to + // treat specially — post-revert there is no comparison left, so this is + // ordinary `headingsSeen > 0`). + writeUatDocWithStatus(phaseDir, '1', 'complete', [ + '# UAT', + '', + '## Notes', + '', + 'How to write a row:', + '', + '```', + '### 1. Example Row', + 'expected: x', + 'result: pass', + '```', + '', + ]); + + const payload = parseInspect(tmpDir); + const phase = payload.phases[0]; + // Post-revert: the fold now degrades on a shortfall-only gap, and the + // milestone percentage is withheld — the exact behavior #3707's shield + // used to suppress. + assert.strictEqual(phase.scope, 'truncated'); + assert.ok(payload.diagnostics.some((d) => d.code === 'phase_scope_degraded' && d.subject === phase.dir)); + assert.strictEqual(payload.progress.accepted_phases.percent, null); + assert.ok(payload.diagnostics.some((d) => d.code === 'percent_withheld')); + }); + + test('shortfallAloneStillReportsTruncatedUatScopeAndTheUnreadableDiagnostic', (t) => { + // CONTROL: the per-phase `uat.scope` reporting and the `uat_unreadable` + // diagnostic are untouched by the shield revert — they already went + // TRUNCATED for every gap, shortfall included. Only the FOLD (asserted in + // the row above) changed. + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + const phaseDir = declarePhase(tmpDir, '1', 'Foo'); + writeVerification(phaseDir, '1', 'passed'); writeUatDocWithStatus(phaseDir, '1', 'complete', [ '# UAT', '', @@ -1263,14 +1310,8 @@ describe('planning inspect — evidence kept separate, never folded', () => { const payload = parseInspect(tmpDir); const phase = payload.phases[0]; - // REPORTED … - assert.ok(payload.diagnostics.some((d) => d.code === 'uat_unreadable' && d.subject.includes('1-UAT.md'))); assert.strictEqual(phase.uat.scope, 'truncated'); - // … but NOT degraded, and the numbers still publish. - assert.strictEqual(phase.scope, 'complete'); - assert.strictEqual(payload.diagnostics.filter((d) => d.code === 'phase_scope_degraded').length, 0); - assert.strictEqual(payload.diagnostics.filter((d) => d.code === 'percent_withheld').length, 0); - assert.strictEqual(payload.progress.accepted_phases.percent, 100); + assert.ok(payload.diagnostics.some((d) => d.code === 'uat_unreadable' && d.subject.includes('1-UAT.md'))); }); test('aNonShortfallParseGapStillDegradesThePhaseAndWithholdsThePercentage', (t) => { @@ -1315,6 +1356,250 @@ describe('planning inspect — evidence kept separate, never folded', () => { assert.strictEqual(payload.progress.accepted_phases.percent, null); }); + test('aPhaseWithNoUatGapAtAllStillPublishesThePercentage', (t) => { + // CONTROL for the catastrophic-revert failure mode: a revert that sets + // `foldScope = SCOPE.TRUNCATED` unconditionally, OUTSIDE the + // `headingsSeen > 0` branch, would withhold every percentage in the + // project — including this phase, which has no gap whatsoever. + // Boundary case: headingsSeen === 0, shortfallBlocks === 0. + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + const phaseDir = declarePhase(tmpDir, '1', 'Foo'); + writeVerification(phaseDir, '1', 'passed'); + writeUatDocWithStatus(phaseDir, '1', 'complete', [ + '### 1. Alpha', + 'expected: a', + 'result: pass', + '', + ]); + + const payload = parseInspect(tmpDir); + const phase = payload.phases[0]; + assert.strictEqual(phase.uat.scope, 'complete'); + assert.strictEqual(phase.scope, 'complete'); + assert.strictEqual(payload.diagnostics.filter((d) => d.code === 'phase_scope_degraded').length, 0); + assert.strictEqual(payload.diagnostics.filter((d) => d.code === 'uat_unreadable').length, 0); + assert.strictEqual(payload.progress.accepted_phases.percent, 100); + }); + + test('aMixedShortfallAndGenuineParseGapPhaseDegradesTheFold', (t) => { + // CONTROL: a file carrying BOTH a shortfall block and a genuine + // (non-shortfall) parse gap in the same document — headingsSeen === 2, + // shortfallBlocks === 1. `headingsSeen > shortfallBlocks` was already true + // under the OLD shielded rule (2 > 1), so this row degraded the fold + // before the revert too; it stays green throughout. + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + const phaseDir = declarePhase(tmpDir, '1', 'Foo'); + writeVerification(phaseDir, '1', 'passed'); + writeUatDocWithStatus(phaseDir, '1', 'complete', [ + // A genuine gap: a column-0 `### N.` block with no `result:` line. + '### 1. Alpha', + 'expected: a', + '', + // A shortfall: a closed-fence documentation sample with literal digits. + '## Notes', + '', + '```', + '### 2. Example Row', + 'expected: x', + 'result: pass', + '```', + '', + ]); + + const payload = parseInspect(tmpDir); + const phase = payload.phases[0]; + assert.strictEqual(phase.uat.scope, 'truncated'); + assert.strictEqual(phase.scope, 'truncated'); + assert.ok(payload.diagnostics.some((d) => d.code === 'phase_scope_degraded' && d.subject === phase.dir)); + assert.strictEqual(payload.progress.accepted_phases.percent, null); + }); + + // ─── #3707-CR security review MEDIUM ──────────────────────────────────────── + // + // A lone CR (`String.fromCharCode(13)`, no paired LF) is a CommonMark line + // ending — a document using it renders as separate lines to a human reader. + // `src/uat.cts` now normalizes every line ending at parse ingress + // (`normalizeLineEndings`), so the row is no longer hidden: it is parsed and + // surfaces as a visible outstanding `uat.unresolved` item, exactly like its + // LF twin. See `tests/uat.test.cjs`'s "#3707-CR" describe block for the + // parser-level pin this end-to-end case is downstream of. + // + // This test does NOT assert `percent === null` (an earlier version of this + // test did, and was wrong — reasoning from the PRE-fix symptom instead of + // the post-fix behavior). A surfaced `result: blocked` row is VISIBLE + // outstanding work, not unreadable evidence, and this module's pinned + // invariant is that visible outstanding UAT work deliberately does NOT + // withhold percentages or degrade scope — only genuinely UNREADABLE + // evidence does (`keepsUnresolvedUatAndPassingVerificationSeparateWithNoCombinedVerdict`, + // `uatAbsenceDoesNotAffectAcceptedPhases`, "evidence kept separate, never + // folded"). Do not "fix" this test back to `percent === null` — the + // asymmetry it once demanded would be the bug, not the fix. + // + // The contract that actually matters, and is much harder to satisfy + // accidentally, is PARITY: a line-ending convention must not change what + // the audit reports. Both documents below are derived from ONE source + // string, differing only in which separator carries the line break, so the + // two fixtures cannot drift apart under later editing. + test('lineEndingConventionDoesNotChangeUatAuditOutputLoneCrMatchesLf', () => { + const LF = '\n'; + const CR = String.fromCharCode(13); + const sourceLines = [ + '---', + 'status: complete', + '---', + '', + '### 1. Alpha', + 'expected: a', + 'result: pass', + '', + 'Notes.', + '### 2. Beta', + 'expected: the export works', + 'result: blocked', + '', + ]; + + function runWithEol(eol) { + const tmpDir = createTempProject(); + const phaseDir = declarePhase(tmpDir, '1', 'Foo'); + writeVerification(phaseDir, '1', 'passed'); + writeUatDoc(phaseDir, '1', sourceLines, eol); + const payload = parseInspect(tmpDir); + cleanup(tmpDir); + return payload; + } + + const lfPayload = runWithEol(LF); + const crPayload = runWithEol(CR); + const lfPhase = lfPayload.phases[0]; + const crPhase = crPayload.phases[0]; + const describeAll = () => JSON.stringify({ lf: lfPhase, cr: crPhase }, null, 2); + + const rowIdentity = (row) => ({ test: row.test, name: row.name, result: row.result }); + const diagnosticCodes = (payload) => [...new Set(payload.diagnostics.map((d) => d.code))].sort(); + + assert.strictEqual(crPhase.uat.scope, lfPhase.uat.scope, describeAll()); + assert.strictEqual(crPhase.scope, lfPhase.scope, describeAll()); + assert.strictEqual( + crPayload.progress.accepted_phases.percent, + lfPayload.progress.accepted_phases.percent, + describeAll(), + ); + assert.deepStrictEqual(diagnosticCodes(crPayload), diagnosticCodes(lfPayload), describeAll()); + + // LOAD-BEARING (#3707-CR MINOR 2): with normalization stripped from + // src/uat.cts, `uat.scope`, `phase.scope`, `accepted_phases.percent`, and + // `diagnosticCodes` above are ALL identical between the lone-CR and LF + // payloads even while the bug is present — a lone-CR document degrades + // scope to 'truncated' on BOTH sides identically (the CR document simply + // fails to parse either row, LF parses both), so those four assertions + // pass regardless of whether the CR fix exists. The ONLY assertion below + // that actually discriminates the fix from the bug is the + // `uat.unresolved` row-identity `deepStrictEqual`: pre-fix, `crPhase.uat. + // unresolved` is `[]` while `lfPhase.uat.unresolved` contains the "Beta" + // row, so this is the one comparison that fails without the fix. Do NOT + // remove this assertion as "redundant" with the four above — removing it + // makes this whole test vacuously green under the pre-fix behavior. + assert.deepStrictEqual( + crPhase.uat.unresolved.map(rowIdentity), + lfPhase.uat.unresolved.map(rowIdentity), + describeAll(), + ); + + // Sanity: the row is genuinely surfaced on both sides, not vacuously + // absent from both (which would make the equality checks above trivially + // pass without proving anything). + assert.strictEqual(lfPhase.uat.scope, 'complete', describeAll()); + assert.strictEqual(lfPayload.progress.accepted_phases.percent, 100, describeAll()); + assert.ok(lfPhase.uat.unresolved.some((r) => r.name === 'Beta' && r.result === 'blocked'), describeAll()); + }); + + // ─── #3707-CR follow-up MINOR 1 ───────────────────────────────────────────── + // + // A lone-CR VERIFICATION.md with `status: passed` was read by + // `readVerificationStatus` (src/verification.cts) as `status: "missing"` — + // `extractFrontmatter`'s byte-0 `---\n` / `---\r\n` fence check never + // matches a lone-CR `---\r`, so the frontmatter block was invisible and the + // completed verification was reported as though the step never ran. + // Under-reports rather than over-reports (fail-safe direction), but the + // same root cause as the false-clean class fixed above: a line-ending + // convention must not change what `planning.inspect` reports. + test('loneCrVerificationStatusPassedIsNotReportedAsMissing', () => { + const tmpDir = createTempProject(); + const phaseDir = declarePhase(tmpDir, '1', 'Foo'); + const CR = String.fromCharCode(13); + writeVerification(phaseDir, '1', 'passed', CR); + + const payload = parseInspect(tmpDir); + cleanup(tmpDir); + const phase = payload.phases[0]; + + assert.strictEqual(phase.verification.status, 'passed', + `lone-CR VERIFICATION.md with status: passed must not report as missing: ${JSON.stringify(phase.verification)}`); + }); + + // ─── Multi-file degrade ───────────────────────────────────────────────────── + // + // Two files scope to the SAME phase: one carries a shortfall-only gap, one + // is entirely clean. The fold must degrade when EITHER file order is used — + // `fs.readdirSync` order is deterministically controlled via method + // monkeypatching (never mode bits; real directory order is OS/filesystem- + // dependent and would make this a flaky race), per CLAUDE.md's + // cross-platform IO-failure-injection rule. + // + // These two variants do NOT test file-order independence as a guarantee: + // `foldScope` in `buildUatRows` is monotonic (it is only ever set to + // `SCOPE.TRUNCATED`, never reset back to `SCOPE.COMPLETE`), so which file is + // visited first is structurally irrelevant to the current implementation, + // not something this test asserts. What the `shortfallFileFirst` variant + // DOES incidentally guard is a future regression that adds a reset path + // (e.g. code that sets `foldScope` back to COMPLETE upon encountering a + // later clean file) — running the shortfall file first and the clean file + // second is exactly the ordering such a bug would need to slip through. + + function writeCustomUatFile(phaseDir, fileName, status, bodyLines) { + writeAbs(path.join(phaseDir, fileName), ['---', `status: ${status}`, '---', '', ...bodyLines].join('\n')); + } + + const CLEAN_UAT_BODY = ['### 1. Alpha', 'expected: a', 'result: pass', '']; + const SHORTFALL_UAT_BODY = [ + '# UAT', '', '## Notes', '', 'How to write a row:', '', + '```', '### 1. Example Row', 'expected: x', 'result: pass', '```', '', + ]; + + for (const [label, order] of [ + ['cleanFileFirst', ['1-UAT-clean.md', '1-UAT-shortfall.md']], + ['shortfallFileFirst', ['1-UAT-shortfall.md', '1-UAT-clean.md']], + ]) { + test(`multiFileUatDegradesFoldWhenAnyFileHasShortfallOnlyGap_${label}`, (t) => { + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + const phaseDir = declarePhase(tmpDir, '1', 'Foo'); + writeVerification(phaseDir, '1', 'passed'); + writeCustomUatFile(phaseDir, '1-UAT-clean.md', 'complete', CLEAN_UAT_BODY); + writeCustomUatFile(phaseDir, '1-UAT-shortfall.md', 'complete', SHORTFALL_UAT_BODY); + + const planningInspectLib = require('../gsd-core/bin/lib/planning-inspect.cjs'); + const originalReaddirSync = fs.readdirSync; + t.mock.method(fs, 'readdirSync', function mockedReaddirSync(target, ...rest) { + const result = originalReaddirSync.call(this, target, ...rest); + if (target === phaseDir) { + const others = result.filter((f) => f !== '1-UAT-clean.md' && f !== '1-UAT-shortfall.md'); + return [...order, ...others]; + } + return result; + }); + + const payload = planningInspectLib.buildPlanningInspect(tmpDir); + const phase = payload.phases[0]; + assert.strictEqual(phase.scope, 'truncated'); + assert.ok(payload.diagnostics.some((d) => d.code === 'phase_scope_degraded' && d.subject === phase.dir)); + assert.strictEqual(payload.progress.accepted_phases.percent, null); + }); + } + test('roadmapAcceptanceIsNeverAuthoritativeOnAnyPhaseRow', (t) => { const tmpDir = createTempProject(); t.after(() => cleanup(tmpDir)); diff --git a/tests/uat-predicate.test.cjs b/tests/uat-predicate.test.cjs index 9abab0f15..b2a38c886 100644 --- a/tests/uat-predicate.test.cjs +++ b/tests/uat-predicate.test.cjs @@ -21,6 +21,7 @@ const { analyzeMarkdown, evaluateUatPassed, } = require('../gsd-core/bin/lib/uat-predicate.cjs'); +const { parseUatItemsWithStats } = require('../gsd-core/bin/lib/uat.cjs'); const { cleanup } = require('./helpers.cjs'); // ─── Helpers ────────────────────────────────────────────────────────────────── @@ -1194,6 +1195,32 @@ describe('FIX B — cross-line result: value must be on the same line', () => { 'cross-line result must yield missing (blocker)'); }); + test('result: whose value sits on a following INDENTED line → missing (pinned #3078-CR divergence)', () => { + // #3078-CR (security review follow-up): on origin/next, the old + // `/^result:\s*\[?(\w+)\]?.*$/im` regex's `\s*` is greedy and matches + // ACROSS a newline, so `result:\n blocked` parsed as `blocked` — a real + // row this shape reported 1/blocked. The split-then-match rewrite tests + // `result:` against a SINGLE already-split line, per the documented + // "value must sit on the SAME line as result:" contract (see the comment + // above RESULT_LINE_RE), so this now yields 'missing' (a parse gap, with + // the percentage withheld) instead of silently crossing the newline. + // This is a DELIBERATE, FAIL-SAFE divergence from the old cross-newline + // `\s*` behavior — pinned here so it is never "fixed" back by accident. + const content = [ + '### 1. Indented-continuation Test', + 'expected: Y', + 'result:', + ' blocked', + '', + ].join('\n'); + const items = parseUatResultItems(content); + assert.strictEqual(items.length, 1); + assert.notStrictEqual(items[0].result, 'blocked', + 'a value on a following indented line must not be captured across the newline'); + assert.strictEqual(items[0].result, 'missing', + 'result: with its value on the next (even indented) line must yield missing, not the old cross-newline capture'); + }); + test('evaluateUatPassed → passed:false for cross-line result:passed', () => { const tmpDir = makeTmpDir(); try { @@ -1457,3 +1484,173 @@ describe('evaluateUatPassed — property: wrapping in false-positive context nev ); }); }); + +// ─── #3078-CR MEDIUM: acceptance gate (uat-predicate.cjs) must AGREE with the ── +// ─── audit surface (uat.cjs's parseUatItemsWithStats) on the same bytes ─────── + +describe('#3078-CR: evaluateUatPassed agrees with the audit surface (parseUatItemsWithStats)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = makeTmpDir(); + }); + + afterEach(() => { + rmDir(tmpDir); + }); + + test('U+2028 scalar-injection: gate blocks, audit surface reports the outstanding row — they AGREE', () => { + // A `result:` line reachable only via a U+2028 LINE SEPARATOR sitting inside + // an `expected: |` block-scalar body must not be read as a genuine + // column-0 match by EITHER surface. The real, later `result: blocked` line + // is the one that must win. + const LS = String.fromCharCode(0x2028); // never a raw separator in source: a formatter that normalizes line separators would silently turn this fixture into an ordinary-character control that still passes + const body = [ + '---', + 'status: passed', + '---', + '', + '# UAT', + '', + '### 1. Alpha', + 'expected: |', + ' x' + LS + 'result: pass', + 'result: blocked', + '', + ].join('\n'); + writeFile(tmpDir, '01-alpha-UAT.md', body); + + const gateReport = evaluateUatPassed(tmpDir); + const auditReport = parseUatItemsWithStats(body); + + // AGREEMENT, asserted explicitly (not each surface independently): both + // surfaces must consider this phase NOT clean, on the same test row. + assert.equal(gateReport.passed, false, 'gate: must not accept a blocked test as passed'); + assert.equal(auditReport.items.length, 1, 'audit: the blocked row must surface as outstanding'); + assert.equal(auditReport.items[0].result, 'blocked', 'audit: must read the real result, not the injected one'); + const gateCheck = gateReport.checks.find((c) => c.test === 1); + assert.ok(gateCheck, 'gate: must record the test-1 check'); + assert.equal(gateCheck.result, 'blocked', 'gate: must read the real result, not the injected one'); + assert.equal(gateCheck.passing, false); + // Cross-surface identity: same test number, same result token. + assert.equal(gateCheck.result, auditReport.items[0].result, 'gate and audit surface must agree on the result token'); + }); + + test('H-U28 restored: a heading delimited by U+2028 (not \n) is still found and blocks', () => { + // #3078-CR MEDIUM 1 (security review follow-up): origin/next found this + // heading via an /m-anchored scan whose LineTerminator set includes + // U+2028/U+2029; a naive split('\n')-only port of that scan silently + // stopped finding it, making the gate MORE PERMISSIVE than origin/next + // (measured: HEAD passed:true/0 blockers, origin/next passed:false/1 + // blocker, for this exact shape). The heading scan now splits on + // \n/U+2028/U+2029 (a STRUCTURE frame) while the result: scan below it + // stays \n-only (an ATTRIBUTION frame, unchanged) -- so the heading is + // found, but its result: line -- separated from the heading by the same + // exotic separator -- is correctly NOT read across that boundary (that is + // the attribution guard I-U28 below exists to prove), yielding + // 'missing' rather than 'blocked'. Either token is a non-passing, + // blocking state, so the gate still BLOCKS -- the outcome origin/next + // produced, restored. + const LS = String.fromCharCode(0x2028); // never a raw separator in source: a formatter that normalizes line separators would silently turn this fixture into an ordinary-character control that still passes + const content = 'Notes.' + LS + '### 2. B' + LS + 'result: blocked'; + const items = parseUatResultItems(content); + assert.strictEqual(items.length, 1, 'a U+2028-delimited heading must still be found'); + assert.strictEqual(items[0].test, 2); + assert.strictEqual(items[0].name, 'B'); + // IDENTITY, not a proxy: the exact token. `notStrictEqual(..., 'passed')` also passes on + // 'pass', which IS in UAT_PASS_RESULTS -- so it could not catch a regression that + // attributed a PASSING result to the recovered heading, which is the whole risk here. + assert.strictEqual(items[0].result, 'missing', + 'the result: line sits across the same exotic separator, so it is correctly NOT attributed -- ' + + 'missing is a non-passing, blocking state'); + }); + + test('H-U28 restored: evaluateUatPassed BLOCKS on the U+2028-delimited heading shape', () => { + const tmpDir = makeTmpDir(); + try { + const LS = String.fromCharCode(0x2028); // never a raw separator in source: a formatter that normalizes line separators would silently turn this fixture into an ordinary-character control that still passes + const content = 'Notes.' + LS + '### 2. B' + LS + 'result: blocked'; + const body = ['---', 'status: passed', '---', '', '# UAT', '', content, ''].join('\n'); + writeFile(tmpDir, '01-h28-UAT.md', body); + const report = evaluateUatPassed(tmpDir); + assert.strictEqual(report.passed, false, 'gate must block on the U+2028-delimited heading -- origin/next parity'); + assert.ok(report.blockers.length > 0, 'a blocker must be recorded'); + } finally { + rmDir(tmpDir); + } + }); + + test('CR-fenced case: gate and audit surface (parseUatItemsWithStats) agree -- neither silently clean', () => { + // #3078-CR MEDIUM 1 follow-up evidence: a lone-CR document + // (see:CRfenceCR### 2. BCRresult: blockedCRfence) has its CRs normalized + // to \n before parsing, which turns a literal fence-marker sequence into + // a REAL fence delimiter it was not before normalization -- the row ends + // up fenced and stripped on the gate side. Both surfaces must agree this + // phase is NOT clean (the gate must not pass while the audit surface + // reports a shortfall/gap for the same document). + const crBody = ['see:', '```', '### 2. B', 'result: blocked', '```'].join('\r'); + const tmpDir = makeTmpDir(); + try { + writeFile(tmpDir, '01-crfence-UAT.md', crBody); + const gateReport = evaluateUatPassed(tmpDir); + const auditReport = parseUatItemsWithStats(crBody); + assert.strictEqual(gateReport.passed, false, 'gate must not report a clean pass for this document'); + assert.ok(auditReport.headingsSeen > 0, 'audit surface must see the heading exists'); + assert.ok(auditReport.items.length === 0 && auditReport.shortfallBlocks > 0, + 'audit surface must record the fenced row as an unresolved shortfall, not silently drop it'); + } finally { + rmDir(tmpDir); + } + }); + + test('lone-CR frontmatter: gate no longer silently drops a blocking status hidden by an unnormalized read', () => { + // A lone-CR-terminated frontmatter fence (`---\rstatus: partial\r---`) must + // still be recognised as frontmatter — the raw, unnormalized read this + // fix replaces treated the whole fence as one unbroken line, so + // `extractFrontmatter` never matched it and the blocking `status: partial` + // was silently dropped (fail-OPEN, the false-clean this fix closes). + const body = [ + '---\rstatus: partial\r---', + '', + '# UAT', + '', + '### 1. Alpha\rresult: passed', + '### 2. Beta\rresult: pass', + '', + ].join('\r'); + writeFile(tmpDir, '01-beta-UAT.md', body); + + const gateReport = evaluateUatPassed(tmpDir); + assert.equal(gateReport.passed, false, 'gate: the hidden status: partial must now block'); + assert.ok( + gateReport.blockers.some((b) => b.includes('status=partial')), + 'gate: the frontmatter status blocker must be surfaced, not silently dropped', + ); + }); + + test('clean control: a normally-passing file agrees as passed on both surfaces', () => { + const body = [ + '---', + 'status: passed', + '---', + '', + '# UAT', + '', + '### 1. Alpha', + 'result: passed', + '', + '### 2. Beta', + 'result: pass', + '', + ].join('\n'); + writeFile(tmpDir, '01-gamma-UAT.md', body); + + const gateReport = evaluateUatPassed(tmpDir); + const auditReport = parseUatItemsWithStats(body); + + // AGREEMENT: the gate accepts, and the audit surface reports NO outstanding + // (non-passing) rows for the same bytes. + assert.equal(gateReport.passed, true, 'gate: a clean file must still pass'); + assert.equal(auditReport.items.length, 0, 'audit: a clean file must have no outstanding rows'); + }); +}); diff --git a/tests/uat.test.cjs b/tests/uat.test.cjs index 8d470779e..0523b1ef1 100644 --- a/tests/uat.test.cjs +++ b/tests/uat.test.cjs @@ -6023,3 +6023,552 @@ v1.1 - Example Milestone assert.strictEqual(entry.archived_milestone, 'v0.5.0', describeAll()); }); }); + +// ─── #3707-CR security review MEDIUM: a lone CR is not a line boundary anywhere ── +// +// [FAILING-FIRST, DO NOT "FIX" src/ TO MAKE THIS PASS — see dispatch brief] +// +// CommonMark treats a lone CR (no paired LF) as a line ending — a document +// using it RENDERS as separate lines to a human reader. This parser's row +// scan (`content.split('\n')` feeding both `tokenizeHeadings` and the +// column-0 `TEST_HEADING_LINE_RE` shortfall scan, uat.cjs:1193/869) and the +// #3078 round-7 symmetry invariant (both sides of the shortfall comparison +// are whole-document, uat.cjs:1137-1166) both key on `\n` alone. A lone CR +// never becomes a boundary on EITHER side, so a `### N.` row separated from +// its predecessor only by CR is invisible to `tokenizeHeadings` (no token), +// to the raw-line shortfall scan (`TEST_HEADING_LINE_RE.test(line)` only +// matches `^`, and the whole multi-row chunk is now ONE unsplit "line" whose +// `^` sits before earlier content, not before the buried heading), AND to +// `parseGapsItems`'s own `content.split('\n')` walk. No item, no shortfall, +// no headingsSeen: a TOTAL false-clean, not merely a missed row. +const CR = String.fromCharCode(13); +const LF = String.fromCharCode(10); +const CRLF = CR + LF; + +describe('#3707-CR: a lone CR line ending must not hide an outstanding UAT row', () => { + /** + * `join(sep)` on lines already containing an embedded body — used so the + * fixture text itself stays free of literal CR characters that an editor + * or a diff viewer could silently rewrite (CLAUDE.md IO-injection rule). + */ + function bodyWith(sep) { + return [ + '---', + 'status: partial', + 'phase: 01-a', + '---', + '', + '## Tests', + '', + '### 1. Alpha', + 'expected: ok', + 'result: pass', + '', + 'Notes.', + '### 2. Beta', + 'expected: the export works', + 'result: blocked', + '', + ].join(sep); + } + + test('[RED] a lone-CR document should surface the hidden `### 2. Beta` row by full identity', () => { + const { items, headingsSeen, shortfallBlocks } = parseUatItemsWithStats(bodyWith(CR)); + const describeAll = () => JSON.stringify({ items, headingsSeen, shortfallBlocks }, null, 2); + + const beta = items.find((i) => i.name === 'Beta'); + assert.ok(beta, `hidden row 2 "Beta" absent from items: ${describeAll()}`); + assert.strictEqual(beta.test, 2, describeAll()); + assert.strictEqual(beta.name, 'Beta', describeAll()); + assert.strictEqual(beta.result, 'blocked', describeAll()); + }); + + test('[GREEN] CONTROL: the LF equivalent of the same body surfaces the identical row identity', () => { + const { items } = parseUatItemsWithStats(bodyWith(LF)); + const describeAll = () => JSON.stringify(items, null, 2); + + const beta = items.find((i) => i.name === 'Beta'); + assert.ok(beta, `row 2 "Beta" absent from LF control: ${describeAll()}`); + assert.strictEqual(beta.test, 2, describeAll()); + assert.strictEqual(beta.name, 'Beta', describeAll()); + assert.strictEqual(beta.result, 'blocked', describeAll()); + }); + + test('[GREEN] CONTROL: CRLF still parses exactly as today — no double-count, no strip', () => { + const { items, headingsSeen, shortfallBlocks } = parseUatItemsWithStats(bodyWith(CRLF)); + const describeAll = () => JSON.stringify({ items, headingsSeen, shortfallBlocks }, null, 2); + + assert.strictEqual(items.length, 1, describeAll()); + const [beta] = items; + assert.strictEqual(beta.test, 2, describeAll()); + assert.strictEqual(beta.name, 'Beta', describeAll()); + assert.strictEqual(beta.result, 'blocked', describeAll()); + assert.strictEqual(headingsSeen, 0, describeAll()); + assert.strictEqual(shortfallBlocks, 0, describeAll()); + }); + + test('[GREEN] CONTROL: a literal CR inside a fenced block is not torn into extra rows', () => { + const content = [ + '## Tests', + '', + '### 1. Alpha', + 'expected: |', + '```', + `sample${CR}line`, + '```', + 'result: pass', + '', + '### 2. Beta', + 'result: blocked', + '', + ].join(LF); + const { items, headingsSeen } = parseUatItemsWithStats(content); + const describeAll = () => JSON.stringify({ items, headingsSeen }, null, 2); + + // Row 1 ("Alpha") carries `result: pass`, which is deliberately excluded + // from `items` by design (a PASS token is the one case a heading yields + // no item without being a parse gap) — so exactly ONE item is expected + // here, not two. Asserting `headingsSeen === 0` is what proves Alpha's + // heading was still correctly SEEN and attributed, not silently dropped + // by the embedded CR splitting its fence/scalar content into extra rows. + assert.strictEqual(items.length, 1, describeAll()); + const beta = items.find((i) => i.test === 2); + assert.ok(beta, `row 2 absent: ${describeAll()}`); + assert.strictEqual(beta.name, 'Beta', describeAll()); + assert.strictEqual(beta.result, 'blocked', describeAll()); + assert.strictEqual(headingsSeen, 0, describeAll()); + }); + + test('[GREEN] CONTROL: a literal CR inside an `expected: |` block-scalar body is not torn into extra rows', () => { + const content = [ + '## Tests', + '', + '### 1. Alpha', + 'expected: |', + ` line one${CR}still the scalar`, + ' line two', + 'result: pass', + '', + '### 2. Beta', + 'result: blocked', + '', + ].join(LF); + const { items, headingsSeen } = parseUatItemsWithStats(content); + const describeAll = () => JSON.stringify({ items, headingsSeen }, null, 2); + + // Same PASS-exclusion rule as the fenced-CR control above: Alpha's + // `result: pass` yields no item by design, so exactly ONE item (Beta) is + // expected, and `headingsSeen === 0` proves Alpha was still attributed. + assert.strictEqual(items.length, 1, describeAll()); + const beta = items.find((i) => i.test === 2); + assert.ok(beta, `row 2 absent: ${describeAll()}`); + assert.strictEqual(beta.name, 'Beta', describeAll()); + assert.strictEqual(beta.result, 'blocked', describeAll()); + assert.strictEqual(headingsSeen, 0, describeAll()); + }); + + test('[RED] boundary: a lone CR at the very start of the document also hides the very first row', () => { + // A second manifestation of the same defect, not a distinct one: the + // leading CR is not a `\n`, so `content.split('\n')` yields a single + // first "line" of `"\r### 1. Alpha"` — the heading text no longer sits + // at column 0 of that split unit, so `TEST_HEADING_LINE_RE`'s `^#{3}` + // anchor and the tokenizer's own column-0 check both refuse it. + const content = CR + [ + '### 1. Alpha', + 'result: blocked', + '', + ].join(LF); + const { items } = parseUatItemsWithStats(content); + const describeAll = () => JSON.stringify(items, null, 2); + + const alpha = items.find((i) => i.name === 'Alpha'); + assert.ok(alpha, `row "Alpha" absent: ${describeAll()}`); + assert.strictEqual(alpha.test, 1, describeAll()); + assert.strictEqual(alpha.result, 'blocked', describeAll()); + }); + + test('[GREEN] boundary: a lone CR at the very end of the document is harmless', () => { + const content = [ + '### 1. Alpha', + 'result: blocked', + ].join(LF) + CR; + const { items } = parseUatItemsWithStats(content); + const describeAll = () => JSON.stringify(items, null, 2); + + const alpha = items.find((i) => i.name === 'Alpha'); + assert.ok(alpha, `row "Alpha" absent: ${describeAll()}`); + assert.strictEqual(alpha.test, 1, describeAll()); + assert.strictEqual(alpha.result, 'blocked', describeAll()); + }); + + test('[RED] boundary: two consecutive lone CRs between rows still hides the row, though the whole-document shortfall scan happens to flag it', () => { + // With no `\n` anywhere in this fixture, `content.split('\n')` returns + // ONE line: the entire document text. That single line legitimately + // starts with `### 1. Alpha` (true string start, column 0), so the raw + // `TEST_HEADING_LINE_RE` shortfall scan (uat.cjs:1193) counts exactly one + // shaped heading line for the WHOLE document, while the tokenizer-backed + // `subHeadings` side finds none it can attribute — `headingsSeen`/ + // `shortfallBlocks` land at 1, so this shape is not a TOTAL silent + // false-clean like the primary repro. But `items` is still empty: the + // "Beta" row's own identity (number, name, result) is not recovered by + // that shortfall count, which is why this assertion is on identity, not + // presence of a nonzero counter. + const content = [ + '### 1. Alpha', + 'result: pass', + '', + '### 2. Beta', + 'result: blocked', + '', + ].join(CR + CR); + const { items } = parseUatItemsWithStats(content); + const describeAll = () => JSON.stringify(items, null, 2); + + const beta = items.find((i) => i.name === 'Beta'); + assert.ok(beta, `row 2 "Beta" absent: ${describeAll()}`); + assert.strictEqual(beta.test, 2, describeAll()); + assert.strictEqual(beta.result, 'blocked', describeAll()); + }); +}); + +// ─── #3707-CR follow-up MAJOR: the two OTHER cmdAuditUat ingresses ───────────── +// +// The original #3707-CR fix normalized line endings inside two of +// `cmdAuditUat`'s FOUR parsers (`parseUatItemsWithStats`, `parseCurrentTest`) +// and declared the class closed. It was not: `parseVerificationItems` +// (VERIFICATION.md) and `parseDeferredItems` (deferred-items.md) are reached +// from the SAME function via their own, separately unnormalized +// `fs.readFileSync` calls, so a lone-CR VERIFICATION.md or deferred-items.md +// hit the identical total false-clean this issue exists to close. The fix +// this time is at the READ BOUNDARY (`readNormalizedDocument` in +// src/uat.cts), not per-parser — these tests drive the full CLI end-to-end so +// they exercise that boundary, not a parser function directly. +describe('#3707-CR follow-up MAJOR: VERIFICATION.md and deferred-items.md ingresses normalize at the read boundary', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + function audit() { + const result = runGsdTools('audit-uat --raw', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + return JSON.parse(result.output); + } + + function deferredBody(eol) { + return [ + '## Deferred Items', + '', + '- First deferred item, still open.', + '- Second deferred item, still open.', + ].join(eol); + } + + test('[RED] a lone-CR deferred-items.md surfaces both items by identity', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-foundation'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, 'deferred-items.md'), deferredBody(CR)); + + const output = audit(); + const describeAll = () => JSON.stringify(output, null, 2); + + const entry = output.results.find((r) => r.file === 'deferred-items.md'); + assert.ok(entry, `deferred-items.md entry absent: ${describeAll()}`); + const names = entry.items.map((i) => ({ name: i.name, result: i.result })); + assert.deepStrictEqual(names, [ + { name: 'First deferred item, still open.', result: 'unresolved' }, + { name: 'Second deferred item, still open.', result: 'unresolved' }, + ], describeAll()); + assert.strictEqual(output.summary.total_items, 2, describeAll()); + }); + + test('[GREEN] CONTROL: the LF twin of the same deferred-items.md surfaces the identical items', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-foundation'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, 'deferred-items.md'), deferredBody(LF)); + + const output = audit(); + const describeAll = () => JSON.stringify(output, null, 2); + + const entry = output.results.find((r) => r.file === 'deferred-items.md'); + assert.ok(entry, `deferred-items.md entry absent: ${describeAll()}`); + const names = entry.items.map((i) => ({ name: i.name, result: i.result })); + assert.deepStrictEqual(names, [ + { name: 'First deferred item, still open.', result: 'unresolved' }, + { name: 'Second deferred item, still open.', result: 'unresolved' }, + ], describeAll()); + assert.strictEqual(output.summary.total_items, 2, describeAll()); + }); + + test('[GREEN] CONTROL: a CRLF deferred-items.md is unchanged (no double-count, no strip)', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-foundation'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, 'deferred-items.md'), deferredBody(CRLF)); + + const output = audit(); + const describeAll = () => JSON.stringify(output, null, 2); + + const entry = output.results.find((r) => r.file === 'deferred-items.md'); + assert.ok(entry, `deferred-items.md entry absent: ${describeAll()}`); + const names = entry.items.map((i) => ({ name: i.name, result: i.result })); + assert.deepStrictEqual(names, [ + { name: 'First deferred item, still open.', result: 'unresolved' }, + { name: 'Second deferred item, still open.', result: 'unresolved' }, + ], describeAll()); + assert.strictEqual(output.summary.total_items, 2, describeAll()); + }); + + function verificationBody(eol) { + return [ + '---', + 'status: human_needed', + 'phase: 02-auth', + '---', + '', + '## Human Verification', + '', + '1. Test SSO login with Google account', + '2. Test password reset flow end-to-end', + '', + ].join(eol); + } + + test('[RED] a lone-CR VERIFICATION.md (status: human_needed) surfaces both human-verification items by identity', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '02-auth'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '02-VERIFICATION.md'), verificationBody(CR)); + + const output = audit(); + const describeAll = () => JSON.stringify(output, null, 2); + + const entry = output.results.find((r) => r.type === 'verification'); + assert.ok(entry, `VERIFICATION entry absent: ${describeAll()}`); + const names = entry.items.map((i) => i.name); + assert.deepStrictEqual(names, [ + 'Test SSO login with Google account', + 'Test password reset flow end-to-end', + ], describeAll()); + assert.strictEqual(output.summary.total_items, 2, describeAll()); + }); + + test('[GREEN] CONTROL: the LF twin of the same VERIFICATION.md surfaces the identical items', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '02-auth'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '02-VERIFICATION.md'), verificationBody(LF)); + + const output = audit(); + const describeAll = () => JSON.stringify(output, null, 2); + + const entry = output.results.find((r) => r.type === 'verification'); + assert.ok(entry, `VERIFICATION entry absent: ${describeAll()}`); + const names = entry.items.map((i) => i.name); + assert.deepStrictEqual(names, [ + 'Test SSO login with Google account', + 'Test password reset flow end-to-end', + ], describeAll()); + assert.strictEqual(output.summary.total_items, 2, describeAll()); + }); + + test('[GREEN] CONTROL: a CRLF VERIFICATION.md is unchanged (no double-count, no strip)', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '02-auth'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '02-VERIFICATION.md'), verificationBody(CRLF)); + + const output = audit(); + const describeAll = () => JSON.stringify(output, null, 2); + + const entry = output.results.find((r) => r.type === 'verification'); + assert.ok(entry, `VERIFICATION entry absent: ${describeAll()}`); + const names = entry.items.map((i) => i.name); + assert.deepStrictEqual(names, [ + 'Test SSO login with Google account', + 'Test password reset flow end-to-end', + ], describeAll()); + assert.strictEqual(output.summary.total_items, 2, describeAll()); + }); + + // The end-to-end phase carrying BOTH a VERIFICATION.md and a + // deferred-items.md, written twice from one source (LF and lone-CR), + // exercising ALL FOUR ingresses in one audit-uat run at once. + test('[RED] a phase with both VERIFICATION.md and deferred-items.md: lone-CR and LF produce identical audit output', () => { + function build(eol) { + const dir = createTempProject(); + const phaseDir = path.join(dir, '.planning', 'phases', '03-combo'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '03-VERIFICATION.md'), verificationBody(eol)); + fs.writeFileSync(path.join(phaseDir, 'deferred-items.md'), deferredBody(eol)); + return dir; + } + + const lfDir = build(LF); + const crDir = build(CR); + try { + const lfResult = runGsdTools('audit-uat --raw', lfDir); + const crResult = runGsdTools('audit-uat --raw', crDir); + assert.ok(lfResult.success, `LF run failed: ${lfResult.error}`); + assert.ok(crResult.success, `CR run failed: ${crResult.error}`); + const lfOutput = JSON.parse(lfResult.output); + const crOutput = JSON.parse(crResult.output); + const describeAll = () => JSON.stringify({ lf: lfOutput, cr: crOutput }, null, 2); + + assert.strictEqual(lfOutput.summary.total_files, 2, describeAll()); + assert.strictEqual(lfOutput.summary.total_items, 4, describeAll()); + assert.strictEqual(crOutput.summary.total_files, lfOutput.summary.total_files, describeAll()); + assert.strictEqual(crOutput.summary.total_items, lfOutput.summary.total_items, describeAll()); + assert.strictEqual(crOutput.summary.parse_gap_files, lfOutput.summary.parse_gap_files, describeAll()); + + const rowIdentity = (r) => ({ + type: r.type, + items: r.items.map((i) => ({ name: i.name, result: i.result })).sort((a, b) => a.name.localeCompare(b.name)), + }); + assert.deepStrictEqual( + crOutput.results.map(rowIdentity).sort((a, b) => a.type.localeCompare(b.type)), + lfOutput.results.map(rowIdentity).sort((a, b) => a.type.localeCompare(b.type)), + describeAll(), + ); + } finally { + cleanup(lfDir); + cleanup(crDir); + } + }); +}); + +// #3078-CR: a column-0 line-terminator boundary defect in +// `parseUatItemsWithStats`'s `result:` scan, reproduced directly against the +// LIVE built copy (`../gsd-core/bin/lib/uat.cjs`, imported at the top of this +// file) rather than through a rebuilt fixture, so these rows fail against the +// ACTUAL shipped parser, not a stale mental model of it. +// +// DEFECT A (fixed): `/^result:.../im` treats U+2028 LINE SEPARATOR and U+2029 +// PARAGRAPH SEPARATOR as line-start boundaries (native JS `/m` behaviour), +// but `normalizeLineEndings` (core-utils.cjs) folds only `\r`/`\r\n` and never +// touches U+2028/U+2029, and neither does `split('\n')` or the heading +// tokenizer. A `result:`-shaped line living INSIDE an `expected: |` scalar +// body, immediately after one of these separators, was therefore read as a +// real line start by the regex engine and — because `.match()` without `/g` +// returns the LEFTMOST match in the whole string — won over a genuine +// column-0 `result:` line appearing later, discarding it with no gap raised. +// Fixed by testing each `split('\n')`-produced line individually instead of +// running an `/m`-anchored regex over the whole block: `split('\n')` never +// treats U+2028/U+2029 as a delimiter, so neither can manufacture a line +// start. +// +// A "defect B" (more than one column-0 `result:` line reported as an +// ambiguous parse gap rather than resolving to the first) was attempted and +// REVERTED: its boundary-truncation heuristic mistook an indented `### N.` +// living inside a legitimate block scalar for a heading boundary, which +// broke every scalar/indent guard this module has (see the `#3078` scalar +// guard tests elsewhere in this file). Two column-0 `result:` lines resolve +// to the FIRST one — the pre-existing, pinned behaviour — see controls B3/B4 +// below. +describe('parseUatItemsWithStats — result: line-scan boundary defects (#3078-CR)', () => { + const LINE_SEPARATOR = String.fromCharCode(0x2028); + const PARAGRAPH_SEPARATOR = String.fromCharCode(0x2029); + + // Row 1 ("### 1. Alpha") by IDENTITY: test number AND name AND result. + // A bare count or a bare `result` check both pass for the wrong reason — + // e.g. a phantom row from the scalar's OWN `result: pass` clause matching + // `items.length === 1` just as readily as the real blocked row would. + function findAlphaBlocked(items) { + return items.find((i) => i.test === 1 && i.name === 'Alpha' && i.result === 'blocked'); + } + function findAnyPassItem(items) { + return items.find((i) => i.result === 'pass' || i.result === 'passed'); + } + + function defectADoc(marker) { + // An `expected: |` scalar body whose text ends in "...xresult: + // pass", followed by the block's REAL column-0 "result: blocked" line. + return '## Tests\n\n### 1. Alpha\nexpected: |\n x' + marker + 'result: pass\nresult: blocked\n'; + } + + test('[RED] A1: U+2028 inside an expected scalar must not swallow the column-0 blocked row', () => { + const { items, headingsSeen } = parseUatItemsWithStats(defectADoc(LINE_SEPARATOR)); + const describeAll = () => JSON.stringify({ items, headingsSeen }, null, 2); + + assert.ok(findAlphaBlocked(items), `expected outstanding row 1/Alpha/blocked absent: ${describeAll()}`); + assert.strictEqual(findAnyPassItem(items), undefined, `a phantom pass item must not be emitted: ${describeAll()}`); + }); + + test('[RED] A2: U+2029 inside an expected scalar must not swallow the column-0 blocked row', () => { + const { items, headingsSeen } = parseUatItemsWithStats(defectADoc(PARAGRAPH_SEPARATOR)); + const describeAll = () => JSON.stringify({ items, headingsSeen }, null, 2); + + assert.ok(findAlphaBlocked(items), `expected outstanding row 1/Alpha/blocked absent: ${describeAll()}`); + assert.strictEqual(findAnyPassItem(items), undefined, `a phantom pass item must not be emitted: ${describeAll()}`); + }); + + test('[CONTROL] A3: an ordinary (non-line-terminator) marker in the same position still yields the blocked row', () => { + // Proves A1/A2 are a SEPARATOR defect, not a content defect: swap the + // exotic separator for two literal "@@" characters, which JS never + // treats as a line terminator under any regex flag. + const { items, headingsSeen } = parseUatItemsWithStats(defectADoc('@@')); + const describeAll = () => JSON.stringify({ items, headingsSeen }, null, 2); + + assert.ok(findAlphaBlocked(items), `control document must still surface 1/Alpha/blocked: ${describeAll()}`); + assert.strictEqual(findAnyPassItem(items), undefined, describeAll()); + }); + + test('[CONTROL] A4: U+2028 living in ordinary prose (not faking a line start) parses unaffected', () => { + // The separator sits between two prose words, never immediately before a + // "result:"-shaped token, so it cannot fake a line start that matters — + // this must parse exactly as it does today, both before and after any + // future fix to the boundary handling. + const doc = '## Tests\n\n### 1. Alpha\nexpected: |\n some prose' + LINE_SEPARATOR + 'continues here\nresult: blocked\n'; + const { items, headingsSeen } = parseUatItemsWithStats(doc); + const describeAll = () => JSON.stringify({ items, headingsSeen }, null, 2); + + assert.ok(findAlphaBlocked(items), `legitimate U+2028 content must not perturb parsing: ${describeAll()}`); + assert.strictEqual(items.length, 1, `no extra/phantom item may appear: ${describeAll()}`); + assert.strictEqual(headingsSeen, 0, describeAll()); + }); + + test('[CONTROL] B3: a block with exactly one result: line is unchanged', () => { + const doc = '### 1. Alpha\nexpected: ok\nresult: blocked\n'; + const { items, headingsSeen } = parseUatItemsWithStats(doc); + const describeAll = () => JSON.stringify({ items, headingsSeen }, null, 2); + + assert.strictEqual(items.length, 1, describeAll()); + assert.ok(findAlphaBlocked(items), `unambiguous single-result block must still surface 1/Alpha/blocked: ${describeAll()}`); + assert.strictEqual(headingsSeen, 0, describeAll()); + }); + + test('[CONTROL] B4: a result: line inside a fenced code sample does not count as a second column-0 occurrence', () => { + // The fenced "result: pass" sample line is document content (a fenced + // code block is stripped before the result-line scan runs), not a + // second real result declaration — only the genuine column-0 + // "result: blocked" line below the fence is the row's outcome. + const doc = '### 1. Alpha\nexpected: ok\n```\nresult: pass\n```\nresult: blocked\n'; + const { items, headingsSeen } = parseUatItemsWithStats(doc); + const describeAll = () => JSON.stringify({ items, headingsSeen }, null, 2); + + assert.strictEqual(items.length, 1, describeAll()); + assert.ok(findAlphaBlocked(items), `fenced sample result: line must not compete with the real row: ${describeAll()}`); + assert.strictEqual(findAnyPassItem(items), undefined, describeAll()); + assert.strictEqual(headingsSeen, 0, describeAll()); + }); + + test('[REGRESSION] a column-0 result: line whose trailing text contains U+2028 parses identically to its plain-LF twin', () => { + // Final review MINOR 1: the per-line pattern kept `.*$` after the fix + // above dropped `/m`, and `.` never matches U+2028/U+2029, so `$` was + // unreachable on a line whose TRAILING text (after the token) contained + // one of these separators — the line failed to match at all. Compare by + // IDENTITY (test number AND name AND result) against the plain-LF + // equivalent, not just presence/count, per this suite's own convention. + const withSeparator = '### 1. Alpha\nexpected: ok\nresult: blocked' + LINE_SEPARATOR + 'trailing note\n'; + const plainLf = '### 1. Alpha\nexpected: ok\nresult: blocked trailing note\n'; + + const withSeparatorResult = parseUatItemsWithStats(withSeparator); + const plainLfResult = parseUatItemsWithStats(plainLf); + const describeAll = () => JSON.stringify({ withSeparatorResult, plainLfResult }, null, 2); + + assert.ok(findAlphaBlocked(withSeparatorResult.items), `expected outstanding row 1/Alpha/blocked absent: ${describeAll()}`); + assert.deepStrictEqual(withSeparatorResult.items, plainLfResult.items, `must match the plain-LF twin by identity: ${describeAll()}`); + assert.strictEqual(withSeparatorResult.headingsSeen, plainLfResult.headingsSeen, describeAll()); + }); +});