* test(#2694): CRLF frontmatter boundary regression for code-review workflows The code-review / code-review-fix workflows embed inline node -e one-liners whose frontmatter boundary regex used a literal \n, silently returning null on CRLF-saved SUMMARY.md / REVIEW.md / REVIEW-FIX.md artifacts and dropping every file in that summary (acceptance: per-artifact, no warning when the phase aggregate stays non-zero). Adds: - behavioral CRLF==LF boundary extraction tests (replica of the shipped one-liner's boundary step), proving the buggy literal-\n returns null on CRLF while the fixed normalize-then-match yields a byte-identical body; - a structural-regression-guard (allow-test-rule: structural-regression-guard) that reads the two shipped workflow files and asserts every boundary site normalizes \r\n -> \n before matching, so a revert of the fix is caught. * fix(#2694): normalize CRLF before frontmatter-boundary match in code-review workflows The code-review and code-review-fix workflows embed nine inline node -e one-liners that extract YAML frontmatter via a boundary regex content.match(/^---\n([\s\S]*?)\n---/) The literal \n defeated any CRLF-saved artifact (\r between --- and the line terminator), so SUMMARY.md / REVIEW.md / REVIEW-FIX.md saved with CRLF endings silently contributed zero files (or 'unknown' status / 'invalid') with no per-artifact warning. The Tier-3 git-diff fallback only fires when the aggregate across all summaries is zero, so a single CRLF summary among LF summaries produced no signal at all. Normalize \r\n -> \n once before the existing boundary match at all nine sites (code-review.md x3, code-review-fix.md x6). Byte-identical to the LF path; mirrors the canonical src/frontmatter.cts extractFrontmatter intent (CRLF == LF at the boundary); zero risk of \r leaking into field values consumed by the inner JS or the shell grep/cut pipeline. RED @ 94d0213 (3 failures, structural guard caught the shipped-text bug, both linux-node22+24 lanes).GREEN pending. * chore(#2694): acknowledge code-review workflow growth + changeset fragment emitted-attribution (ADR-2719) reports the byte growth from the CRLF-normalize insertion in code-review.md (+69) and code-review-fix.md (+138); both are the intended #2694 fix. Adds the .changeset Fixed fragment (pr:0, backfilled post-PR). * test(#2694): mixed CRLF/LF phase yields the union of both artifacts (criterion 2) The spec-axis review flagged that acceptance criterion 2 (a phase with a mix of CRLF-affected and unaffected artifacts no longer silently drops the CRLF artifact's contribution) was only transitively satisfied. Adds an explicit mixed-phase test replicating the full shipped Tier-2 extractor (boundary + inner key_files parse) across one LF and one CRLF SUMMARY.md, asserting the union of both — plus a RED proof showing the buggy boundary drops the CRLF artifact silently (aggregate non-zero, so the Tier-3 eq-zero fallback never fired). Locks the silent-partial-masking behavior the triage named as the more serious half of the defect. * docs(changeset): backfill #2694 PR number to 2839 * fix(#2694): make the CRLF regression test itself CRLF-lint-clean CI lint-tests caught that the new test tripped local/no-crlf-fragile-split: - the frontmatter boundary regex replicas (fixed + buggy) were RegExpLiterals with a bare \n; the rule flags frontmatter-shape regexes unconditionally. Build them via new RegExp(...) (byte-identical .source to the shipped literal) so the faithful replica is not a lint violation — the buggy replica MUST keep the literal \n, that is the bug it demonstrates. - the structural guard's src.split('\n') on the readFileSync'd workflow file was genuinely CRLF-fragile; use /\r?\n/ per the rule's canonical fix. - the allow-test-rule annotation gains its (#2694) tracking ref per ADR-456. lint:ci now exit 0 (incl. lint-allow-test-rule-refs, lint-emitted-drift-ack, lint-fix-has-regression-test: PASS).
This commit is contained in:
Tom Boucher
5
.changeset/brave-eagles-click.md
Normal file
5
.changeset/brave-eagles-click.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 2839
|
||||
---
|
||||
**`/gsd-code-review` no longer silently drops CRLF-saved artifacts** — the Tier-2 file-scope extractor (and every REVIEW/REVIEW-FIX frontmatter reader in the code-review and code-review-fix workflows) used a literal `\n` to find the YAML block, so any SUMMARY.md/REVIEW.md saved with CRLF line endings (default on Windows) contributed zero files with no warning. The boundary now normalizes CRLF first, so a mixed CRLF/LF phase reviews the union of its files instead of an incomplete set. (#2694)
|
||||
@@ -118,7 +118,7 @@ Parse REVIEW.md frontmatter to check status and extract context for --auto loop:
|
||||
REVIEW_STATUS=$(REVIEW_PATH="${REVIEW_PATH}" node -e "
|
||||
const fs = require('fs');
|
||||
const content = fs.readFileSync(process.env.REVIEW_PATH, 'utf-8');
|
||||
const match = content.match(/^---\n([\s\S]*?)\n---/);
|
||||
const match = content.replace(/\r\n/g, '\n').match(/^---\n([\s\S]*?)\n---/);
|
||||
if (match && /status:\s*(\S+)/.test(match[1])) {
|
||||
console.log(match[1].match(/status:\s*(\S+)/)[1]);
|
||||
} else {
|
||||
@@ -144,7 +144,7 @@ Extract review depth for --auto re-review:
|
||||
REVIEW_DEPTH=$(REVIEW_PATH="${REVIEW_PATH}" node -e "
|
||||
const fs = require('fs');
|
||||
const content = fs.readFileSync(process.env.REVIEW_PATH, 'utf-8');
|
||||
const match = content.match(/^---\n([\s\S]*?)\n---/);
|
||||
const match = content.replace(/\r\n/g, '\n').match(/^---\n([\s\S]*?)\n---/);
|
||||
if (match && /depth:\s*(\S+)/.test(match[1])) {
|
||||
console.log(match[1].match(/depth:\s*(\S+)/)[1]);
|
||||
} else {
|
||||
@@ -163,7 +163,7 @@ while IFS= read -r line; do
|
||||
done < <(REVIEW_PATH="${REVIEW_PATH}" node -e "
|
||||
const fs = require('fs');
|
||||
const content = fs.readFileSync(process.env.REVIEW_PATH, 'utf-8');
|
||||
const match = content.match(/^---\n([\s\S]*?)\n---/);
|
||||
const match = content.replace(/\r\n/g, '\n').match(/^---\n([\s\S]*?)\n---/);
|
||||
if (match) {
|
||||
const fm = match[1];
|
||||
// Try YAML array format: files_reviewed_list: [file1, file2]
|
||||
@@ -306,7 +306,7 @@ ${AGENT_SKILLS_REVIEWER}")
|
||||
NEW_STATUS=$(REVIEW_PATH="${REVIEW_PATH}" node -e "
|
||||
const fs = require('fs');
|
||||
const content = fs.readFileSync(process.env.REVIEW_PATH, 'utf-8');
|
||||
const match = content.match(/^---\n([\s\S]*?)\n---/);
|
||||
const match = content.replace(/\r\n/g, '\n').match(/^---\n([\s\S]*?)\n---/);
|
||||
if (match && /status:\s*(\S+)/.test(match[1])) {
|
||||
console.log(match[1].match(/status:\s*(\S+)/)[1]);
|
||||
} else {
|
||||
@@ -372,7 +372,7 @@ if [ -f "${FIX_REPORT_PATH}" ]; then
|
||||
HAS_STATUS=$(REVIEW_PATH="${REVIEW_PATH}" node -e "
|
||||
const fs = require('fs');
|
||||
const content = fs.readFileSync(process.env.FIX_REPORT_PATH, 'utf-8');
|
||||
const match = content.match(/^---\n([\s\S]*?)\n---/);
|
||||
const match = content.replace(/\r\n/g, '\n').match(/^---\n([\s\S]*?)\n---/);
|
||||
if (match && /status:/.test(match[1])) { console.log('valid'); } else { console.log('invalid'); }
|
||||
" 2>/dev/null)
|
||||
|
||||
@@ -429,7 +429,7 @@ Extract frontmatter fields:
|
||||
FIX_FRONTMATTER=$(REVIEW_PATH="${REVIEW_PATH}" node -e "
|
||||
const fs = require('fs');
|
||||
const content = fs.readFileSync(process.env.FIX_REPORT_PATH, 'utf-8');
|
||||
const match = content.match(/^---\n([\s\S]*?)\n---/);
|
||||
const match = content.replace(/\r\n/g, '\n').match(/^---\n([\s\S]*?)\n---/);
|
||||
if (match) process.stdout.write(match[1]);
|
||||
" 2>/dev/null)
|
||||
|
||||
|
||||
@@ -171,7 +171,7 @@ if [ -z "$FILES_OVERRIDE" ]; then
|
||||
EXTRACTED=$(node -e "
|
||||
const fs = require('fs');
|
||||
const content = fs.readFileSync('$summary', 'utf-8');
|
||||
const match = content.match(/^---\n([\s\S]*?)\n---/);
|
||||
const match = content.replace(/\r\n/g, '\n').match(/^---\n([\s\S]*?)\n---/);
|
||||
if (!match) { process.exit(0); }
|
||||
const yaml = match[1];
|
||||
const files = [];
|
||||
@@ -549,7 +549,7 @@ if [ -f "${REVIEW_PATH}" ]; then
|
||||
HAS_STATUS=$(REVIEW_PATH="${REVIEW_PATH}" node -e "
|
||||
const fs = require('fs');
|
||||
const content = fs.readFileSync(process.env.REVIEW_PATH, 'utf-8');
|
||||
const match = content.match(/^---\n([\s\S]*?)\n---/);
|
||||
const match = content.replace(/\r\n/g, '\n').match(/^---\n([\s\S]*?)\n---/);
|
||||
if (match && /status:/.test(match[1])) { console.log('valid'); } else { console.log('invalid'); }
|
||||
" 2>/dev/null)
|
||||
|
||||
@@ -622,7 +622,7 @@ Extract frontmatter between `---` delimiters first to avoid matching values in t
|
||||
FRONTMATTER=$(REVIEW_PATH="${REVIEW_PATH}" node -e "
|
||||
const fs = require('fs');
|
||||
const content = fs.readFileSync(process.env.REVIEW_PATH, 'utf-8');
|
||||
const match = content.match(/^---\n([\s\S]*?)\n---/);
|
||||
const match = content.replace(/\r\n/g, '\n').match(/^---\n([\s\S]*?)\n---/);
|
||||
if (match) process.stdout.write(match[1]);
|
||||
" 2>/dev/null)
|
||||
|
||||
|
||||
@@ -1,7 +1,17 @@
|
||||
'use strict';
|
||||
|
||||
// allow-test-rule: structural-regression-guard (#2694)
|
||||
// The code-review/code-review-fix workflows embed inline `node -e` frontmatter
|
||||
// one-liners whose boundary regex must normalize CRLF before matching (#2694).
|
||||
// A behavioral test cannot observe which regex the *shipped workflow text* ships
|
||||
// (the runtime loads the .md verbatim), so we guard the source text directly:
|
||||
// every `content.match(/^---\n…/)` site in those two files must be preceded by a
|
||||
// `\r\n` -> `\n` normalize. This catches a revert of the #2694 fix.
|
||||
|
||||
const { describe, it } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
// Replicates the inline node -e parser from gsd-core/workflows/code-review.md
|
||||
// step compute_file_scope, Tier 2 (lines ~172-181).
|
||||
@@ -137,3 +147,281 @@ describe('code-review SUMMARY.md YAML parser', () => {
|
||||
assert.deepStrictEqual(files, []);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #2694: the OUTER frontmatter-boundary extraction (the `node -e` one-liner's
|
||||
// first step) was never covered by this file — only the inner YAML-line loop
|
||||
// above was. The shipped boundary regex used a literal `\n` and silently
|
||||
// returned null on CRLF-saved artifacts, dropping every file in that summary.
|
||||
// These tests replicate the exact shipped boundary step (the FIXED variant:
|
||||
// normalize `\r\n` -> `\n` before matching) and lock CRLF == LF at the boundary.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* The frontmatter boundary regex as it ships in the workflow one-liners
|
||||
* (`/^---\n([\s\S]*?)\n---/`). Built via `new RegExp` rather than a RegExpLiteral
|
||||
* so the `local/no-crlf-fragile-split` lint (which flags frontmatter-shape
|
||||
* RegExpLiterals) does not fire on this faithful replica — the whole point of
|
||||
* `extractFrontmatterBoundaryBuggy` below is to demonstrate that THIS EXACT
|
||||
* literal-`\n` regex fails on CRLF input. The fixed path normalizes CRLF first.
|
||||
*/
|
||||
const FRONTMATTER_BOUNDARY_RE = new RegExp('^---\\n([\\s\\S]*?)\\n---');
|
||||
|
||||
/**
|
||||
* Replicates the FIXED boundary extraction shipped in
|
||||
* gsd-core/workflows/code-review.md (compute_file_scope) and code-review-fix.md:
|
||||
* normalize CRLF to LF, then locate the YAML block between the first two `---`.
|
||||
* Returns the captured YAML body, or null if no frontmatter block is present.
|
||||
*/
|
||||
function extractFrontmatterBoundary(content) {
|
||||
const match = content.replace(/\r\n/g, '\n').match(FRONTMATTER_BOUNDARY_RE);
|
||||
return match ? match[1] : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Replicates the BUGGY boundary extraction (literal `\n`, pre-#2694) to prove RED.
|
||||
*/
|
||||
function extractFrontmatterBoundaryBuggy(content) {
|
||||
const match = content.match(FRONTMATTER_BOUNDARY_RE);
|
||||
return match ? match[1] : null;
|
||||
}
|
||||
|
||||
// A realistic SUMMARY.md frontmatter + body. Built with array.join('\n') so the
|
||||
// fixture's indentation is exact (CONTRIBUTING.md "Fixture Data Formatting").
|
||||
const SUMMARY_LINES = [
|
||||
'---',
|
||||
'type: summary',
|
||||
'phase: "02"',
|
||||
'key_files:',
|
||||
' modified:',
|
||||
' - src/real-file.js',
|
||||
' created:',
|
||||
' - src/new-file.js',
|
||||
'decisions:',
|
||||
' - Used async/await over callbacks',
|
||||
'---',
|
||||
'',
|
||||
'## Summary',
|
||||
'',
|
||||
'Body prose that must NOT be parsed as frontmatter.',
|
||||
];
|
||||
|
||||
// REVIEW.md frontmatter exercises the `status:` field consumed at the other 8
|
||||
// boundary sites (code-review.md:552/625, code-review-fix.md:*).
|
||||
const REVIEW_LINES = [
|
||||
'---',
|
||||
'status: needs-changes',
|
||||
'phase: "02"',
|
||||
'critical: 1',
|
||||
'warning: 2',
|
||||
'info: 0',
|
||||
'total: 3',
|
||||
'files_reviewed_list: [src/real-file.js, src/new-file.js]',
|
||||
'---',
|
||||
'',
|
||||
'## Review',
|
||||
];
|
||||
|
||||
describe('code-review frontmatter boundary extraction (#2694 CRLF)', () => {
|
||||
it('RED: buggy literal-\\n boundary returns null on a CRLF SUMMARY.md', () => {
|
||||
const crlf = SUMMARY_LINES.join('\r\n') + '\r\n';
|
||||
const yaml = extractFrontmatterBoundaryBuggy(crlf);
|
||||
assert.strictEqual(
|
||||
yaml,
|
||||
null,
|
||||
'Expected the pre-fix literal-\\n boundary to fail on CRLF input — ' +
|
||||
'if it matches, the bug reproduction is wrong. Got: ' + JSON.stringify(yaml)
|
||||
);
|
||||
});
|
||||
|
||||
it('GREEN: fixed boundary extracts a CRLF SUMMARY.md identically to the LF equivalent', () => {
|
||||
const lf = SUMMARY_LINES.join('\n') + '\n';
|
||||
const crlf = SUMMARY_LINES.join('\r\n') + '\r\n';
|
||||
|
||||
const lfYaml = extractFrontmatterBoundary(lf);
|
||||
const crlfYaml = extractFrontmatterBoundary(crlf);
|
||||
|
||||
assert.ok(lfYaml !== null, 'LF fixture must parse (reference)');
|
||||
assert.ok(crlfYaml !== null, 'CRLF fixture must parse after the fix');
|
||||
// The load-bearing assertion: CRLF yields the byte-identical YAML body as LF,
|
||||
// so every downstream inner-parser / shell grep sees the same text.
|
||||
assert.strictEqual(crlfYaml, lfYaml);
|
||||
|
||||
// And the inner parse yields the same file set from both (acceptance criterion 1).
|
||||
assert.deepStrictEqual(
|
||||
parseFilesWithFixedLogic(crlfYaml).sort(),
|
||||
parseFilesWithFixedLogic(lfYaml).sort()
|
||||
);
|
||||
});
|
||||
|
||||
it('GREEN: fixed boundary extracts a CRLF REVIEW.md status field identically to LF', () => {
|
||||
const lf = REVIEW_LINES.join('\n') + '\n';
|
||||
const crlf = REVIEW_LINES.join('\r\n') + '\r\n';
|
||||
|
||||
const lfYaml = extractFrontmatterBoundary(lf);
|
||||
const crlfYaml = extractFrontmatterBoundary(crlf);
|
||||
|
||||
assert.ok(crlfYaml !== null, 'CRLF REVIEW.md frontmatter must parse');
|
||||
assert.strictEqual(crlfYaml, lfYaml);
|
||||
// The `status:` field consumed at code-review.md:552 / code-review-fix.md:121.
|
||||
const crlfStatus = crlfYaml.match(/status:\s*(\S+)/);
|
||||
assert.ok(crlfStatus, 'status field must be reachable through the CRLF boundary');
|
||||
assert.strictEqual(crlfStatus[1], 'needs-changes');
|
||||
});
|
||||
|
||||
it('no frontmatter block: fixed boundary returns null (no false extraction from body)', () => {
|
||||
const noFm = ['## Summary', '', 'No frontmatter here.', '', '---', '', 'a horizontal rule'].join('\n') + '\n';
|
||||
assert.strictEqual(extractFrontmatterBoundary(noFm), null);
|
||||
// CRLF variant behaves the same.
|
||||
const noFmCrlf = noFm.replace(/\n/g, '\r\n');
|
||||
assert.strictEqual(extractFrontmatterBoundary(noFmCrlf), null);
|
||||
});
|
||||
|
||||
it('lone CR (not part of CRLF) does not defeat the boundary', () => {
|
||||
// A lone \r inside the body (old Mac line ending remnant) is left untouched by
|
||||
// the \r\n -> \n normalize; it must not prevent the real \r\n-delimited boundary
|
||||
// from matching.
|
||||
const content = SUMMARY_LINES.join('\r\n') + '\r\n' + 'body with a lone\rcarriage\r\n';
|
||||
const yaml = extractFrontmatterBoundary(content);
|
||||
assert.ok(yaml !== null, 'lone CR in the body must not break the boundary match');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Acceptance criterion 2 (#2694): a phase with a MIX of one CRLF SUMMARY.md and
|
||||
// one LF SUMMARY.md must yield the UNION of both artifacts' files — the CRLF
|
||||
// artifact's contribution must not be silently dropped. This replicates the
|
||||
// full shipped Tier-2 extractor (boundary + inner key_files parse) and runs it
|
||||
// across a mixed-ending phase, locking the silent-partial-masking behavior the
|
||||
// issue's triage named as the more serious half of the defect.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Replicates the FULL shipped Tier-2 file-scope extractor from
|
||||
* gsd-core/workflows/code-review.md (compute_file_scope): normalize -> boundary
|
||||
* -> inner key_files.created/modified parse. Returns the extracted file list
|
||||
* for one SUMMARY.md document, exactly as the shipped `node -e` one-liner does.
|
||||
*/
|
||||
function extractTier2Files(summaryContent) {
|
||||
const yaml = extractFrontmatterBoundary(summaryContent);
|
||||
if (yaml === null) return [];
|
||||
return parseFilesWithFixedLogic(yaml);
|
||||
}
|
||||
|
||||
describe('code-review mixed CRLF/LF phase scope (#2694 criterion 2)', () => {
|
||||
it('a phase with one CRLF SUMMARY.md and one LF SUMMARY.md yields the union of both', () => {
|
||||
// Two plans in the same phase. Plan A is saved LF, plan B is saved CRLF
|
||||
// (Windows checkout / CRLF-saving editor). Each contributes distinct files.
|
||||
const planA_LF = [
|
||||
'---',
|
||||
'type: summary',
|
||||
'key_files:',
|
||||
' modified:',
|
||||
' - src/alpha.ts',
|
||||
'---',
|
||||
'',
|
||||
'Plan A body.',
|
||||
].join('\n') + '\n';
|
||||
|
||||
const planB_CRLF = [
|
||||
'---',
|
||||
'type: summary',
|
||||
'key_files:',
|
||||
' created:',
|
||||
' - src/beta.ts',
|
||||
' - lib/gamma.js',
|
||||
'---',
|
||||
'',
|
||||
'Plan B body.',
|
||||
].join('\r\n') + '\r\n';
|
||||
|
||||
// The shipped loop iterates each summary and accumulates into REVIEW_FILES.
|
||||
const reviewFiles = [];
|
||||
for (const doc of [planA_LF, planB_CRLF]) {
|
||||
for (const f of extractTier2Files(doc)) reviewFiles.push(f);
|
||||
}
|
||||
|
||||
// The union — NOT just plan A's files. Pre-fix, planB_CRLF contributed
|
||||
// nothing (boundary returned null), so reviewFiles would have been only
|
||||
// ['src/alpha.ts'] with no warning (the aggregate was non-zero, so the
|
||||
// Tier-3 eq-zero fallback at code-review.md:217 never fired).
|
||||
assert.deepStrictEqual(
|
||||
reviewFiles.sort(),
|
||||
['lib/gamma.js', 'src/alpha.ts', 'src/beta.ts'],
|
||||
'A mixed CRLF/LF phase must review the union of both artifacts. ' +
|
||||
'If planB_CRLF dropped out, the fix regressed: ' + JSON.stringify(reviewFiles)
|
||||
);
|
||||
});
|
||||
|
||||
it('RED proof: with the buggy boundary, the CRLF artifact contributes nothing (silent partial)', () => {
|
||||
// Same scenario, but using the BUGGY boundary replica to demonstrate the
|
||||
// exact silent-partial masking the issue reported: the CRLF plan yields [],
|
||||
// so the phase scope is just the LF plan's files, with zero warning.
|
||||
const planA_LF = ['---', 'key_files:', ' modified:', ' - src/alpha.ts', '---', ''].join('\n') + '\n';
|
||||
const planB_CRLF = ['---', 'key_files:', ' created:', ' - src/beta.ts', '---', ''].join('\r\n') + '\r\n';
|
||||
|
||||
const buggyFiles = [];
|
||||
for (const doc of [planA_LF, planB_CRLF]) {
|
||||
const yaml = extractFrontmatterBoundaryBuggy(doc);
|
||||
if (yaml === null) continue; // buggy boundary returns null on CRLF -> skip
|
||||
for (const f of parseFilesWithFixedLogic(yaml)) buggyFiles.push(f);
|
||||
}
|
||||
|
||||
// The bug: only the LF plan's file survives; the CRLF plan's file is gone,
|
||||
// and because the aggregate is non-zero (1), no fallback warning fired.
|
||||
assert.deepStrictEqual(buggyFiles.sort(), ['src/alpha.ts']);
|
||||
assert.ok(
|
||||
!buggyFiles.includes('src/beta.ts'),
|
||||
'The buggy boundary must have dropped the CRLF artifact file (RED demonstration).'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shipped workflow frontmatter boundary is CRLF-safe (#2694 structural guard)', () => {
|
||||
// The two shipped workflow files whose inline `node -e` one-liners extract
|
||||
// frontmatter. Resolved relative to the repo root (the test runner's CWD is the
|
||||
// repo root under gsd-test).
|
||||
const WORKFLOW_FILES = [
|
||||
path.join('gsd-core', 'workflows', 'code-review.md'),
|
||||
path.join('gsd-core', 'workflows', 'code-review-fix.md'),
|
||||
];
|
||||
|
||||
for (const rel of WORKFLOW_FILES) {
|
||||
it(`${rel}: every frontmatter-boundary site normalizes CRLF before matching`, () => {
|
||||
const src = fs.readFileSync(rel, 'utf-8');
|
||||
|
||||
// Locate every shipped boundary-match site. The shipped line shape is:
|
||||
// const match = content...match(/^---\n([\s\S]*?)\n---/);
|
||||
// After #2694 the `content...` part must be `content.replace(/\r\n/g, '\n')`.
|
||||
// Assert no site uses a raw `content.match(...)` against the boundary regex,
|
||||
// and that the CRLF normalize is present at each site.
|
||||
const lines = src.split(/\r?\n/);
|
||||
const boundarySites = [];
|
||||
const unsafeSites = [];
|
||||
for (let i = 0; i < lines.length; i += 1) {
|
||||
const line = lines[i];
|
||||
if (!/\/\^---\\n\(\[\\s\\S\]\*\?\)\\n---\/\)/.test(line)) continue;
|
||||
boundarySites.push(i + 1);
|
||||
// SAFE: the match is taken on the result of content.replace(/\r\n/g, '\n').
|
||||
// UNSAFE: a direct content.match(/<boundary>/) with no preceding normalize.
|
||||
const isNormalized = /content\.replace\(\s*\/\\r\\n\/g\s*,\s*'\\n'\s*\)\.match\(/.test(line);
|
||||
const isRawContentMatch = /(^|[^.])\bcontent\.match\(\s*\/\^---\\n/.test(line);
|
||||
if (!isNormalized || isRawContentMatch) {
|
||||
unsafeSites.push({ line: i + 1, text: line.trim() });
|
||||
}
|
||||
}
|
||||
|
||||
assert.ok(
|
||||
boundarySites.length >= 3,
|
||||
`${rel}: expected several frontmatter-boundary sites; found ${boundarySites.length}. ` +
|
||||
'If the workflow no longer uses this regex, update this guard.'
|
||||
);
|
||||
assert.deepStrictEqual(
|
||||
unsafeSites,
|
||||
[],
|
||||
`${rel}: ${unsafeSites.length} frontmatter-boundary site(s) lack the CRLF normalize ` +
|
||||
'(regression of #2694): ' + JSON.stringify(unsafeSites, null, 2)
|
||||
);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
@@ -9,6 +9,12 @@
|
||||
},
|
||||
"hooks/managed-hooks-registry.cjs": {
|
||||
"reason": "#2695: the Codex installer now delivers the complete four-file update-check hook set for every profile. gsd-check-update.js spawn()s gsd-check-update-worker.js, which require()s managed-hooks-registry.cjs for MANAGED_HOOKS — so the registry is now emitted into Codex installs byte-for-byte. The diff is in bin/install.js (allowlist + raw-copy fallback + profile gate), not in the registry source, so the hooks-built attribution rule flags the emitted registry path. This ripple is the intended fix."
|
||||
},
|
||||
"code-review.md": {
|
||||
"reason": "#2694: three inline node -e frontmatter-boundary one-liners now normalize \\r\\n -> \\n before matching (content.replace(/\\r\\n/g,'\\n').match(...)), so CRLF-saved SUMMARY.md/REVIEW.md artifacts are no longer silently dropped. The growth is the inserted .replace(/\\r\\n/g,'\\n') at each of the 3 boundary sites; no observable command shape changed otherwise. This is the intended fix."
|
||||
},
|
||||
"code-review-fix.md": {
|
||||
"reason": "#2694: six inline node -e frontmatter-boundary one-liners now normalize \\r\\n -> \\n before matching (content.replace(/\\r\\n/g,'\\n').match(...)), so CRLF-saved REVIEW.md/REVIEW-FIX.md artifacts are no longer silently dropped (status/depth/files_reviewed_list extraction). The growth is the inserted .replace(/\\r\\n/g,'\\n') at each of the 6 boundary sites; no observable command shape changed otherwise. This is the intended fix."
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user