diff --git a/.changeset/lazy-lemurs-jump.md b/.changeset/lazy-lemurs-jump.md new file mode 100644 index 000000000..dcc0f768a --- /dev/null +++ b/.changeset/lazy-lemurs-jump.md @@ -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. diff --git a/src/check-command-router.cts b/src/check-command-router.cts index c0d8f59a1..a20f65e60 100644 --- a/src/check-command-router.cts +++ b/src/check-command-router.cts @@ -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)) { diff --git a/tests/check-tdd-review-checkpoint-e2e.test.cjs b/tests/check-tdd-review-checkpoint-e2e.test.cjs index c3c4a10b2..83a18fda1 100644 --- a/tests/check-tdd-review-checkpoint-e2e.test.cjs +++ b/tests/check-tdd-review-checkpoint-e2e.test.cjs @@ -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)