fix(#2769): tolerate Requirements header with colon inside bold delimiters (#2782)

* fix(#2769): tolerate Requirements header with colon inside bold delimiters extractReqIds in sdk/src/query/init.ts and the legacy init.cjs port only matched `**Requirements**:` (colon outside bold), so phases declared with the equally-valid markdown form `**Requirements:**` (colon inside bold, which is what the project's own templates emit) returned phase_req_ids: null for both `init plan-phase` and `init execute-phase`. The mirror-image bug in `phase complete`'s REQUIREMENTS.md traceability sweep at get-shit-done/bin/lib/phase.cjs:871 only matched the inside-bold form, silently skipping the REQ-ID checkbox flips for any roadmap that used the outside-bold form. Both parsers now share the same canonical regex that accepts all three rendered-identical variants: **Requirements:** (colon inside bold) **Requirements**: (colon outside bold) **Requirements** : (space before outside colon) Tests: - tests/init.test.cjs — parameterized over the three header variants for both init plan-phase and init execute-phase (6 new behavioral cases). - sdk/src/query/init.test.ts — describe.each over the same variants exercising initPlanPhase through the SDK. - tests/bug-2769-requirements-header-variants.test.cjs — phase complete flips REQ-001 in REQUIREMENTS.md across all three header variants. Closes #2769 * refactor(#2769): centralize REQUIREMENTS_HEADER_RE constant per CodeRabbit
2026-04-27 12:31:49 -04:00
parent 54e6da3126
commit 936cf26706
6 changed files with 227 additions and 4 deletions
--- a/get-shit-done/bin/lib/init.cjs
+++ b/get-shit-done/bin/lib/init.cjs
@@ -7,6 +7,11 @@ const path = require('path');
 const { execSync } = require('child_process');
 const { loadConfig, resolveModelInternal, findPhaseInternal, getRoadmapPhaseInternal, pathExistsInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, normalizePhaseName, planningPaths, planningDir, planningRoot, toPosixPath, output, error, checkAgentsInstalled, phaseTokenMatches } = require('./core.cjs');

+// Accept all bold/colon variants of the Requirements header (#2769):
+// **Requirements:** / **Requirements**: / **Requirements** : render the
+// same in markdown but differ textually.
+const REQUIREMENTS_HEADER_RE = /^\*\*Requirements:?\*\*[^\S\n]*:?[^\S\n]*([^\n]*)$/m;
+
 function getLatestCompletedMilestone(cwd) {
  const milestonesPath = path.join(planningRoot(cwd), 'MILESTONES.md');
  if (!fs.existsSync(milestonesPath)) return null;
@@ -102,7 +107,7 @@ function cmdInitExecutePhase(cwd, phase, raw, options = {}) {
      has_reviews: false,
    };
  }
-  const reqMatch = roadmapPhase?.section?.match(/^\*\*Requirements\*\*:[^\S\n]*([^\n]*)$/m);
+  const reqMatch = roadmapPhase?.section?.match(REQUIREMENTS_HEADER_RE);
  const reqExtracted = reqMatch
    ? reqMatch[1].replace(/[\[\]]/g, '').split(',').map(s => s.trim()).filter(Boolean).join(', ')
    : null;
@@ -235,7 +240,7 @@ function cmdInitPlanPhase(cwd, phase, raw, options = {}) {
      has_reviews: false,
    };
  }
-  const reqMatch = roadmapPhase?.section?.match(/^\*\*Requirements\*\*:[^\S\n]*([^\n]*)$/m);
+  const reqMatch = roadmapPhase?.section?.match(REQUIREMENTS_HEADER_RE);
  const reqExtracted = reqMatch
    ? reqMatch[1].replace(/[\[\]]/g, '').split(',').map(s => s.trim()).filter(Boolean).join(', ')
    : null;
--- a/get-shit-done/bin/lib/phase.cjs
+++ b/get-shit-done/bin/lib/phase.cjs
@@ -868,7 +868,11 @@ function cmdPhaseComplete(cwd, phaseNum, raw) {
        );

        const sectionText = phaseSectionMatch ? phaseSectionMatch[1] : '';
-        const reqMatch = sectionText.match(/\*\*Requirements:\*\*\s*([^\n]+)/i);
+        // Accept all bold/colon variants (#2769) — the previous pattern only
+        // matched **Requirements:** (colon inside bold) and silently skipped
+        // **Requirements**: (colon outside), preventing the matching REQ-IDs
+        // from being ticked off in REQUIREMENTS.md on phase completion.
+        const reqMatch = sectionText.match(/\*\*Requirements:?\*\*[^\S\n]*:?[^\S\n]*([^\n]+)/i);

        let reqContent = fs.readFileSync(reqPath, 'utf-8');

--- a/sdk/src/query/init.test.ts
+++ b/sdk/src/query/init.test.ts
@@ -351,6 +351,40 @@ describe('initPlanPhase', () => {
    const data = result.data as Record<string, unknown>;
    expect(data.error).toBeDefined();
  });
+
+  // #2769: extractReqIds must accept all bold/colon variants of the
+  // Requirements header. The forms render identically in markdown but differ
+  // textually; the previous regex only matched **Requirements**: (colon
+  // outside bold) and silently returned null for **Requirements:** (colon
+  // inside bold) and **Requirements** : (spaced).
+  describe.each([
+    { name: 'colon inside bold', header: '**Requirements:** RV-01, RV-02' },
+    { name: 'colon outside bold', header: '**Requirements**: RV-01, RV-02' },
+    { name: 'space before colon', header: '**Requirements** : RV-01, RV-02' },
+  ])('phase_req_ids extraction (#2769)', ({ name, header }) => {
+    it(`parses Requirements header with ${name}`, async () => {
+      // Overwrite ROADMAP.md so phase 9 carries the variant header.
+      await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), [
+        '# Roadmap',
+        '',
+        '## v3.0: SDK-First Migration',
+        '',
+        '### Phase 9: Foundation',
+        '',
+        '**Goal:** Build foundation',
+        header,
+        '',
+        '### Phase 10: Read-Only Queries',
+        '',
+        '**Goal:** Implement queries',
+        '',
+      ].join('\n'));
+
+      const result = await initPlanPhase(['9'], tmpDir);
+      const data = result.data as Record<string, unknown>;
+      expect(data.phase_req_ids).toBe('RV-01, RV-02');
+    });
+  });
 });

 describe('initNewMilestone', () => {
--- a/sdk/src/query/init.ts
+++ b/sdk/src/query/init.ts
@@ -203,7 +203,12 @@ async function getPhaseInfoForVerifyWork(
 */
 function extractReqIds(roadmapPhase: Record<string, unknown> | null): string | null {
  const section = roadmapPhase?.section as string | undefined;
-  const reqMatch = section?.match(/^\*\*Requirements\*\*:[^\S\n]*([^\n]*)$/m);
+  // Accept all bold/colon variants of the Requirements header. The forms
+  //   **Requirements:**  (colon inside bold)
+  //   **Requirements**:  (colon outside bold)
+  //   **Requirements** : (space before outside colon)
+  // render identically in markdown but differ textually. Issue #2769.
+  const reqMatch = section?.match(/^\*\*Requirements:?\*\*[^\S\n]*:?[^\S\n]*([^\n]*)$/m);
  const reqExtracted = reqMatch
    ? reqMatch[1].replace(/[\[\]]/g, '').split(',').map((s: string) => s.trim()).filter(Boolean).join(', ')
    : null;
--- a/tests/bug-2769-requirements-header-variants.test.cjs
+++ b/tests/bug-2769-requirements-header-variants.test.cjs
@@ -0,0 +1,117 @@
+/**
+ * Regression tests for issue #2769
+ *
+ * The Requirements header in ROADMAP.md phase blocks renders identically in
+ * markdown for three textually distinct forms:
+ *
+ *   **Requirements:**          colon INSIDE bold delimiters
+ *   **Requirements**:          colon OUTSIDE bold delimiters
+ *   **Requirements** :         space-then-colon outside bold
+ *
+ * Two parsers in the codebase used opposing strict regexes — one only
+ * matched the outside-colon form (init.cjs / init.ts), the other only the
+ * inside-colon form (phase.cjs `cmdPhaseComplete` REQUIREMENTS.md
+ * traceability sweep). Both must accept all three variants so phase
+ * metadata propagation is robust to authoring style.
+ *
+ * Tests for the init query side live in `tests/init.test.cjs` (parameterized
+ * over the three variants). This file exercises the inverse bug in
+ * `phase complete`: the REQUIREMENTS.md checkbox must flip when ROADMAP
+ * uses the outside-colon form, which previously was silently skipped.
+ */
+
+'use strict';
+
+const { describe, test, beforeEach, afterEach } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
+
+describe('bug #2769: phase complete ticks REQUIREMENTS.md across header variants', () => {
+  let tmpDir;
+
+  beforeEach(() => {
+    tmpDir = createTempProject();
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'STATE.md'),
+      ['---', 'current_phase: 1', 'status: executing', '---', '# State', ''].join('\n'),
+    );
+  });
+
+  afterEach(() => {
+    cleanup(tmpDir);
+  });
+
+  const headerVariants = [
+    { name: 'colon inside bold (**Requirements:**)', header: '**Requirements:** REQ-001' },
+    { name: 'colon outside bold (**Requirements**:)', header: '**Requirements**: REQ-001' },
+    { name: 'space before colon (**Requirements** :)', header: '**Requirements** : REQ-001' },
+  ];
+
+  for (const variant of headerVariants) {
+    test(`flips REQ-001 checkbox in REQUIREMENTS.md when ROADMAP uses ${variant.name}`, () => {
+      const phasesDir = path.join(tmpDir, '.planning', 'phases', '01-foundation');
+      fs.mkdirSync(phasesDir, { recursive: true });
+      fs.writeFileSync(
+        path.join(phasesDir, '01-1-PLAN.md'),
+        ['---', 'phase: 1', 'plan: 1', '---', '# Plan 1', ''].join('\n'),
+      );
+      fs.writeFileSync(
+        path.join(phasesDir, '01-1-SUMMARY.md'),
+        ['---', 'status: complete', '---', '# Summary', 'Done.'].join('\n'),
+      );
+
+      const roadmap = [
+        '# Roadmap',
+        '',
+        '### Phase 1: Foundation',
+        '',
+        '**Goal:** Build core',
+        variant.header,
+        '**Plans:** 1 plans',
+        '',
+        'Plans:',
+        '- [x] 01-1-PLAN.md',
+        '',
+        '| Phase | Plans | Status | Completed |',
+        '|-------|-------|--------|-----------|',
+        '| 1. Foundation | 0/1 | Pending | - |',
+      ].join('\n');
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap);
+
+      const requirements = [
+        '# Requirements',
+        '',
+        '## Functional Requirements',
+        '',
+        '- [ ] **REQ-001**: Core data model',
+        '',
+        '## Traceability',
+        '',
+        '| REQ-ID | Phase | Status |',
+        '|--------|-------|--------|',
+        '| REQ-001 | 1 | Pending |',
+      ].join('\n');
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), requirements);
+
+      const result = runGsdTools(['phase', 'complete', '1'], tmpDir);
+      assert.ok(result.success, `phase complete failed: ${result.error}`);
+
+      const updated = fs.readFileSync(
+        path.join(tmpDir, '.planning', 'REQUIREMENTS.md'),
+        'utf-8',
+      );
+      assert.match(
+        updated,
+        /-\s*\[x\]\s*\*\*REQ-001\*\*/,
+        `REQ-001 checkbox must be flipped to [x] when ROADMAP header is "${variant.header}". Got:\n${updated}`,
+      );
+      assert.match(
+        updated,
+        /\|\s*REQ-001\s*\|\s*1\s*\|\s*Complete\s*\|/,
+        `Traceability row for REQ-001 must be marked Complete. Got:\n${updated}`,
+      );
+    });
+  }
+});
--- a/tests/init.test.cjs
+++ b/tests/init.test.cjs
@@ -252,6 +252,64 @@ describe('init commands', () => {
    assert.strictEqual(output.phase_req_ids, null, 'TBD placeholder should return null');
  });

+  // ── #2769: Requirements header bold/colon variants ───────────────────────
+  // The visible label "**Requirements:**" (colon INSIDE bold) and
+  // "**Requirements**:" (colon OUTSIDE bold) render identically. The parser
+  // must accept both, plus the spaced "**Requirements** :" variant and the
+  // plain "## Requirements" header form (used in REQUIREMENTS.md), so phase
+  // metadata is robust to authoring style.
+  const headerVariants = [
+    { name: 'colon inside bold (**Requirements:**)', header: '**Requirements:** RV-01, RV-02' },
+    { name: 'colon outside bold (**Requirements**:)', header: '**Requirements**: RV-01, RV-02' },
+    { name: 'space before colon (**Requirements** :)', header: '**Requirements** : RV-01, RV-02' },
+  ];
+
+  for (const variant of headerVariants) {
+    test(`init plan-phase parses Requirements with ${variant.name}`, () => {
+      fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '03-api'), { recursive: true });
+      const roadmap = [
+        '# Roadmap',
+        '',
+        '### Phase 3: API',
+        '**Goal:** Build API',
+        variant.header,
+        '**Plans:** 0 plans',
+        '',
+      ].join('\n');
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap);
+
+      const result = runGsdTools('init plan-phase 3', tmpDir);
+      assert.ok(result.success, `Command failed: ${result.error}`);
+
+      const output = JSON.parse(result.output);
+      assert.strictEqual(output.phase_req_ids, 'RV-01, RV-02',
+        `phase_req_ids must be parsed when header uses "${variant.header}"`);
+    });
+
+    test(`init execute-phase parses Requirements with ${variant.name}`, () => {
+      const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api');
+      fs.mkdirSync(phaseDir, { recursive: true });
+      fs.writeFileSync(path.join(phaseDir, '03-01-PLAN.md'), '# Plan');
+      const roadmap = [
+        '# Roadmap',
+        '',
+        '### Phase 3: API',
+        '**Goal:** Build API',
+        variant.header,
+        '**Plans:** 1 plans',
+        '',
+      ].join('\n');
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap);
+
+      const result = runGsdTools('init execute-phase 3', tmpDir);
+      assert.ok(result.success, `Command failed: ${result.error}`);
+
+      const output = JSON.parse(result.output);
+      assert.strictEqual(output.phase_req_ids, 'RV-01, RV-02',
+        `phase_req_ids must be parsed when header uses "${variant.header}"`);
+    });
+  }
+
  test('init execute-phase returns null phase_req_ids when Requirements line is absent', () => {
    const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api');
    fs.mkdirSync(phaseDir, { recursive: true });