diff --git a/.changeset/zesty-ibex-hop.md b/.changeset/zesty-ibex-hop.md new file mode 100644 index 000000000..baad828c0 --- /dev/null +++ b/.changeset/zesty-ibex-hop.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3420 +--- +**Plan files with Windows-style CRLF line endings now correctly enforce their `must_haves` contract** — `truths`, `artifacts`, `key_links`, and `prohibitions` blocks previously parsed to an empty list on any CRLF-authored plan file, silently degrading goal-backward verification to LLM-derived truths instead of the authored contract, with no error surfaced for the most common failure shape. (#3360) diff --git a/.gitignore b/.gitignore index cb90eb946..59f94d3ca 100644 --- a/.gitignore +++ b/.gitignore @@ -197,6 +197,7 @@ build/ /gsd-core/bin/lib/planning-scope.cjs /gsd-core/bin/lib/planning-snapshot.cjs /gsd-core/bin/lib/pattern.cjs +/gsd-core/bin/lib/text-lines.cjs /gsd-core/bin/lib/health-diagnostic-types.cjs /gsd-core/bin/lib/health-diagnostic.cjs /gsd-core/bin/lib/health-diagnostic-rules/root-existence.cjs diff --git a/CONTEXT.md b/CONTEXT.md index d5ecd0d49..bd89a58b2 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -106,6 +106,9 @@ Leaf module owning the frozen `SCOPE` discriminator (`COMPLETE` / `TRUNCATED` / ### Pattern Module Leaf module owning the construction of a regex from a **runtime value**, per ADR-3212 §1 (epic #3212 Phase 1, #3412). Exposes `escapeRegex(value) → string` and `literalPattern(value, flags?) → RegExp`. `escapeRegex` **delegates to the built-in `RegExp.escape`** (TC39 Stage 4, ES2026, Node 24+) — it is deliberately NOT an implementation, which is the whole point: the ~39 hand-rolled copies it replaces existed because every author re-derived the metacharacter set, and TC39 standardized the primitive precisely because userland versions "miss edge cases." `escapeRegex` is the primary export (the large majority of call sites build a regex *source string* and interpolate it into a larger pattern); `literalPattern` is the minority convenience for the `new RegExp(escapeRegex(v))` shape. Pure and import-free — a leaf, so any consumer can depend on it without a cycle (mirrors the Planning Scope and Phase Id modules' position). **Behavioral note, measured not assumed:** `RegExp.escape` is a *superset* escaper and produces different pattern SOURCE TEXT than the hand-rolled class did — it hex-escapes the leading character of nearly every string (`"abc"` → `"\x61bc"`), plus `-`, space, `/`, and control chars. It is **match-equivalent** (verified by a seeded `fast-check` property test against the deleted implementation as oracle, plus a fixed corpus), so no consumer's matching behavior changes; but anything asserting on pattern text rather than match results does. It also **fixes a latent bug as a side effect**: a hyphen-bearing value interpolated into a character class previously formed a real range (`[a-z]` built from an escaped `"a-z"` matched `"m"`), and no longer does. Enforced by `eslint-rules/no-adhoc-regex-escape.cjs` (fires on the `.replace(, '\\$&')` shape anywhere outside this module, matching on shape rather than exact bytes, and on `new RegExp` built from an unescaped runtime value; exempts reviewed pattern-fragment constants such as `PHASE_NUMBER_TOKEN_SOURCE` by structural provenance — a module-scope const with a static initializer — not by name alone) plus `scripts/lint-no-adhoc-regex-escape.cjs`, a whole-tree companion covering directories ESLint's globs miss. Requires `engines.node >= 24.0.0`; a seam test asserts `typeof RegExp.escape === 'function'` so the floor and the capability cannot silently diverge. Source of truth: `gsd-core/bin/lib/pattern.cjs` (generated from `src/pattern.cts`). Design: `.gsd/phase/chore-3412-pattern-seam/40-design.md`. +### Text Lines Module +Leaf module owning `\r?\n` line-terminator splitting and CRLF normalization, per ADR-3212 §3 (epic #3212 Phase 2, #3413). Exposes `splitLines(content) → string[]` (splits on `\r\n` or `\n`; a lone `\r` is not a delimiter), `normalizeEol(content) → string` (strips every `\r`, matching the four `scripts/gen-*.cjs` `--check` copies it replaces), `detectEol(content) → '\n' | '\r\n'` (dominant terminator, `'\r\n'`-default on a tie or no terminator), and `joinLines(lines, eol?) → string` (inverse of `splitLines`; round-trips byte-for-byte with `detectEol`). Closes #3360: `parseMustHavesBlock` (`src/frontmatter.cts`) matched `^(\s*)must_haves:\s*$` and a sibling block-header regex against the WHOLE multi-line YAML string under `/m`, and `\r` is its own ECMA-262 LineTerminator — a greedy `\s*` anchored on `^` could cross a CRLF boundary and inflate the captured indent by one character, tripping the nesting guard and silently returning `[]` for every `must_haves` block on a CRLF plan file. The fix reroutes both lookups through split-then-match (`splitLines` first, then match per already-split line), which cannot straddle the delimiter that produced it. Pure and import-free — a leaf, so any consumer can depend on it without a cycle (mirrors the Pattern Module's position). Enforced going forward by the widened `eslint-rules/no-crlf-fragile-split.cjs` (now also scanned against `src/**/*.cts`, not only `tests/`). Source of truth: `gsd-core/bin/lib/text-lines.cjs` (generated from `src/text-lines.cts`). Design: `.gsd/phase/chore-3413-text-lines-seam/40-design.md`. + ### Planning Snapshot Module Module owning the parsed projection of `.planning/` that a diagnostic rule may read, per ADR-3180 §8.1 (Decision 8, Phase 10, #3308). `buildPlanningSnapshot(cwd) → PlanningSnapshot` is composed EXCLUSIVELY from the already-consolidated §7 owners — `getMilestoneInfo` (Roadmap Parser Module), `listMilestonePhaseDirs` (Phase Locator Module), `isPhaseComplete` (Verification Module), `scanPhasePlans` (Plan Scan Module), `stateFieldValue`/`stateCurrentPositionSlice` (STATE.md Document Module), `planningPaths` (Planning Workspace Module) — and introduces no new semantic derivation of its own. `PlanningSnapshot` exposes `milestone`/`phaseDirs`/`phases`/`currentPhaseLabel`, each a `{value, scope}` pair per the Planning Scope Module's frozen `SCOPE` enum; `phases` additionally carries a `PhaseSnapshot[]` (`dir`, `complete`, `verificationStatus`, `planCount`, `summaryCount`, `scope`). The one new piece of logic this module adds is `worstScope(...scopes) → Scope`, a pure severity-ordered combinator (`UNREADABLE` > `UNSCOPED` > `TRUNCATED` > `COMPLETE`) that folds several independently-scoped owner answers about the same phase directory into one composite signal — NOT a re-derivation of any owner (each owner's own algorithm is untouched; only their already-computed `scope` verdicts are combined), but new coordination logic no single owner has the visibility to express. Every exposed field carries PARSED values only, never raw document text — this is structural, not advisory: a diagnostic rule given only the parsed value cannot re-derive a field's location the way `#3162`'s three inert `Current Phase` literal-search predicates did. Read failures on STATE.md (exists-but-unreadable, distinct from absent) are reported via the Unusable Input Diagnostic Module's `warnUnusableInput(UNUSABLE_REASON.STATE_UNREADABLE)`. Guarded by `scripts/lint-planning-snapshot-bypass-drift.cjs` (ratcheted per Decision 4(e), scoped to `DIAGNOSTIC_RULE_FUNCTIONS` — currently `cmdValidateHealth` in `src/verify.cts` only, acknowledging its existing raw `.planning/` reads as debt owned by Phase 11, #3309, which migrates it onto this snapshot). Source of truth: `gsd-core/bin/lib/planning-snapshot.cjs` (generated from `src/planning-snapshot.cts`). Design: `.gsd/phase/refactor-3308-planning-snapshot-parsed-projection/40-design.md`. diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index ff55391dc..b7c504f61 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -498,6 +498,7 @@ "task-command-router.cjs", "teams-status.cjs", "template.cjs", + "text-lines.cjs", "uat-predicate.cjs", "uat.cjs", "ui-consideration-probe.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 981cb8aa9..f91f85a78 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -598,6 +598,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `surface.cjs` | Runtime surface module — manages the runtime enable/disable surface state independently of the install-time profile marker (ADR-0011 Phase 2) | | `task-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools task` | | `template.cjs` | Template selection and filling with variable substitution | +| `text-lines.cjs` | Line-terminator handling seam — `splitLines`/`normalizeEol`/`detectEol`/`joinLines`, the sole owner of `\r?\n` splitting and CRLF normalization; closes #3360's split-then-match fix in `frontmatter.cjs` (ADR-3212 §3, epic #3212 Phase 2, #3413) | | `normalize-test-command.cjs` | Normalizes a resolved test command to a one-shot form so a watch-mode runner (vitest/jest) cannot hang a verification gate (#1857); shared by all four test-command gates (regression, post-merge, audit-fix, verify-phase) | | `uat.cjs` | UAT file parsing, verification debt tracking, audit-uat support | | `uat-predicate.cjs` | UAT-passed predicate — markdown-aware evaluation of HUMAN-UAT results; returns pass only when all required checks pass; ignores false-positive contexts (frontmatter, fenced code, blockquotes, HTML comments) | diff --git a/eslint-rules/no-crlf-fragile-split.cjs b/eslint-rules/no-crlf-fragile-split.cjs index 30ba887c7..fdd4443c7 100644 --- a/eslint-rules/no-crlf-fragile-split.cjs +++ b/eslint-rules/no-crlf-fragile-split.cjs @@ -14,7 +14,7 @@ * (transitively) a `readFileSync`/`fs.readFileSync` result — directly, * via a chain, or via an Identifier that scope-resolves to a variable * initialized from readFileSync. - * Message: use `.split(/\r?\n/)`. + * Message: use `splitLines()` from `src/text-lines.cts`. * * G2/G3 — a RegExpLiteral whose pattern contains a bare `\n` (a `\n` not * part of `\r?\n` / `\r\n` / `[\r\n]` etc.) used as the pattern of a @@ -22,7 +22,8 @@ * call on a readFileSync-derived receiver. ALSO flags a RegExpLiteral * with a bare `\n` whose source contains a markdown fence (```) or a * frontmatter anchor (`^---`), since those shapes target file content. - * Message: use `\r?\n` (Windows git-autocrlf yields `\r\n`). + * Message: use `splitLines()` from `src/text-lines.cts` (or the raw + * `\r?\n` regex, for a file that cannot import the compiled seam). * * ## Known boundaries * @@ -53,11 +54,11 @@ const rule = { crlfFragileSplit: 'Splitting on literal "\\n" on readFileSync content is CRLF-fragile ' + '(DEFECT.WINDOWS-CRLF-TEST-PORTABILITY): Windows git-autocrlf yields "\\r\\n" ' + - 'line endings. Use .split(/\\r?\\n/) instead.', + 'line endings. Use splitLines() from src/text-lines.cts instead.', crlfFragileRegex: 'RegExp with a bare "\\n" on readFileSync content is CRLF-fragile ' + '(DEFECT.WINDOWS-CRLF-TEST-PORTABILITY): Windows git-autocrlf yields "\\r\\n" ' + - 'line endings. Use \\r?\\n (or [\\r\\n]) instead.', + 'line endings. Use splitLines() from src/text-lines.cts instead.', }, }, diff --git a/eslint.config.mjs b/eslint.config.mjs index 11bb01aa8..850c377e9 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -138,6 +138,7 @@ export default tseslint.config( 'gsd-core/bin/lib/state-document.cjs', 'gsd-core/bin/lib/planning-snapshot.cjs', 'gsd-core/bin/lib/pattern.cjs', + 'gsd-core/bin/lib/text-lines.cjs', 'gsd-core/bin/lib/health-diagnostic-types.cjs', 'gsd-core/bin/lib/health-diagnostic.cjs', 'gsd-core/bin/lib/health-diagnostic-rules/root-existence.cjs', @@ -314,6 +315,8 @@ export default tseslint.config( // escape-all-metachars .replace() helper or an unrouted new RegExp() // from a runtime value. 'local/no-adhoc-regex-escape': 'error', + // ADR-3212 Phase 2 (#3413): widen the CRLF-fragile-split prohibition from tests/ to src/. + 'local/no-crlf-fragile-split': 'error', // ADR-1703 Phase 5: flag path-returning calls interpolated into content // (markdown @-references, workflow files, generated docs) without POSIX // normalization. Promoted to 'error' after precision review (path.basename diff --git a/scripts/gen-capability-registry.cjs b/scripts/gen-capability-registry.cjs index c22597a0a..dbd167dc4 100644 --- a/scripts/gen-capability-registry.cjs +++ b/scripts/gen-capability-registry.cjs @@ -19,6 +19,7 @@ const fs = require('node:fs'); const path = require('node:path'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); +const { normalizeEol } = require('../gsd-core/bin/lib/text-lines.cjs'); const ROOT = path.resolve(__dirname, '..'); const CAPABILITIES_DIR = path.join(ROOT, 'capabilities'); @@ -808,19 +809,6 @@ function stripGeneratedComment(content) { .join('\n'); } -/** - * Normalize line endings to LF. - * The generator always writes LF, but Windows git (autocrlf) checks out committed files with - * CRLF. The --check comparison must be line-ending-agnostic so it only fails on REAL content - * differences, not on checkout-introduced whitespace differences. - * - * @param {string} content - * @returns {string} - */ -function normalizeLineEndings(content) { - return content.replace(/\r/g, ''); -} - // ─── Main ───────────────────────────────────────────────────────────────────── @@ -859,7 +847,7 @@ function main() { } const committed = fs.readFileSync(REGISTRY_PATH, 'utf8'); - if (normalizeLineEndings(stripGeneratedComment(committed)) !== normalizeLineEndings(stripGeneratedComment(live))) { + if (normalizeEol(stripGeneratedComment(committed)) !== normalizeEol(stripGeneratedComment(live))) { process.stderr.write( 'gsd-core/bin/lib/capability-registry.cjs is stale. Run:\n' + ' node scripts/gen-capability-registry.cjs --write\n', @@ -931,7 +919,7 @@ module.exports = { serializeRegistry, computeRequiresClosure, topoSortSteps, - normalizeLineEndings, + normalizeLineEndings: normalizeEol, stripGeneratedComment, validateConfigSliceEntry, VALID_CONFIG_SLICE_TYPES, diff --git a/scripts/gen-context-index.cjs b/scripts/gen-context-index.cjs index aa6e512cf..585e916f9 100644 --- a/scripts/gen-context-index.cjs +++ b/scripts/gen-context-index.cjs @@ -46,6 +46,7 @@ const fs = require('node:fs'); const path = require('node:path'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); +const { normalizeEol } = require('../gsd-core/bin/lib/text-lines.cjs'); const ROOT = path.resolve(__dirname, '..'); const CONTEXT_PREDICATES_LIB_PATH = path.join(ROOT, 'gsd-core', 'bin', 'lib', 'context-predicates.cjs'); @@ -144,16 +145,6 @@ function serializeIndex(index) { return JSON.stringify(index, null, 2) + '\n'; } -/** - * Normalize line endings to LF for CRLF-agnostic full-content comparison. - * - * @param {string} content - * @returns {string} - */ -function normalizeLineEndings(content) { - return content.replace(/\r/g, ''); -} - /** * Extract the sorted list of duplicate predicate ids from a built index. * @@ -431,7 +422,7 @@ module.exports = { readContextMarkdown, buildFreshIndex, serializeIndex, - normalizeLineEndings, + normalizeLineEndings: normalizeEol, duplicateIds, checkReport, parseArgs, diff --git a/scripts/gen-loop-host-contract.cjs b/scripts/gen-loop-host-contract.cjs index 012d1ad72..1347df829 100644 --- a/scripts/gen-loop-host-contract.cjs +++ b/scripts/gen-loop-host-contract.cjs @@ -21,6 +21,7 @@ const path = require('node:path'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); const { escapeRegex: escapeRegExp } = require('../gsd-core/bin/lib/pattern.cjs'); +const { normalizeEol } = require('../gsd-core/bin/lib/text-lines.cjs'); const ROOT = path.resolve(__dirname, '..'); const WORKFLOWS_DIR = path.join(ROOT, 'gsd-core', 'workflows'); @@ -369,21 +370,6 @@ function serializeContract(contract) { return lines.join('\n'); } -// ─── --check diff helper ────────────────────────────────────────────────────── - -/** - * Normalize line endings to LF for CRLF-agnostic comparison. - * FIX 4: The serializer has no nondeterministic content (no timestamp), so - * the generated-by-line stripping that was here has been removed — full content - * comparison is now used so header drift is caught by --check. - * - * @param {string} content - * @returns {string} - */ -function normalizeLineEndings(content) { - return content.replace(/\r/g, ''); -} - // ─── Main ───────────────────────────────────────────────────────────────────── function main() { @@ -409,7 +395,7 @@ function main() { const committed = fs.readFileSync(CONTRACT_PATH, 'utf8'); // FIX 4: Compare full content (no generated-by stripping) so header drift is caught. - if (normalizeLineEndings(committed) !== normalizeLineEndings(live)) { + if (normalizeEol(committed) !== normalizeEol(live)) { process.stderr.write( 'gsd-core/bin/lib/loop-host-contract.cjs is stale. Run:\n' + ' node scripts/gen-loop-host-contract.cjs --write\n', @@ -503,7 +489,7 @@ module.exports = { assertPointsCoverage, buildContract, serializeContract, - normalizeLineEndings, + normalizeLineEndings: normalizeEol, STEP_WORKFLOWS, HOST_LOOP_FILES, CANONICAL_POINTS, diff --git a/scripts/gen-registry.cjs b/scripts/gen-registry.cjs index 93c10202e..fb33ee74c 100644 --- a/scripts/gen-registry.cjs +++ b/scripts/gen-registry.cjs @@ -35,6 +35,7 @@ const path = require('node:path'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); const { renderMarkdown } = require('./registry-schema.cjs'); +const { normalizeEol } = require('../gsd-core/bin/lib/text-lines.cjs'); const SOURCES = [ { type: 'capability', jsonFile: 'capabilities.json', mdFile: 'capability-registry.md' }, @@ -42,18 +43,6 @@ const SOURCES = [ { type: 'reviewer', jsonFile: 'reviewers.json', mdFile: 'reviewer-registry.md', optional: true }, ]; -/** - * The generator always writes LF; a Windows checkout (autocrlf) may present - * committed files with CRLF. Normalize before comparing so `--check` only - * fails on real content drift, not checkout-introduced line-ending noise. - * - * @param {string} content - * @returns {string} - */ -function normalizeLineEndings(content) { - return content.replace(/\r/g, ''); -} - function getRegistriesDir() { return path.join(process.cwd(), 'docs', 'registries'); } @@ -126,7 +115,7 @@ function main() { continue; } const committed = fs.readFileSync(mdPath, 'utf8'); - if (normalizeLineEndings(committed) !== normalizeLineEndings(rendered)) { + if (normalizeEol(committed) !== normalizeEol(rendered)) { process.stderr.write(`${mdFile} is stale. Run:\n node scripts/gen-registry.cjs --write\n`); anyDrift = true; } @@ -149,4 +138,4 @@ function main() { if (require.main === module) runMain(main); -module.exports = { main, renderFor, SOURCES, normalizeLineEndings }; +module.exports = { main, renderFor, SOURCES, normalizeLineEndings: normalizeEol }; diff --git a/src/audit.cts b/src/audit.cts index 8d51fb092..c336240ed 100644 --- a/src/audit.cts +++ b/src/audit.cts @@ -15,6 +15,7 @@ import fs from 'node:fs'; import path from 'node:path'; 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 planningWorkspace = require('./planning-workspace.cjs'); const { planningDir } = planningWorkspace; @@ -411,8 +412,8 @@ function scanTodos(planDir: string): TodoItem[] { const fm = extractFrontmatter(content, safeFilePath); // Extract first line of body after frontmatter - const bodyMatch = content.replace(/^---[\s\S]*?---\n?/, ''); - const firstLine = bodyMatch.trim().split('\n')[0] || ''; + const bodyMatch = content.replace(/^---[\s\S]*?---\r?\n?/, ''); + const firstLine = splitLines(bodyMatch.trim())[0] || ''; const summary = sanitizeForDisplay(firstLine.slice(0, 100)); results.push({ diff --git a/src/broken-windows.cts b/src/broken-windows.cts index eed91a659..67fcae63c 100644 --- a/src/broken-windows.cts +++ b/src/broken-windows.cts @@ -743,7 +743,7 @@ function writeLedgerAtomic(cwd: string, ledger: Ledger): void { if (fenceEnd !== -1) { const afterFence = existing.slice(fenceEnd + JSON_FENCE_CLOSE.length); // Drop leading newlines; keep the rest as prose. - trailingProse = afterFence.replace(/^\n+/, ''); + trailingProse = afterFence.replace(/^(?:\r?\n)+/, ''); } } } catch { diff --git a/src/core-utils.cts b/src/core-utils.cts index 105cf9ea6..f5ee239b8 100644 --- a/src/core-utils.cts +++ b/src/core-utils.cts @@ -69,7 +69,7 @@ function detectSubRepos(cwd: string): string[] { function extractOneLinerFromBody(content: string | null | undefined): string | null { if (!content) return null; const normalized = content.replace(/\r\n/g, '\n').replace(/\r/g, '\n'); - const body = normalized.replace(/^---\n[\s\S]*?\n---\n*/, ''); + const body = normalized.replace(/^---\r?\n[\s\S]*?\r?\n---\r?\n*/, ''); // #3170: anchor to a summary-shaped heading (Summary / Overview / // Accomplishments) so an incidental first heading (a rule list, task // breakdown, deviation note) does not contribute its first bold run as the diff --git a/src/frontmatter.cts b/src/frontmatter.cts index ee77e4e39..3e3fd2cc8 100644 --- a/src/frontmatter.cts +++ b/src/frontmatter.cts @@ -13,6 +13,7 @@ import ioMod = require('./io.cjs'); const { output, error } = ioMod; import { platformReadSync as safeReadFile, platformWriteSync } from './shell-command-projection.cjs'; import { textEncodingError } from './validate.cjs'; +import { splitLines } from './text-lines.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import unusableInputMod = require('./unusable-input.cjs'); const { UNUSABLE_REASON, warnUnusableInput } = unusableInputMod; @@ -84,7 +85,7 @@ const UNTERMINATED_KEY_THRESHOLD = 2; * has a false-positive class the other closes. */ function isFrontmatterShaped(region: string): boolean { - const lines = region.split(/\r?\n/).filter((line) => line.trim() !== ''); + const lines = splitLines(region).filter((line) => line.trim() !== ''); if (lines.length === 0) return false; return lines.every((line) => ( /^\s*[a-zA-Z0-9_-]+:/.test(line) // key: value @@ -116,7 +117,7 @@ type FullLineCommentChannel = { leading: Record; trailing: str */ function parseYamlRegion(yaml: string): Frontmatter { const frontmatter: Frontmatter = {}; - const lines = yaml.split(/\r?\n/); + const lines = splitLines(yaml); // #3257: pending column-0 full-line comments, attached to the next top-level key. let pendingComments: string[] = []; @@ -425,7 +426,7 @@ function propagateCommentChannel(source: Frontmatter, target: Frontmatter): void * not modify (e.g. must_haves.artifacts / .prohibitions). */ function sliceTopLevelFrontmatterSegments(yaml: string): Array<{ key: string; raw: string }> { - const lines = yaml.split(/\r?\n/); + const lines = splitLines(yaml); const segments: Array<{ key: string; raw: string }> = []; let current: { key: string; raw: string[] } | null = null; for (const line of lines) { @@ -491,7 +492,7 @@ function spliceFrontmatter(content: string, newObj: Frontmatter): string { // unrelated `must_haves` block. Keys absent from the original (genuinely new) are // regenerated and appended; keys absent from `newObj` are preserved (never silently // deleted by a set/merge). - const fmLines = fmBlock.split(/\r?\n/); + const fmLines = splitLines(fmBlock); const inner = fmLines.slice(1, -1).join('\n'); // drop the opening `---` and closing `---` let originalParsed: Frontmatter; try { originalParsed = extractFrontmatter(fmBlock); } catch { originalParsed = {}; } @@ -571,28 +572,28 @@ function parseMustHavesBlock(content: string, blockName: string): unknown[] { if (!fmMatch) return []; const yaml = fmMatch[1]; + const yamlLines = splitLines(yaml); - // Find must_haves: first to detect its indentation level - const mustHavesMatch = yaml.match(/^(\s*)must_haves:\s*$/m); - if (!mustHavesMatch) return []; - const mustHavesIndent = mustHavesMatch[1].length; + // Find must_haves: first to detect its indentation level. Split-then-scan + // (rather than a whole-string /m match) so a CRLF or blank-line boundary + // can never be absorbed into the indent capture (#3360) — see + // .gsd/phase/chore-3413-text-lines-seam/40-design.md. + const mustHavesLinePattern = /^(\s*)must_haves:\s*$/; + const mustHavesLineIndex = yamlLines.findIndex((line) => mustHavesLinePattern.test(line)); + if (mustHavesLineIndex === -1) return []; + const mustHavesIndent = (yamlLines[mustHavesLineIndex].match(/^(\s*)/) as RegExpMatchArray)[1].length; // Find the block (e.g., "truths:", "artifacts:", "key_links:") under must_haves // It must be indented more than must_haves but we detect the actual indent dynamically - const blockPattern = new RegExp(`^(\\s+)${blockName}:\\s*$`, 'm'); - const blockMatch = yaml.match(blockPattern); - if (!blockMatch) return []; + const blockLinePattern = new RegExp(`^(\\s+)${blockName}:\\s*$`); + const blockLineIndex = yamlLines.findIndex((line) => blockLinePattern.test(line)); + if (blockLineIndex === -1) return []; - const blockIndent = blockMatch[1].length; + const blockIndent = (yamlLines[blockLineIndex].match(/^(\s*)/) as RegExpMatchArray)[1].length; // The block must be nested under must_haves (more indented) if (blockIndent <= mustHavesIndent) return []; - // Find where the block starts in the yaml string - const blockStart = yaml.indexOf(blockMatch[0]); - if (blockStart === -1) return []; - - const afterBlock = yaml.slice(blockStart); - const blockLines = afterBlock.split(/\r?\n/).slice(1); // skip the header line + const blockLines = yamlLines.slice(blockLineIndex + 1); // skip the header line // List items are indented one level deeper than blockIndent // Continuation KVs are indented one level deeper than list items diff --git a/src/init.cts b/src/init.cts index d64f62183..2112cb629 100644 --- a/src/init.cts +++ b/src/init.cts @@ -3683,7 +3683,7 @@ function buildSkillManifest(cwd: string, skillsDir: string | null = null): Skill const description = (frontmatter['description'] as string) || ''; const triggers: string[] = []; - const bodyMatch = content.match(/^---[\s\S]*?---\s*\n([\s\S]*)$/); + const bodyMatch = content.match(/^---[\s\S]*?---\s*\r?\n([\s\S]*)$/); if (bodyMatch) { const body = bodyMatch[1]; const triggerLines = body.match(/^TRIGGER\s+when:\s*(.+)$/gmi); diff --git a/src/milestone.cts b/src/milestone.cts index bd02444ba..d998dc30d 100644 --- a/src/milestone.cts +++ b/src/milestone.cts @@ -824,7 +824,7 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo platformWriteSync(milestonesPath, `# Milestones\n\n${milestoneEntry}`); } else { // Insert after the header line(s) for reverse chronological order (newest first) - const headerMatch = existing.match(/^(#{1,3}\s+[^\n]*\n\n?)/); + const headerMatch = existing.match(/^(#{1,3}\s+[^\r\n]*\r?\n(?:\r?\n)?)/); if (headerMatch) { const header = headerMatch[1]; const rest = existing.slice(header.length); diff --git a/src/phase-estimation.cts b/src/phase-estimation.cts index fefb90307..fcf798440 100644 --- a/src/phase-estimation.cts +++ b/src/phase-estimation.cts @@ -341,7 +341,7 @@ export function extractFrontmatterBlock(text: unknown, key: string): Record \n for any .md target — so whatever terminator is + // used here in memory is erased before the file is ever written, and + // templating it via detectEol(rawContent) was inert dead code. '\n' + // matches what platformWriteSync enforces anyway. const bulletEntry = `\n- [ ] ${phaseLabel}`; + // #3413: was `[^\n]*`, which on CRLF content swallows the line's + // trailing \r into the match, shifting bulletLineEnd to land BETWEEN + // the \r and \n of the original CRLF pair — a pure splice-POSITION + // bug on the not-yet-write-normalized CRLF read (independent of the + // final on-disk EOL, which platformWriteSync always forces to LF for + // .md targets regardless). Widening to [^\r\n]* stops the match at the + // true line-content boundary so bulletLineEnd lands cleanly before the + // terminator. const targetBulletPattern = new RegExp( - `(-\\s*\\[[ x]\\]\\s*(?:\\*\\*)?Phase\\s+${afterPhaseEscaped}${OPTIONAL_PHASE_TAG_SOURCE}[:\\s][^\\n]*)`, + `(-\\s*\\[[ x]\\]\\s*(?:\\*\\*)?Phase\\s+${afterPhaseEscaped}${OPTIONAL_PHASE_TAG_SOURCE}[:\\s][^\\r\\n]*)`, 'i', ); const bulletMatchResult = rawContent.match(targetBulletPattern); @@ -1331,7 +1346,7 @@ function cmdPhaseInsert(cwd: string, afterPhase: string, description: string, ra const bulletLineEnd = rawContent.indexOf(bulletMatchResult![0]) + bulletMatchResult![0].length; const afterBullet = rawContent.slice(bulletLineEnd); - const nextBulletMatch = afterBullet.match(/\n-\s*\[[ x]\]\s*(?:\*\*)?Phase\s+\d/i); + const nextBulletMatch = afterBullet.match(/\r?\n-\s*\[[ x]\]\s*(?:\*\*)?Phase\s+\d/i); let insertIdx: number; if (nextBulletMatch) { @@ -1357,7 +1372,7 @@ function cmdPhaseInsert(cwd: string, afterPhase: string, description: string, ra const headerIdx = rawContent.indexOf(headerMatch![0]); const afterHeader = rawContent.slice(headerIdx + headerMatch![0].length); - const nextPhaseMatch = afterHeader.match(/\n#{2,4}\s+Phase\s+\d[\d.]*/i); + const nextPhaseMatch = afterHeader.match(/\r?\n#{2,4}\s+Phase\s+\d[\d.]*/i); let insertIdx: number; if (nextPhaseMatch) { @@ -1630,7 +1645,7 @@ function updateRoadmapAfterPhaseRemoval( // #1729: fold an optional pre-colon ( ) tag into the suffix capture so it // is re-emitted verbatim — a tagged later phase still gets renumbered. content = content.replace( - /(#{2,4}\s*Phase\s+)(\d+(?:\.\d+)?)((?:\s*\([^)\n]{0,200}\))?\s*:)/gi, + /(#{2,4}\s*Phase\s+)(\d+(?:\.\d+)?)((?:\s*\([^)\r\n]{0,200}\))?\s*:)/gi, (_match, prefix: string, num: string, suffix: string) => `${prefix}${decrementRoadmapPhaseToken(num, removedInt)}${suffix}`, ); diff --git a/src/profile-output.cts b/src/profile-output.cts index 1a26c1e23..26100e4f1 100644 --- a/src/profile-output.cts +++ b/src/profile-output.cts @@ -553,7 +553,7 @@ function generateSkillsSection(cwd: string): SectionResult { */ function extractSkillFrontmatter(content: string): { name: string; description: string } { const result = { name: '', description: '' }; - const fmMatch = content.match(/^---\s*\n([\s\S]*?)\n---/); + const fmMatch = content.match(/^---\s*\r?\n([\s\S]*?)\r?\n---/); if (!fmMatch) return result; const fmBlock = fmMatch[1]; diff --git a/src/roadmap-upgrade.cts b/src/roadmap-upgrade.cts index 589653af4..9be5e9aba 100644 --- a/src/roadmap-upgrade.cts +++ b/src/roadmap-upgrade.cts @@ -465,6 +465,42 @@ function computeMigrationPlan(cwd: string, options: Record = {} }; } +/** + * Apply roadmap line edits via character-offset splicing against the + * ORIGINAL content string — never a full split/rejoin (#3413). `lineIndex` + * boundaries are found by scanning for the next bare `\n`, exactly matching + * how computeMigrationPlan() itself indexes lines (`roadmapContent.split('\n')`) + * — both sides must agree on line indexing for `lineText === edit.from` to + * match, and this keeps a `\r` that precedes a `\n` as part of the LINE text + * rather than a separately-normalized terminator. Only a line whose text + * exactly equals an edit's `from` is replaced; every other character — + * including every line's own terminator, touched or not — is copied + * byte-for-byte from the original, so a mixed-EOL ROADMAP.md never has its + * untouched lines silently flattened to one dominant style. + */ +function applyRoadmapEdits(content: string, edits: RoadmapEdit[]): string { + const editByLine = new Map(); + for (const edit of edits) editByLine.set(edit.lineIndex, edit); + + let result = ''; + let pos = 0; + let lineIndex = 0; + + for (;;) { + const nlIdx = content.indexOf('\n', pos); + const lineEnd = nlIdx === -1 ? content.length : nlIdx; + const lineText = content.slice(pos, lineEnd); + const edit = editByLine.get(lineIndex); + result += edit && lineText === edit.from ? edit.to : lineText; + if (nlIdx === -1) break; + result += '\n'; + pos = nlIdx + 1; + lineIndex++; + } + + return result; +} + // ─── applyMigration ─────────────────────────────────────────────────────────── /** @@ -538,18 +574,10 @@ function applyMigration(cwd: string, plan: MigrationPlan, options: { dryRun?: bo // 2. Rewrite ROADMAP.md phase headings if (plan.roadmapEdits.length > 0) { const roadmapContent = fs.readFileSync(roadmapPath, 'utf8'); - const lines = roadmapContent.split('\n'); - - // Sort edits by lineIndex to apply in order - const sortedEdits = [...plan.roadmapEdits].sort((a, b) => a.lineIndex - b.lineIndex); - for (const edit of sortedEdits) { - if (lines[edit.lineIndex] === edit.from) { - lines[edit.lineIndex] = edit.to; - } - } + const newRoadmapContent = applyRoadmapEdits(roadmapContent, plan.roadmapEdits); snapshotFile(roadmapPath); - fs.writeFileSync(roadmapPath, lines.join('\n'), 'utf8'); + fs.writeFileSync(roadmapPath, newRoadmapContent, 'utf8'); editedFiles.push('ROADMAP.md'); } diff --git a/src/roadmap.cts b/src/roadmap.cts index 65d1291f6..ac8068d8f 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -10,6 +10,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { realClock } from './clock.cjs'; import { escapeRegex } from './pattern.cjs'; +import { splitLines, detectEol, joinLines } from './text-lines.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import ioMod = require('./io.cjs'); const { output, error } = ioMod; @@ -973,6 +974,11 @@ function cmdRoadmapAnnotateDependencies(cwd: string, phaseNum: string | null | u let updated = false; withPlanningLock(cwd, () => { const content = fs.readFileSync(roadmapPath, 'utf-8'); + // #3413: preserve the file's own EOL style when the checklist block below + // is rebuilt and spliced back in — splitLines() cleans each captured line + // of any dangling \r, so rejoining with a bare '\n' would silently + // downgrade a CRLF ROADMAP.md's rewritten block to LF only. + const eol = detectEol(content); // Find the phase section. // #3537: padding-tolerant fragment so the caller's resolved padded id @@ -1006,12 +1012,12 @@ function cmdRoadmapAnnotateDependencies(cwd: string, phaseNum: string | null | u // Review fix (F2): `(?:^|\n)` anchors the match to start-of-line so mid-line // occurrences like `***Plans:***` embedded in a sentence or `OpenPlans: foo` // do not trigger a false match. Groups 1 and 2 retain the same semantics. - const plansBlockMatch = phaseSection.match(/(?:^|\n)(\*{0,2}Plans\*{0,2}:[^\n]*\n)((?:\s*-\s*\[[ x]\][^\n]*\n?)+)/i); + const plansBlockMatch = phaseSection.match(/(?:^|\r?\n)(\*{0,2}Plans\*{0,2}:[^\r\n]*\r?\n)((?:\s*-\s*\[[ x]\][^\r\n]*\r?\n?)+)/i); if (!plansBlockMatch) return; const plansHeader = plansBlockMatch[1]; const existingList = plansBlockMatch[2]; - const listLines = existingList.split('\n').filter(l => /^\s*-\s*\[/.test(l)); + const listLines = splitLines(existingList).filter(l => /^\s*-\s*\[/.test(l)); if (listLines.length === 0) return; @@ -1065,13 +1071,24 @@ function cmdRoadmapAnnotateDependencies(cwd: string, phaseNum: string | null | u } } - const newListBlock = annotatedLines.join('\n') + '\n'; - // #1103: when `(?:^|\n)` consumed a leading `\n` (mid-string match), re-emit it - // so the line preceding the Plans: header is not fused onto it. - const leadingNewline = plansBlockMatch[0].startsWith('\n') ? '\n' : ''; + const newListBlock = joinLines(annotatedLines, eol) + eol; + // #1103: when `(?:^|\r?\n)` consumed a leading terminator (mid-string + // match), re-emit it verbatim so the line preceding the Plans: header is + // not fused onto it. #3413: the widened `(?:^|\r?\n)` can now consume a + // 2-char `\r\n` — re-emit whatever was actually captured (`''`, `'\n'`, + // or `'\r\n'`), not a hardcoded `'\n'`, or a CRLF file loses its `\r`. + const leadingMatch = /^\r?\n/.exec(plansBlockMatch[0]); + const leadingNewline = leadingMatch ? leadingMatch[0] : ''; + // Review fix (#3413 security): use the FUNCTION-replacement form. The + // string-replacement form expands String#replace's special patterns + // (`$&`, `` $` ``, `$'`, `$$`, `$1`-`$9`) inside the replacement — and + // newListBlock is built from author-controlled truths/plan-file content, + // so a line containing a literal `` $` `` (etc.) would splice unrelated + // surrounding phaseSection text into the result. A function replacer is + // never pattern-interpreted. const newPhaseSection = phaseSection.replace( plansBlockMatch[0], - leadingNewline + plansHeader + newListBlock + () => leadingNewline + plansHeader + newListBlock ); const nextContent = content.slice(0, phaseStart) + newPhaseSection + content.slice(phaseEnd); diff --git a/src/text-lines.cts b/src/text-lines.cts new file mode 100644 index 000000000..4e53f4423 --- /dev/null +++ b/src/text-lines.cts @@ -0,0 +1,77 @@ +/** + * text-lines.cts — the line-terminator seam (ADR-3212 §3, epic #3212 Phase 2, + * #3413). + * + * Source in src/text-lines.cts, compiled to gsd-core/bin/lib/text-lines.cjs + * (gitignored), per the repo's ADR-457 build-at-publish convention. + * + * Sole owner of `\r?\n` splitting and CRLF normalization. Closes #3360: a + * regex anchored on `^`/`$` under `/m` against a whole multi-line string is + * exposed to `\r` being its own LineTerminator (ECMA-262), so a greedy `\s*` + * can cross a CRLF boundary and inflate a captured indent by one character. + * Split-then-match — call `splitLines` first, then match per already-split + * line — is immune by construction, because a single split line can never + * contain the delimiter that produced it. This module exists so that shape + * is the easy, obvious way to write indentation-sensitive parsing, per the + * design doc (.gsd/phase/chore-3413-text-lines-seam/40-design.md). + * + * A "genuine leaf" module (CONTEXT.md's term, shared with Phase 1's sibling + * seam `src/pattern.cts`): zero I/O, zero imports from other `src/*.cts` + * modules, pure string functions only. + */ + +/** + * Split document content into lines on `\r\n` or `\n`. A lone `\r` (bare CR, + * no following `\n`) is NOT a delimiter — this matches every existing + * `\r?\n` call site in the repo and is intentionally narrower than "treat + * any LineTerminator as a break." Matches `String.prototype.split`'s own + * contract for a trailing terminator (a trailing empty element) and for the + * empty string (`[""]`). + */ +export function splitLines(content: string): string[] { + if (typeof content !== 'string') { + throw new TypeError(`splitLines: expected a string, got ${typeof content}`); + } + return content.split(/\r?\n/); +} + +/** + * Normalize all line endings to LF by stripping every `\r` character — not + * only `\r\n` pairs. This is byte-for-byte the same operation as the 4 + * `scripts/gen-*.cjs` copies of `normalizeLineEndings` this module + * consolidates (`content.replace(/\r/g, '')`), so a lone/unpaired `\r` (old + * Mac-style content) is stripped too. Deliberately a different contract from + * `splitLines`, which does NOT treat a bare `\r` as a delimiter (row 6) — + * these are two different operations kept as two separate functions rather + * than one "handle all CR-ish things" primitive. + */ +export function normalizeEol(content: string): string { + return content.replace(/\r/g, ''); +} + +/** + * Detect the dominant line-terminator in `content`: `'\r\n'` when CRLF pairs + * outnumber bare LFs, `'\n'` when bare LFs outnumber CRLF pairs, and `'\r\n'` + * as the default when there is no bare LF at all (all-CRLF content, or no + * terminator present whatsoever — an empty or single-line input). See the + * design doc's Rejected section for why the tie/empty default is `'\r\n'` + * rather than `'\n'`: this function exists to PRESERVE whatever a file + * already uses, not to bias toward LF the way a write-time normalizer would. + */ +export function detectEol(content: string): '\n' | '\r\n' { + const crlfCount = (content.match(/\r\n/g) || []).length; + const totalLfCount = (content.match(/\n/g) || []).length; + const bareLfCount = totalLfCount - crlfCount; + if (bareLfCount === 0 || crlfCount >= bareLfCount) return '\r\n'; + return '\n'; +} + +/** + * Join lines back into a single string with `eol` (default `'\n'`) between + * each pair — the inverse of `splitLines`. `joinLines(splitLines(x), + * detectEol(x))` reproduces `x` byte-for-byte for any `x` already in + * canonical (single-terminator) form (ADR-3212 §3's round-trip property). + */ +export function joinLines(lines: string[], eol?: '\n' | '\r\n'): string { + return lines.join(eol ?? '\n'); +} diff --git a/tests/frontmatter.test.cjs b/tests/frontmatter.test.cjs index 1e460729b..11afffe7d 100644 --- a/tests/frontmatter.test.cjs +++ b/tests/frontmatter.test.cjs @@ -390,6 +390,12 @@ describe('spliceFrontmatter', () => { // ─── parseMustHavesBlock ──────────────────────────────────────────────────── +// #3413 / #3360: LF -> CRLF fixture converter. Naming precedent: +// tests/codex-agent-toml.test.cjs's toCrlf, tests/agent-install-check.test.cjs's toCrlf. +function crlf(s) { + return s.replace(/\n/g, '\r\n'); +} + describe('parseMustHavesBlock', () => { test('extracts truths as string array', () => { const content = `--- @@ -644,6 +650,122 @@ must_haves: // The nested array should be captured assert.ok(result[0].exports !== undefined, 'should have exports field'); }); + + test('#3360: parseMustHavesBlock returns real items for a CRLF plan (no leading blank line) — rows 24-25', () => { + // Exact #3360 repro (design doc 40-design.md, "Rubber-duck" section): + // \s inside an anchored /m pattern is not "whitespace on this line" — it + // is "whitespace, including the boundary I just anchored on." A CRLF + // pair inflates the captured indent by one char, tripping the + // blockIndent <= mustHavesIndent nesting guard on a block that IS + // legitimately nested. Today (pre-fix) this returns [] for both blocks. + const lfPlan = `--- +phase: 01 +must_haves: + truths: + - "first truth" + - "second truth" + prohibitions: + - "MUST NOT drop the table" +--- + +Body content.`; + const crlfPlan = crlf(lfPlan); + const truths = parseMustHavesBlock(crlfPlan, 'truths'); + const prohibitions = parseMustHavesBlock(crlfPlan, 'prohibitions'); + assert.ok(Array.isArray(truths), 'truths should return an array'); + assert.deepStrictEqual(truths, ['first truth', 'second truth']); + assert.ok(Array.isArray(prohibitions), 'prohibitions should return an array'); + assert.deepStrictEqual(prohibitions, ['MUST NOT drop the table']); + }); + + test('#3360: the silent-exit variant (blank line before must_haves:) also recovers — row 26', () => { + // #3360 "second variant": a blank line preceding `must_haves:` lets the + // (\s*) capture before it absorb the blank line's own terminator too — + // same indent-inflation mechanism, but with zero diagnostic emitted + // today (the silent exit the design doc calls out). + const lfPlanWithBlankLine = `--- +phase: 01 + +must_haves: + truths: + - "first truth" + - "second truth" + prohibitions: + - "MUST NOT drop the table" +--- + +Body content.`; + const crlfPlanWithBlankLine = crlf(lfPlanWithBlankLine); + const truths = parseMustHavesBlock(crlfPlanWithBlankLine, 'truths'); + assert.ok(Array.isArray(truths), 'should return an array'); + assert.deepStrictEqual(truths, ['first truth', 'second truth']); + }); + + test('#3360 parity: CRLF and LF plans parse to identical must_haves for every block name (maintainer-established invariant — see #3360, Cortex-recorded prior art for repeated CRLF-fix parity assertions in this file\'s neighborhood) — row 28', () => { + // Reuses the ACTUAL fixture shapes already present in this describe + // block (not a fourth parallel fixture set) so this generalizes real + // existing coverage rather than adding new, narrower cases. + const fourSpaceIndentTruths = `--- +phase: 01 +must_haves: + truths: + - "All tests pass on CI" + - "Coverage exceeds 80%" +--- + +Body content.`; + const twoSpaceIndentTruths = `--- +phase: 01 +must_haves: + truths: + - "All tests pass on CI" + - "Coverage exceeds 80%" +--- +`; + const quotedColonTruths = `--- +phase: 01 +must_haves: + truths: + - "App-side UUIDv4: generated locally" + - "No colon in this one" + - "Another colon: example" +--- +`; + + for (const fixture of [fourSpaceIndentTruths, twoSpaceIndentTruths, quotedColonTruths]) { + assert.deepStrictEqual( + parseMustHavesBlock(crlf(fixture), 'truths'), + parseMustHavesBlock(fixture, 'truths'), + `CRLF/LF parity diverged for fixture:\n${fixture}` + ); + } + }); + + test('#3360 regression guard: the nesting guard still rejects a non-nested block on CRLF input — row 29', () => { + // `truths:` sits at the SAME indent (column 0) as `must_haves:` — a + // sibling, not a nested child — so the blockIndent <= mustHavesIndent + // guard must still reject it, even on CRLF input, post-fix. + const lfPlan = `--- +phase: 01 +must_haves: +truths: + - "should not be picked up" +--- +`; + const result = parseMustHavesBlock(crlf(lfPlan), 'truths'); + assert.deepStrictEqual(result, []); + }); + + test('parseMustHavesBlock: CRLF content with no must_haves block still returns [] — row 30', () => { + const lfPlan = `--- +phase: 01 +truths: + - "Some truth" +--- +`; + const result = parseMustHavesBlock(crlf(lfPlan), 'truths'); + assert.deepStrictEqual(result, []); + }); }); // ─── stripFrontmatter ─────────────────────────────────────────────────────── diff --git a/tests/no-crlf-fragile-split.rule.test.cjs b/tests/no-crlf-fragile-split.rule.test.cjs index bfc73c712..4cf54a1d6 100644 --- a/tests/no-crlf-fragile-split.rule.test.cjs +++ b/tests/no-crlf-fragile-split.rule.test.cjs @@ -336,3 +336,61 @@ describe('G2/G3 — no-crlf-fragile-split: valid cases', () => { }); }); }); + +// ─── #3413 (#3212 Phase 2) — widened glob + message text ────────────────── +// +// RuleTester invokes the rule function directly against a code string; it +// does NOT read eslint.config.mjs, so it never "sees" a glob at all. A +// RuleTester case for the G1 trigger shape is already something the rule's +// CURRENT logic detects (G1 doesn't look at file extension), regardless of +// which files eslint.config.mjs applies it to. What Phase 2 actually changes +// is the MESSAGE TEXT (fix-hint should name `splitLines` instead of +// `.split(/\r?\n/)`) — the glob widening itself (tests/ -> tests/ + src/) is +// a separate, non-RuleTester-visible eslint.config.mjs change. + +describe('no-crlf-fragile-split: #3413 message-text update (row 32, RED)', () => { + test('row 32: G1 trigger in a src/*.cts-shaped file expects the FUTURE splitLines-hint message text', () => { + // Verifies the exact rendered message text for a G1 trigger in a + // src/*.cts-shaped file: the crlfFragileSplit message names + // `splitLines()` from `src/text-lines.cts` as the fix. + ruleTester.run('no-crlf-fragile-split', rule, { + valid: [], + invalid: [ + { + // Same G1 trigger shape as the existing "direct chain" invalid + // case above, just in a src/*.cts-shaped filename. + code: "const lines = fs.readFileSync(p, 'utf8').split('\\n');", + filename: 'src/some-module.cts', + errors: [ + { + message: + 'Splitting on literal "\\n" on readFileSync content is CRLF-fragile ' + + '(DEFECT.WINDOWS-CRLF-TEST-PORTABILITY): Windows git-autocrlf yields "\\r\\n" ' + + 'line endings. Use splitLines() from src/text-lines.cts instead.', + }, + ], + }, + ], + }); + }); +}); + +describe('no-crlf-fragile-split: self-reference regression guard (row 33)', () => { + test('row 33: does not flag its own already-correct \\r?\\n split (self-reference check)', () => { + // Locks in behavior `src/text-lines.cts`'s own `splitLines` implementation + // relies on: `.split(/\\r?\\n/)` (the already-correct pattern) must never + // trip G1, even against a src/*.cts-shaped filename. This ALREADY PASSES + // today — `.split(/\\r?\\n/)` was never a G1 trigger (only a bare `'\n'` + // string argument is) — so this is a regression guard verifying the + // design doc's row 25 self-reference claim empirically, not a RED case. + ruleTester.run('no-crlf-fragile-split', rule, { + valid: [ + { + code: "const lines = fs.readFileSync(p, 'utf8').split(/\\r?\\n/);", + filename: 'src/text-lines.cts', + }, + ], + invalid: [], + }); + }); +}); diff --git a/tests/text-lines.test.cjs b/tests/text-lines.test.cjs new file mode 100644 index 000000000..0d6c71b3a --- /dev/null +++ b/tests/text-lines.test.cjs @@ -0,0 +1,168 @@ +'use strict'; + +/** + * Tests for `src/text-lines.cts` — the line-terminator seam (#3212 Phase 2, #3413). + * + * Design: .gsd/phase/chore-3413-text-lines-seam/40-design.md + * Test matrix: .gsd/phase/chore-3413-text-lines-seam/50-test-matrix.md + * ADR: docs/adr/3212-lexical-seam-consolidation.md §3, §6, §7 + * + * TDD RED: `src/text-lines.cts` does not exist yet — this file's + * `require('../gsd-core/bin/lib/text-lines.cjs')` throws MODULE_NOT_FOUND until + * the implementing phase adds it. That is the intended starting state (mirrors + * tests/pattern.test.cjs's RED convention from Phase 1, #3412). + * + * Covers test-matrix rows 1-23: splitLines (1-7), normalizeEol (8-10), + * detectEol (11-16), joinLines (17-20), round-trip (21-23). + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +// Seeded fast-check convention: require the shared setup helper (NOT +// 'fast-check' directly) so numRuns/seed are configured globally before any +// fc.assert() call — mirrors tests/pattern.test.cjs. seed: 42, overridable +// via GSD_FC_SEED. +const fc = require('./helpers/fast-check-setup.cjs'); + +const { splitLines, normalizeEol, detectEol, joinLines } = require('../gsd-core/bin/lib/text-lines.cjs'); + +// ─── splitLines — rows 1-7 ───────────────────────────────────────────────── + +describe('splitLines', () => { + test('row 1: splits LF-only content', () => { + assert.deepStrictEqual(splitLines('a\nb\nc'), ['a', 'b', 'c']); + }); + + test('row 2: splits CRLF content with no trailing \\r per line', () => { + const result = splitLines('a\r\nb\r\nc'); + assert.deepStrictEqual(result, ['a', 'b', 'c']); + for (const line of result) { + assert.ok(!line.includes('\r'), `line ${JSON.stringify(line)} must not carry a trailing \\r`); + } + }); + + test('row 3: handles mixed CRLF/LF in one document', () => { + assert.deepStrictEqual(splitLines('a\r\nb\nc'), ['a', 'b', 'c']); + }); + + test('row 4: empty string matches native String#split(/\\r?\\n/) contract', () => { + assert.deepStrictEqual(splitLines(''), ['']); + assert.deepStrictEqual(splitLines(''), ''.split(/\r?\n/)); + }); + + test('row 5: trailing terminator yields a trailing empty element', () => { + assert.deepStrictEqual(splitLines('a\n'), ['a', '']); + }); + + test('row 6: a lone \\r without a following \\n is not a delimiter', () => { + assert.deepStrictEqual(splitLines('a\rb'), ['a\rb']); + }); + + test('row 7: rejects a non-string input', () => { + assert.throws(() => splitLines(42), TypeError); + assert.throws(() => splitLines(null), TypeError); + }); +}); + +// ─── normalizeEol — rows 8-10 ────────────────────────────────────────────── + +describe('normalizeEol', () => { + test('row 8: converts CRLF and leaves LF alone', () => { + assert.strictEqual(normalizeEol('a\r\nb\nc'), 'a\nb\nc'); + }); + + test('row 9: no-op on already-LF content', () => { + assert.strictEqual(normalizeEol('a\nb'), 'a\nb'); + }); + + test('row 10: strips an unpaired bare \\r, matching the scripts/ copies it replaces', () => { + assert.strictEqual(normalizeEol('a\rb\rc'), 'abc'); + }); +}); + +// ─── detectEol — rows 11-16 ──────────────────────────────────────────────── + +describe('detectEol', () => { + test('row 11: all-CRLF content', () => { + assert.strictEqual(detectEol('a\r\nb\r\nc'), '\r\n'); + }); + + test('row 12: all-LF content', () => { + assert.strictEqual(detectEol('a\nb\nc'), '\n'); + }); + + test('row 13: mixed content, CRLF majority wins', () => { + assert.strictEqual(detectEol('a\r\nb\nc\r\n'), '\r\n'); + }); + + test('row 14: mixed content, LF majority wins', () => { + assert.strictEqual(detectEol('a\nb\nc\r\n'), '\n'); + }); + + test('row 14b: an exact 1:1 tie resolves to \\r\\n per the documented default (#3413 review fix)', () => { + assert.strictEqual(detectEol('a\nb\r\nc'), '\r\n'); + }); + + test('row 15: no terminator present returns the documented default', () => { + assert.strictEqual(detectEol('no terminator at all'), '\r\n'); + }); + + test('row 16: empty string returns the documented default', () => { + assert.strictEqual(detectEol(''), '\r\n'); + }); +}); + +// ─── joinLines — rows 17-20 ──────────────────────────────────────────────── + +describe('joinLines', () => { + test('row 17: defaults to LF', () => { + assert.strictEqual(joinLines(['a', 'b', 'c']), 'a\nb\nc'); + }); + + test('row 18: explicit CRLF', () => { + assert.strictEqual(joinLines(['a', 'b', 'c'], '\r\n'), 'a\r\nb\r\nc'); + }); + + test('row 19: empty array yields empty string', () => { + assert.strictEqual(joinLines([]), ''); + }); + + test('row 20: single-element array has no terminator', () => { + assert.strictEqual(joinLines(['only']), 'only'); + }); +}); + +// ─── round-trip — rows 21-23 ─────────────────────────────────────────────── + +describe('round-trip', () => { + test('row 21: split->detect->join reproduces a CRLF document exactly (#3212 §3)', () => { + const x = '---\r\nphase: 01\r\nmust_haves:\r\n truths:\r\n - "first truth"\r\n---\r\n\r\nBody.\r\n'; + assert.strictEqual(joinLines(splitLines(x), detectEol(x)), x); + }); + + test('row 22: split->detect->join reproduces an LF document exactly', () => { + const x = '---\nphase: 01\nmust_haves:\n truths:\n - "first truth"\n---\n\nBody.\n'; + assert.strictEqual(joinLines(splitLines(x), detectEol(x)), x); + }); + + test('row 23: property — joinLines/splitLines are inverses over terminator-free line arrays (seeded)', () => { + // Generator per 50-test-matrix.md's "Property test note (row 23)": arrays + // of strings with no embedded \r/\n, 1 to 20 elements (0-element arrays + // are the known non-invertible edge already covered explicitly at rows + // 4/19 — joinLines([]) === '' but splitLines('') === [''], not [] — so + // that edge is excluded here rather than re-asserted as a property + // failure), eol drawn from ['\n', '\r\n']. + fc.assert( + fc.property( + fc.array( + fc.string().filter(s => !/[\r\n]/.test(s)), + { minLength: 1, maxLength: 20 } + ), + fc.constantFrom('\n', '\r\n'), + (lines, eol) => { + assert.deepStrictEqual(splitLines(joinLines(lines, eol)), lines); + } + ) + ); + }); +});