fix(#2449): make tdd.review-checkpoint frontmatter regex CRLF-tolerant (#2477)

* test(#2449): failing-first CRLF regression for tdd.review-checkpoint

cmdTddReviewCheckpoint's frontmatter regex /^---\\n...\\n---/ (line 751)
cannot match CRLF PLAN.md, so a type:tdd plan with Windows line endings
was silently classified as 'no type:tdd plans' — output indistinguishable
from a phase that genuinely contains no TDD plans. The advisory gate then
short-circuited to a confident pass with no violations table.

Adds tddPlanCrlf() fixture helper (CRLF twin of tddPlan) and a [crlf]
test case that asserts tddPlans===1 for a CRLF type:tdd plan (would be 0
before the fix), plus block:true/violations:1 since the fixture has no
RED/GREEN commits.

* fix(#2449): make tdd-review-checkpoint frontmatter regex CRLF-tolerant

The frontmatter delimiter regex at src/check-command-router.cts:751 used
literal \\n which cannot match a CRLF PLAN.md delimiter (---\\r\\n).
frontmatterMatch was null, the plan was never classified type:tdd,
tddPlanFiles stayed empty, and the advisory gate short-circuited to a
confident pass with no violations table.

The fix replaces /^---\\n([\\s\\S]*?)\\n---/ with
/^---\\r?\\n([\\s\\S]*?)\\r?\\n---/ — the same CRLF-tolerant form
already used elsewhere in the same file at line 205
(extractPlanDesignatedSections). Same canonical pattern; one-line change.

Same bug class previously fixed in #1658, #1668, #2206; this instance is
in a different file and is not a duplicate of any of them.

* fix(#2444): re-resolve body-parser to 2.3.0 in lockfile (GHSA-v422-hmwv-36x6)

GHSA-v422-hmwv-36x6 (body-parser DoS via invalid limit value, low severity,
published 2026-07-20T23:23:26Z) made tests/npm-integrity-gate.test.cjs
(#3588: root workspace production tree has no advisories) fail any subsequent
npm audit --omit=dev. The advisory affects body-parser >=2.0.0 <2.3.0 pulled
transitively via @anthropic-ai/claude-agent-sdk -> @modelcontextprotocol/sdk
-> express -> body-parser@2.2.2.

express@5.2.1 already declares body-parser as ^2.2.1, so 2.3.0 is a valid
re-resolution within express's own compatibility range — no override needed.
Regenerated the lockfile via 'npm audit fix --omit=dev' which re-resolves
transitive deps within their declared ranges; package.json is unchanged.

Verified: npm audit --omit=dev reports 0/0/0/0/0 advisories; body-parser
now reads as 2.3.0 in 'npm ls body-parser --omit=dev'.

* test(#2449): add mixed-endings (CRLF frontmatter + LF body) variant

Per code-review Low finding: CONTRIBUTING.md:490 names 'Mixed CRLF/LF
newlines' as a required adversarial fixture class. The pure-CRLF [crlf]
test covers the reported bug (Windows editor + autocrlf=input). This
[crlf-mixed] variant covers the more adversarial case where frontmatter
delimiters are CRLF but the body is LF (editor that normalizes body text,
or a toolchain concatenating CRLF + LF fragments). The classifier only
reads frontmatter, so detection is unaffected — but the test locks the
behavior.

* docs(changeset): add Fixed fragment for #2449 PR

* docs(changeset): backfill PR number to 2477
This commit is contained in:
Tom Boucher
2026-07-21 08:13:13 -04:00
committed by GitHub
parent 46ba9ed464
commit b6ebf9d4b4
3 changed files with 88 additions and 1 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 2477
---
**`check tdd.review-checkpoint` no longer silently skips TDD plans with CRLF line endings** — the frontmatter regex at `src/check-command-router.cts:751` used literal `\n` which couldn't match a CRLF PLAN.md delimiter (`---\r\n`), so a Windows-authored `type: tdd` plan was silently classified as "no type:tdd plans found" and the advisory gate short-circuited to a confident pass with no violations table. The regex now uses the same CRLF-tolerant form (`/^---\r?\n([\s\S]*?)\r?\n---/`) already in use elsewhere in the same file (line 205, `extractPlanDesignatedSections`). With `core.autocrlf=input`, the triggering CRLF was invisible to `git diff`/`git status`, so the contributor had no way to tell their plan was being misclassified.

View File

@@ -748,7 +748,11 @@ function cmdTddReviewCheckpoint(projectDir: string, args: string[], raw: boolean
const planPath = path.join(phaseDir, file);
const content = readIfExists(planPath);
// Check frontmatter for type: tdd
const frontmatterMatch = content.match(/^---\n([\s\S]*?)\n---/);
// CRLF-tolerant: a PLAN.md written with Windows line endings (---\r\n...---)
// must still match. The same CRLF-tolerant form is already used at line 205
// (extractPlanDesignatedSections); this is the same canonical pattern, applied
// here for the tdd-classification path. Fixes #2449.
const frontmatterMatch = content.match(/^---\r?\n([\s\S]*?)\r?\n---/);
if (frontmatterMatch) {
const fm = frontmatterMatch[1];
if (/^type:\s*tdd\s*$/m.test(fm)) {

View File

@@ -96,6 +96,28 @@ function tddPlan(phaseNum, planId) {
return `---\ntype: tdd\nphase: ${phaseNum}\nslug: ${planId}\n---\n# Task: ${planId}\n`;
}
/**
* Same as tddPlan but with CRLF line endings — reproduces #2449.
* Some Windows editors / toolchains write PLAN.md with CRLF; with
* core.autocrlf=input the working-tree CRLF is invisible to git diff/status
* (the blob is normalized on commit), so the contributor cannot tell their
* plan is being silently misclassified.
*/
function tddPlanCrlf(phaseNum, planId) {
return `---\r\ntype: tdd\r\nphase: ${phaseNum}\r\nslug: ${planId}\r\n---\r\n# Task: ${planId}\r\n`;
}
/**
* Mixed-endings variant: CRLF frontmatter + LF body. Realistic when a Windows
* editor normalizes Markdown body text but preserves frontmatter byte-for-byte,
* or when tooling concatenates CRLF + LF fragments. The frontmatter delimiter
* still must match (the bug class), and the LF body must not be confused by
* downstream code that splits on \n (it gets an LF-prefixed body).
*/
function tddPlanMixedCrlfLf(phaseNum, planId) {
return `---\r\ntype: tdd\r\nphase: ${phaseNum}\r\nslug: ${planId}\r\n---\n# Task: ${planId}\n`;
}
/**
* Build a type:execute PLAN.md (non-TDD) content.
*/
@@ -397,6 +419,62 @@ describe('check tdd.review-checkpoint — CLI subprocess E2E with git fixtures',
}
});
test('[crlf] type:tdd plan with CRLF endings is still detected (#2449)', () => {
// src/check-command-router.cts:751 used /^---\n...\n---/ which can't match
// CRLF frontmatter delimiters, so a CRLF PLAN.md was silently classified
// as "no type:tdd plans" — output indistinguishable from a phase that
// genuinely contains no TDD plans. The advisory gate then short-circuited
// to a confident pass with no violations table.
const { tmpDir } = createTddGitFixture({
planFiles: [
{ dir: '01-phase1', filename: '01-01-PLAN.md', content: tddPlanCrlf(1, '01-01') },
],
});
try {
const result = runTools('check tdd.review-checkpoint 1 --raw', tmpDir);
assert.ok(result.success, `check should succeed. stderr: ${result.error}`);
const out = JSON.parse(result.output);
// SPECIFIC assertion: tddPlans must be 1, not 0 — this is the bug.
// Before the fix, the CRLF frontmatter regex returned null, the plan was
// never classified type:tdd, and tddPlanFiles stayed empty.
assert.strictEqual(out.tddPlans, 1, 'CRLF type:tdd plan must be detected (was silently 0 before #2449 fix)');
// No RED/GREEN commits in the fixture → block:true, violations:1.
assert.strictEqual(out.block, true, 'no commits on the detected plan: block must be TRUE');
assert.strictEqual(out.violations, 1, 'one violation expected for the detected plan');
assert.strictEqual(out.rows.length, 1, 'the detected plan must produce a row');
assert.strictEqual(out.rows[0].planId, '01-01', 'planId must be 01-01');
} finally {
cleanup(tmpDir);
}
});
test('[crlf-mixed] type:tdd plan with CRLF frontmatter + LF body is still detected (#2449)', () => {
// Mixed-endings variant of the [crlf] test: frontmatter delimiters are CRLF
// (the bug class) but the body is LF (e.g. an editor that normalizes body
// text but preserves frontmatter bytes, or a toolchain that concatenates
// CRLF + LF fragments). The classifier only reads frontmatter, so the LF
// body must not change detection — but verify, since the original bug was
// a parser differential and mixed endings are the most adversarial case.
const { tmpDir } = createTddGitFixture({
planFiles: [
{ dir: '01-phase1', filename: '01-01-PLAN.md', content: tddPlanMixedCrlfLf(1, '01-01') },
],
});
try {
const result = runTools('check tdd.review-checkpoint 1 --raw', tmpDir);
assert.ok(result.success, `check should succeed. stderr: ${result.error}`);
const out = JSON.parse(result.output);
assert.strictEqual(out.tddPlans, 1, 'CRLF-frontmatter + LF-body type:tdd plan must be detected');
assert.strictEqual(out.rows[0].planId, '01-01', 'planId must be 01-01');
} finally {
cleanup(tmpDir);
}
});
test('[bva] violations=1 boundary: exactly 1 violation sets block:true and advisory message present', () => {
// Two tdd plans: 01-01 passes (both commits), 01-02 fails (no commits)
// violations = 1 exactly — boundary test (violations > 0 → block:true)