From 69a3427189f863abe3a598e9b46b78317bd276cc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 20 May 2026 23:58:31 -0400 Subject: [PATCH 1/3] fix(3691): match all Plans-block variants in annotate-dependencies Three regex defects in cmdRoadmapAnnotateDependencies (roadmap.cjs): 1. Plans-block detection (line ~553): `Plans:\s*\n` required no text after the colon, silently skipping `Plans: 3 plans\n` and `**Plans:** N\n`. Fixed: `\*{0,2}Plans\*{0,2}:[^\n]*\n` + require `+` checklist lines so a bold summary line above a bare `Plans:` block doesn't consume the match. 2. Phase-section boundary (line ~542): `\d` matched only one digit, so `### Phase 02.3:` was not recognised as a section terminator, allowing plan-list content from adjacent decimal phases to bleed in. Fixed with `\d[\d.]*`. Same one-digit boundary also patched in roadmap.cjs (analyze path), phase.cjs (insert-after path), and init.cjs (section-slice path). 3. Plan-ID extraction (line ~566): `[\w-]+?` excluded `.`, capturing `02` from `02.3-01-PLAN.md` instead of `02.3-01`, so planData.find never resolved and every plan defaulted to wave 1. Fixed: `[\w.-]+?`. Co-Authored-By: Claude Sonnet 4.6 --- get-shit-done/bin/lib/init.cjs | 4 +- get-shit-done/bin/lib/phase.cjs | 4 +- get-shit-done/bin/lib/roadmap.cjs | 25 +- ...nnotate-deps-plans-block-variants.test.cjs | 325 ++++++++++++++++++ 4 files changed, 351 insertions(+), 7 deletions(-) create mode 100644 tests/bug-3691-annotate-deps-plans-block-variants.test.cjs diff --git a/get-shit-done/bin/lib/init.cjs b/get-shit-done/bin/lib/init.cjs index 04fc77cbd..80029a21f 100644 --- a/get-shit-done/bin/lib/init.cjs +++ b/get-shit-done/bin/lib/init.cjs @@ -1170,7 +1170,9 @@ function cmdInitManager(cwd, raw) { const sectionStart = match.index; const restOfContent = content.slice(sectionStart); - const nextHeader = restOfContent.match(/\n#{2,4}\s+Phase\s+\d/i); + // #3691: `\d` → `\d[\d.]*` so decimal phase headings (e.g. `### Phase 02.3:`) are + // recognised as section boundaries. + const nextHeader = restOfContent.match(/\n#{2,4}\s+Phase\s+\d[\d.]*/i); const sectionEnd = nextHeader ? sectionStart + nextHeader.index : content.length; const section = content.slice(sectionStart, sectionEnd); diff --git a/get-shit-done/bin/lib/phase.cjs b/get-shit-done/bin/lib/phase.cjs index dbea48464..f197ac9ba 100644 --- a/get-shit-done/bin/lib/phase.cjs +++ b/get-shit-done/bin/lib/phase.cjs @@ -882,7 +882,9 @@ function cmdPhaseInsert(cwd, afterPhase, description, raw) { 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/i); + // #3691: `\d` → `\d[\d.]*` so decimal phase headings (e.g. `### Phase 02.3:`) are + // recognised as section boundaries. + const nextPhaseMatch = afterHeader.match(/\n#{2,4}\s+Phase\s+\d[\d.]*/i); let insertIdx; if (nextPhaseMatch) { diff --git a/get-shit-done/bin/lib/roadmap.cjs b/get-shit-done/bin/lib/roadmap.cjs index 901b0d3a9..692530307 100644 --- a/get-shit-done/bin/lib/roadmap.cjs +++ b/get-shit-done/bin/lib/roadmap.cjs @@ -223,7 +223,9 @@ function cmdRoadmapAnalyze(cwd, raw) { // Extract goal from the section const sectionStart = match.index; const restOfContent = content.slice(sectionStart); - const nextHeader = restOfContent.match(/\n#{2,4}\s+Phase\s+\d/i); + // #3691: `\d` → `\d[\d.]*` so decimal phase headings (e.g. `### Phase 02.3:`) are + // recognised as section boundaries. + const nextHeader = restOfContent.match(/\n#{2,4}\s+Phase\s+\d[\d.]*/i); const sectionEnd = nextHeader ? sectionStart + nextHeader.index : content.length; const section = content.slice(sectionStart, sectionEnd); @@ -539,7 +541,9 @@ function cmdRoadmapAnnotateDependencies(cwd, phaseNum, raw) { const phaseStart = phaseMatch.index; const restAfterHeader = content.slice(phaseStart); - const nextPhaseOffset = restAfterHeader.slice(1).search(/\n#{2,4}\s+Phase\s+\d/i); + // #3691 Bug 2: `\d` only matches a single digit, missing decimal phase headings + // like `### Phase 02.3:`. Use `\d[\d.]*` to match integers and decimals alike. + const nextPhaseOffset = restAfterHeader.slice(1).search(/\n#{2,4}\s+Phase\s+\d[\d.]*/i); const phaseEnd = nextPhaseOffset >= 0 ? phaseStart + 1 + nextPhaseOffset : content.length; const phaseSection = content.slice(phaseStart, phaseEnd); @@ -549,8 +553,16 @@ function cmdRoadmapAnnotateDependencies(cwd, phaseNum, raw) { /\*\*Cross-cutting constraints:\*\*/i.test(phaseSection) ) return; - // Find the Plans: section within the phase section - const plansBlockMatch = phaseSection.match(/(Plans:\s*\n)((?:\s*-\s*\[[ x]\][^\n]*\n?)*)/i); + // Find the Plans: section within the phase section. + // #3691 Bug 1: `Plans:\s*\n` required no text after the colon, missing variants like + // `Plans: 3 plans across 2 waves\n` or `**Plans:** 3 plans\n` (bold-wrapped). + // `\*{0,2}Plans\*{0,2}:[^\n]*\n` accepts any text (or none) after the colon + // and tolerates optional `**` markdown bold wrappers on either side. + // The checklist group uses `+` (not `*`) so that a bold `**Plans:**` description + // line with no immediately-following checklist items (e.g. a summary line above a + // separate bare `Plans:` block) does not consume the match and prevent the actual + // list from being found. + const plansBlockMatch = phaseSection.match(/(\*{0,2}Plans\*{0,2}:[^\n]*\n)((?:\s*-\s*\[[ x]\][^\n]*\n?)+)/i); if (!plansBlockMatch) return; const plansHeader = plansBlockMatch[1]; @@ -563,7 +575,10 @@ function cmdRoadmapAnnotateDependencies(cwd, phaseNum, raw) { const linesByWave = new Map(); for (const line of listLines) { // Match plan ID from line: "- [ ] 01-01-PLAN.md — ..." or "- [ ] 01-01: ..." - const idMatch = line.match(/\[\s*[x ]\s*\]\s*([\w-]+?)(?:-PLAN\.md|\.md|:|\s—)/i); + // #3691 Bug 3: `[\w-]+?` excluded `.`, so decimal IDs like `02.3-01` were captured + // as `02` only and never matched planData entries. `[\w.-]+?` preserves the + // terminating alternation (`-PLAN.md|.md|:|\s—`) as the boundary anchor. + const idMatch = line.match(/\[\s*[x ]\s*\]\s*([\w.-]+?)(?:-PLAN\.md|\.md|:|\s—)/i); const planId = idMatch ? idMatch[1] : null; const planEntry = planId ? planData.find(p => p.planId === planId) : null; const wave = planEntry ? planEntry.wave : 1; diff --git a/tests/bug-3691-annotate-deps-plans-block-variants.test.cjs b/tests/bug-3691-annotate-deps-plans-block-variants.test.cjs new file mode 100644 index 000000000..c1e856213 --- /dev/null +++ b/tests/bug-3691-annotate-deps-plans-block-variants.test.cjs @@ -0,0 +1,325 @@ +'use strict'; + +// allow-test-rule: source-text-is-the-product +// Reads .md/.json/.yml product files whose deployed text IS what the +// runtime loads — testing text content tests the deployed contract. + +/** + * Regression — issue #3691 + * + * Three regex defects in `roadmap.cjs` function `cmdRoadmapAnnotateDependencies`: + * + * Bug 1 (line ~553) — Plans-block detection regex `/(Plans:\s*\n)/i` requires no text + * after the colon. Headers like `Plans: 3 plans across 2 waves\n` or + * `**Plans:** 3 plans\n` are silently skipped and the function early-returns. + * + * Bug 2 (line ~542) — Phase-section boundary regex `/\n#{2,4}\s+Phase\s+\d/i` uses + * `\d` (single digit) so decimal phase headings like `### Phase 02.3:` are not + * recognized as boundaries. Content from an adjacent decimal phase may bleed into + * the section being annotated. + * + * Bug 3 (line ~566) — Plan-ID extraction regex `/([\w-]+?)/` excludes `.`, so + * decimal plan IDs like `02.3-01` are captured as `02` only, never match + * the planData entry, and every plan defaults to wave 1. + */ + +const { test, describe, 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'); + +// --------------------------------------------------------------------------- +// Shared test helpers +// --------------------------------------------------------------------------- + +function makePlanProject(files = {}) { + const dir = createTempProject(); + fs.mkdirSync(path.join(dir, '.planning', 'phases'), { recursive: true }); + for (const [rel, content] of Object.entries(files)) { + const abs = path.join(dir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content, 'utf-8'); + } + return dir; +} + +/** Build a minimal PLAN.md frontmatter string */ +function makePlan({ phase, plan, wave, dependsOn = [] }) { + return [ + '---', + `phase: "${phase}"`, + `plan: "${plan}"`, + 'type: standard', + `wave: ${wave}`, + `depends_on: [${dependsOn.map(d => `"${d}"`).join(', ')}]`, + 'files_modified: []', + 'autonomous: true', + 'must_haves:', + ' truths: []', + ' artifacts: []', + ' key_links: []', + '---', + '', + `Plan ${plan}`, + '', + ].join('\n'); +} + +// --------------------------------------------------------------------------- +// Bug 1 — Plans-block detection: inline summary text after the colon +// --------------------------------------------------------------------------- + +describe('bug #3691 — Bug 1: Plans-block detection with inline summary', () => { + let tmpDir; + afterEach(() => cleanup(tmpDir)); + + test('Plans: N plans (inline count after colon) is detected as a Plans-block', (t) => { + // Pre-fix: `Plans:\s*\n` requires bare newline — fails for "Plans: 2 plans\n" + // Post-fix: `Plans:[^\n]*\n` accepts any text after the colon + const roadmap = [ + '# Roadmap', + '', + '### Phase 1: Foundation', + '', + '**Goal:** Set up project', + '', + 'Plans: 2 plans', + '- [ ] 01-01-PLAN.md — Task A', + '- [ ] 01-02-PLAN.md — Task B', + '', + ].join('\n'); + + tmpDir = makePlanProject({ + '.planning/ROADMAP.md': roadmap, + '.planning/phases/01-foundation/01-01-PLAN.md': makePlan({ phase: '1', plan: '01-01', wave: 1 }), + '.planning/phases/01-foundation/01-02-PLAN.md': makePlan({ phase: '1', plan: '01-02', wave: 2, dependsOn: ['01-01'] }), + }); + + const result = runGsdTools('roadmap annotate-dependencies 1', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, true, + 'Plans-block with inline summary must be detected and written'); + assert.ok(out.waves >= 1, 'at least one wave must be written'); + + const written = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + assert.ok(written.includes('Wave'), 'wave annotation must appear in ROADMAP.md'); + }); + + test('Plans: N plans across N waves (longer inline text) is detected', (t) => { + const roadmap = [ + '# Roadmap', + '', + '### Phase 1: Foundation', + '', + 'Plans: 3 plans across 2 waves', + '- [ ] 01-01-PLAN.md — Task A', + '- [ ] 01-02-PLAN.md — Task B', + '- [ ] 01-03-PLAN.md — Task C', + '', + ].join('\n'); + + tmpDir = makePlanProject({ + '.planning/ROADMAP.md': roadmap, + '.planning/phases/01-foundation/01-01-PLAN.md': makePlan({ phase: '1', plan: '01-01', wave: 1 }), + '.planning/phases/01-foundation/01-02-PLAN.md': makePlan({ phase: '1', plan: '01-02', wave: 1 }), + '.planning/phases/01-foundation/01-03-PLAN.md': makePlan({ phase: '1', plan: '01-03', wave: 2, dependsOn: ['01-01'] }), + }); + + const result = runGsdTools('roadmap annotate-dependencies 1', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, true, + 'Plans-block with "N plans across N waves" inline text must be detected'); + }); + + test('**Plans:** (bold markdown wrapper) is detected as a Plans-block', (t) => { + // Bold wrapper: `**Plans:** 3 plans across 2 waves` + const roadmap = [ + '# Roadmap', + '', + '### Phase 1: Foundation', + '', + '**Plans:** 2 plans', + '- [ ] 01-01-PLAN.md — Task A', + '- [ ] 01-02-PLAN.md — Task B', + '', + ].join('\n'); + + tmpDir = makePlanProject({ + '.planning/ROADMAP.md': roadmap, + '.planning/phases/01-foundation/01-01-PLAN.md': makePlan({ phase: '1', plan: '01-01', wave: 1 }), + '.planning/phases/01-foundation/01-02-PLAN.md': makePlan({ phase: '1', plan: '01-02', wave: 2 }), + }); + + const result = runGsdTools('roadmap annotate-dependencies 1', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, true, + '**Plans:** bold-wrapped header must be detected as a Plans-block'); + }); + + test('bare Plans: (no inline text, legacy format) still works after fix', (t) => { + // Regression guard: the fix must not break the working case + const roadmap = [ + '# Roadmap', + '', + '### Phase 1: Foundation', + '', + 'Plans:', + '- [ ] 01-01-PLAN.md — Task A', + '- [ ] 01-02-PLAN.md — Task B', + '', + ].join('\n'); + + tmpDir = makePlanProject({ + '.planning/ROADMAP.md': roadmap, + '.planning/phases/01-foundation/01-01-PLAN.md': makePlan({ phase: '1', plan: '01-01', wave: 1 }), + '.planning/phases/01-foundation/01-02-PLAN.md': makePlan({ phase: '1', plan: '01-02', wave: 2 }), + }); + + const result = runGsdTools('roadmap annotate-dependencies 1', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, true, + 'bare Plans: (legacy format) must still be detected after the fix'); + }); +}); + +// --------------------------------------------------------------------------- +// Bug 3 — Plan-ID extraction: decimal phase IDs like 02.3-01 +// --------------------------------------------------------------------------- + +describe('bug #3691 — Bug 3: decimal plan IDs (e.g. 02.3-01-PLAN.md) parse correctly', () => { + let tmpDir; + afterEach(() => cleanup(tmpDir)); + + test('decimal plan ID 02.3-01 is captured fully and matched to the correct wave', (t) => { + // Pre-fix: `[\w-]+?` stops at `.` → captures `02` only → planData.find misses → wave = 1 for all + // Post-fix: `[\w.-]+?` captures `02.3-01` → planData.find resolves → correct wave written + const roadmap = [ + '# Roadmap', + '', + '### Phase 02.3: Surgical edit ops', + '', + 'Plans: 2 plans across 2 waves', + '- [ ] 02.3-01-PLAN.md — Path resolver', + '- [ ] 02.3-02-PLAN.md — Op handlers', + '', + ].join('\n'); + + tmpDir = makePlanProject({ + '.planning/ROADMAP.md': roadmap, + '.planning/phases/02.3-surgical-edit-ops/02.3-01-PLAN.md': makePlan({ phase: '02.3', plan: '02.3-01', wave: 1 }), + '.planning/phases/02.3-surgical-edit-ops/02.3-02-PLAN.md': makePlan({ phase: '02.3', plan: '02.3-02', wave: 2, dependsOn: ['02.3-01'] }), + }); + + const result = runGsdTools('roadmap annotate-dependencies 02.3', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, true, + 'decimal-phase ROADMAP with inline Plans: summary must be annotated'); + assert.strictEqual(out.waves, 2, + 'two distinct waves must be identified (02.3-01→wave 1, 02.3-02→wave 2)'); + + const written = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + assert.ok(/Wave 1/.test(written), 'Wave 1 header must appear in output'); + assert.ok(/Wave 2/.test(written), 'Wave 2 header must appear in output'); + }); + + test('combined fixture: decimal phase + bold Plans: header (both bugs together)', (t) => { + // Exercises Bug 1 (bold **Plans:** header) AND Bug 3 (decimal IDs) simultaneously. + // This is the exact ROADMAP fragment from the issue report. + const roadmap = [ + '# Roadmap', + '', + '## Milestone v1.2', + '', + '### Phase 02.3: Surgical edit ops', + '', + '**Plans:** 3 plans across 2 waves', + '- [ ] 02.3-01-PLAN.md — Path resolver', + '- [ ] 02.3-02-PLAN.md — Op handlers', + '- [ ] 02.3-03-PLAN.md — Tests', + '', + ].join('\n'); + + tmpDir = makePlanProject({ + '.planning/ROADMAP.md': roadmap, + '.planning/phases/02.3-surgical-edit-ops/02.3-01-PLAN.md': makePlan({ phase: '02.3', plan: '02.3-01', wave: 1 }), + '.planning/phases/02.3-surgical-edit-ops/02.3-02-PLAN.md': makePlan({ phase: '02.3', plan: '02.3-02', wave: 1 }), + '.planning/phases/02.3-surgical-edit-ops/02.3-03-PLAN.md': makePlan({ phase: '02.3', plan: '02.3-03', wave: 2, dependsOn: ['02.3-01', '02.3-02'] }), + }); + + const result = runGsdTools('roadmap annotate-dependencies 02.3', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, true, + 'combined fixture (bold Plans: + decimal IDs) must produce updated: true'); + assert.strictEqual(out.waves, 2, + 'wave 2 dependency must be detected from decimal plan IDs'); + }); +}); + +// --------------------------------------------------------------------------- +// Bug 2 — Phase boundary: decimal phase headings as section terminators +// --------------------------------------------------------------------------- + +describe('bug #3691 — Bug 2: decimal phase heading used as section boundary', () => { + let tmpDir; + afterEach(() => cleanup(tmpDir)); + + test('adjacent decimal phase heading terminates current phase section', (t) => { + // Pre-fix: `/\n#{2,4}\s+Phase\s+\d/i` uses bare `\d` → doesn't match `### Phase 02.3:` + // → phaseEnd = content.length → section includes all subsequent phases → plans from + // phase 02.3 are mistakenly processed when annotating phase 02.2. + // Post-fix: `/\n#{2,4}\s+Phase\s+\d[\d.]*/i` → matches decimal headings too + // + // Setup: phase 02.2 has 2 plans at different waves (so wave annotation is written). + // phase 02.3 has 1 unannotated plan. We annotate phase 02.2 only. + // Post-fix: phase 02.3 section must remain untouched. + const roadmap = [ + '# Roadmap', + '', + '### Phase 02.2: First phase', + '', + 'Plans: 2 plans', + '- [ ] 02.2-01-PLAN.md — First task', + '- [ ] 02.2-02-PLAN.md — Second task', + '', + '### Phase 02.3: Surgical edit ops', + '', + 'Plans: 2 plans', + '- [ ] 02.3-01-PLAN.md — Path resolver', + '- [ ] 02.3-02-PLAN.md — Op handlers', + '', + ].join('\n'); + + tmpDir = makePlanProject({ + '.planning/ROADMAP.md': roadmap, + '.planning/phases/02.2-first/02.2-01-PLAN.md': makePlan({ phase: '02.2', plan: '02.2-01', wave: 1 }), + '.planning/phases/02.2-first/02.2-02-PLAN.md': makePlan({ phase: '02.2', plan: '02.2-02', wave: 2, dependsOn: ['02.2-01'] }), + '.planning/phases/02.3-surgical-edit-ops/02.3-01-PLAN.md': makePlan({ phase: '02.3', plan: '02.3-01', wave: 1 }), + '.planning/phases/02.3-surgical-edit-ops/02.3-02-PLAN.md': makePlan({ phase: '02.3', plan: '02.3-02', wave: 1 }), + }); + + // Annotate phase 02.2 only + const result = runGsdTools('roadmap annotate-dependencies 02.2', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, true, 'phase 02.2 must be annotated (2 plans across 2 waves)'); + assert.strictEqual(out.waves, 2, 'phase 02.2 must show 2 waves'); + + const written = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + // Wave headers must appear in phase 02.2 section + const phase022Section = written.split('### Phase 02.3:')[0]; + assert.ok(/\*\*Wave/.test(phase022Section), + 'wave headers must be written into the phase 02.2 section'); + // Phase 02.3 section must NOT contain wave headers (boundary must stop at Phase 02.3 heading) + const phase023Section = written.split('### Phase 02.3:')[1] ?? ''; + assert.ok(!/\*\*Wave/.test(phase023Section), + 'phase 02.3 section must not contain wave headers when only 02.2 was annotated'); + }); +}); From 6f00508e889ecf365483fb785b115f34785c9809 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 21 May 2026 00:18:40 -0400 Subject: [PATCH 2/3] changeset: add Fixed entry for #3691 Co-Authored-By: Claude Sonnet 4.6 --- .changeset/vivid-jaguars-tumble.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/vivid-jaguars-tumble.md diff --git a/.changeset/vivid-jaguars-tumble.md b/.changeset/vivid-jaguars-tumble.md new file mode 100644 index 000000000..5c9a3d0f8 --- /dev/null +++ b/.changeset/vivid-jaguars-tumble.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3794 +--- +Annotate-dependencies now matches all Plans-block heading variants in PLAN.md so downstream wave assignment is correct. From 8d1788020a272db35cc50a6c50845c07bbee9b38 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 21 May 2026 20:03:11 -0400 Subject: [PATCH 3/3] =?UTF-8?q?fix(3691):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20drop=20no-op=20Bug=202=20change,=20anchor=20Plans=20regex,?= =?UTF-8?q?=20guard=20leading-dot=20IDs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses gsd-code-reviewer (Bug 2 no-op proven empirically; unanchored Plans regex) and sonnet adversarial (leading-dot silent wave-1 default; multi-decimal + bare-bold test gaps). - F1: Drop "Bug 2" nextPhaseOffset regex change (\d[\d.]* → \d): confirmed no-op by reverting and verifying all 7 existing tests still pass — phase headings always start with a digit so \d already matches decimal phases like 02.3. - F2: Anchor plansBlockMatch to start-of-line via (?:^|\n) prefix so mid-line occurrences like `***Plans:***` in prose or `OpenPlans:` prefixes do not produce false matches. - F3: Add leading-dot plan ID validation guard before planData.find() — malformed IDs that fail /^\w[\w.-]*$/ are skipped rather than silently defaulting to wave 1. - F4: Add adversarial test cases for 001.10-PLAN.md (multi-decimal leading-zero ID) and **Plans:** bare-bold (no trailing text); delete vacuous Bug 2 describe block. Co-Authored-By: Claude Sonnet 4.6 --- get-shit-done/bin/lib/roadmap.cjs | 14 +- ...nnotate-deps-plans-block-variants.test.cjs | 150 +++++++++++++----- 2 files changed, 117 insertions(+), 47 deletions(-) diff --git a/get-shit-done/bin/lib/roadmap.cjs b/get-shit-done/bin/lib/roadmap.cjs index 692530307..441bddb20 100644 --- a/get-shit-done/bin/lib/roadmap.cjs +++ b/get-shit-done/bin/lib/roadmap.cjs @@ -541,9 +541,7 @@ function cmdRoadmapAnnotateDependencies(cwd, phaseNum, raw) { const phaseStart = phaseMatch.index; const restAfterHeader = content.slice(phaseStart); - // #3691 Bug 2: `\d` only matches a single digit, missing decimal phase headings - // like `### Phase 02.3:`. Use `\d[\d.]*` to match integers and decimals alike. - const nextPhaseOffset = restAfterHeader.slice(1).search(/\n#{2,4}\s+Phase\s+\d[\d.]*/i); + const nextPhaseOffset = restAfterHeader.slice(1).search(/\n#{2,4}\s+Phase\s+\d/i); const phaseEnd = nextPhaseOffset >= 0 ? phaseStart + 1 + nextPhaseOffset : content.length; const phaseSection = content.slice(phaseStart, phaseEnd); @@ -562,7 +560,10 @@ function cmdRoadmapAnnotateDependencies(cwd, phaseNum, raw) { // line with no immediately-following checklist items (e.g. a summary line above a // separate bare `Plans:` block) does not consume the match and prevent the actual // list from being found. - const plansBlockMatch = phaseSection.match(/(\*{0,2}Plans\*{0,2}:[^\n]*\n)((?:\s*-\s*\[[ x]\][^\n]*\n?)+)/i); + // 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); if (!plansBlockMatch) return; const plansHeader = plansBlockMatch[1]; @@ -580,6 +581,11 @@ function cmdRoadmapAnnotateDependencies(cwd, phaseNum, raw) { // terminating alternation (`-PLAN.md|.md|:|\s—`) as the boundary anchor. const idMatch = line.match(/\[\s*[x ]\s*\]\s*([\w.-]+?)(?:-PLAN\.md|\.md|:|\s—)/i); const planId = idMatch ? idMatch[1] : null; + // Review fix (F3): reject malformed IDs that start with `.`, contain consecutive + // dots, or otherwise violate the `^\w[\w.-]*$` contract. A leading-dot ID + // (e.g. `.invalid-PLAN.md`) would silently default to wave 1 — defensively + // skip the line instead so corrupted ROADMAP entries don't corrupt wave layout. + if (planId && !/^\w[\w.-]*$/.test(planId)) continue; const planEntry = planId ? planData.find(p => p.planId === planId) : null; const wave = planEntry ? planEntry.wave : 1; if (!linesByWave.has(wave)) linesByWave.set(wave, []); diff --git a/tests/bug-3691-annotate-deps-plans-block-variants.test.cjs b/tests/bug-3691-annotate-deps-plans-block-variants.test.cjs index c1e856213..f6b48e1a5 100644 --- a/tests/bug-3691-annotate-deps-plans-block-variants.test.cjs +++ b/tests/bug-3691-annotate-deps-plans-block-variants.test.cjs @@ -7,20 +7,29 @@ /** * Regression — issue #3691 * - * Three regex defects in `roadmap.cjs` function `cmdRoadmapAnnotateDependencies`: + * Two regex defects fixed in `roadmap.cjs` function `cmdRoadmapAnnotateDependencies`, + * plus two review-cycle additions (F3 defensive guard, F4 adversarial gaps): * * Bug 1 (line ~553) — Plans-block detection regex `/(Plans:\s*\n)/i` requires no text * after the colon. Headers like `Plans: 3 plans across 2 waves\n` or * `**Plans:** 3 plans\n` are silently skipped and the function early-returns. - * - * Bug 2 (line ~542) — Phase-section boundary regex `/\n#{2,4}\s+Phase\s+\d/i` uses - * `\d` (single digit) so decimal phase headings like `### Phase 02.3:` are not - * recognized as boundaries. Content from an adjacent decimal phase may bleed into - * the section being annotated. + * Fix: `(?:^|\n)(\*{0,2}Plans\*{0,2}:[^\n]*\n)` anchors to start-of-line and + * accepts optional bold wrappers and any trailing text on the header line. * * Bug 3 (line ~566) — Plan-ID extraction regex `/([\w-]+?)/` excludes `.`, so * decimal plan IDs like `02.3-01` are captured as `02` only, never match * the planData entry, and every plan defaults to wave 1. + * Fix: `[\w.-]+?` includes `.` so decimal IDs are captured in full. + * + * Note: "Bug 2" (phase-section boundary `\d` → `\d[\d.]*`) was confirmed empirically + * to be a no-op: any phase heading starts with a digit, so `\d` already matches. + * The no-op change was dropped from the PR; this file has no Bug 2 describe block. + * + * Review additions: + * F3 — Leading-dot plan ID guard: malformed IDs like `.invalid` are rejected + * before planData.find() rather than silently defaulting to wave 1. + * F4 — Adversarial gaps: multi-decimal leading-zero IDs (001.10-PLAN.md) and + * bare-bold `**Plans:**` (no trailing text) are explicitly covered. */ const { test, describe, afterEach } = require('node:test'); @@ -264,62 +273,117 @@ describe('bug #3691 — Bug 3: decimal plan IDs (e.g. 02.3-01-PLAN.md) parse cor }); // --------------------------------------------------------------------------- -// Bug 2 — Phase boundary: decimal phase headings as section terminators +// F3 review fix — Leading-dot ID validation guard (defensive, malformed ROADMAP) // --------------------------------------------------------------------------- -describe('bug #3691 — Bug 2: decimal phase heading used as section boundary', () => { +describe('review fix F3 — leading-dot plan ID is rejected (defensive guard)', () => { let tmpDir; afterEach(() => cleanup(tmpDir)); - test('adjacent decimal phase heading terminates current phase section', (t) => { - // Pre-fix: `/\n#{2,4}\s+Phase\s+\d/i` uses bare `\d` → doesn't match `### Phase 02.3:` - // → phaseEnd = content.length → section includes all subsequent phases → plans from - // phase 02.3 are mistakenly processed when annotating phase 02.2. - // Post-fix: `/\n#{2,4}\s+Phase\s+\d[\d.]*/i` → matches decimal headings too - // - // Setup: phase 02.2 has 2 plans at different waves (so wave annotation is written). - // phase 02.3 has 1 unannotated plan. We annotate phase 02.2 only. - // Post-fix: phase 02.3 section must remain untouched. + test('checklist line with leading-dot plan ID is skipped and does not silently default to wave 1', (t) => { + // Guards: `.invalid-PLAN.md` would be captured as `.invalid` by the `[\w.-]+?` regex + // (since `.` is now included), which starts with a dot — an invalid ID. + // Without the guard, planData.find() misses it and wave defaults to 1, silently + // polluting the output. With the guard, the line is skipped entirely. + // We verify this by having TWO real plans with known waves (1 and 2), plus one + // malformed line. The malformed line must not appear in the wave-annotated output. const roadmap = [ '# Roadmap', '', - '### Phase 02.2: First phase', + '### Phase 1: Foundation', '', - 'Plans: 2 plans', - '- [ ] 02.2-01-PLAN.md — First task', - '- [ ] 02.2-02-PLAN.md — Second task', - '', - '### Phase 02.3: Surgical edit ops', - '', - 'Plans: 2 plans', - '- [ ] 02.3-01-PLAN.md — Path resolver', - '- [ ] 02.3-02-PLAN.md — Op handlers', + 'Plans: 3 items', + '- [ ] 01-01-PLAN.md — Task A', + '- [ ] .invalid-PLAN.md — Corrupted entry', + '- [ ] 01-02-PLAN.md — Task B', '', ].join('\n'); tmpDir = makePlanProject({ '.planning/ROADMAP.md': roadmap, - '.planning/phases/02.2-first/02.2-01-PLAN.md': makePlan({ phase: '02.2', plan: '02.2-01', wave: 1 }), - '.planning/phases/02.2-first/02.2-02-PLAN.md': makePlan({ phase: '02.2', plan: '02.2-02', wave: 2, dependsOn: ['02.2-01'] }), - '.planning/phases/02.3-surgical-edit-ops/02.3-01-PLAN.md': makePlan({ phase: '02.3', plan: '02.3-01', wave: 1 }), - '.planning/phases/02.3-surgical-edit-ops/02.3-02-PLAN.md': makePlan({ phase: '02.3', plan: '02.3-02', wave: 1 }), + '.planning/phases/01-foundation/01-01-PLAN.md': makePlan({ phase: '1', plan: '01-01', wave: 1 }), + '.planning/phases/01-foundation/01-02-PLAN.md': makePlan({ phase: '1', plan: '01-02', wave: 2, dependsOn: ['01-01'] }), }); - // Annotate phase 02.2 only - const result = runGsdTools('roadmap annotate-dependencies 02.2', tmpDir); + const result = runGsdTools('roadmap annotate-dependencies 1', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const out = JSON.parse(result.output); - assert.strictEqual(out.updated, true, 'phase 02.2 must be annotated (2 plans across 2 waves)'); - assert.strictEqual(out.waves, 2, 'phase 02.2 must show 2 waves'); + // Two valid plans resolve to 2 waves — annotation must still proceed + assert.strictEqual(out.updated, true, 'annotation must proceed despite malformed line'); + assert.strictEqual(out.waves, 2, 'two distinct waves from the two valid plans'); const written = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); - // Wave headers must appear in phase 02.2 section - const phase022Section = written.split('### Phase 02.3:')[0]; - assert.ok(/\*\*Wave/.test(phase022Section), - 'wave headers must be written into the phase 02.2 section'); - // Phase 02.3 section must NOT contain wave headers (boundary must stop at Phase 02.3 heading) - const phase023Section = written.split('### Phase 02.3:')[1] ?? ''; - assert.ok(!/\*\*Wave/.test(phase023Section), - 'phase 02.3 section must not contain wave headers when only 02.2 was annotated'); + // The malformed line should not appear in the written output (it was skipped) + assert.ok(!written.includes('.invalid-PLAN.md'), + 'malformed leading-dot entry must be dropped from the annotated output'); + }); +}); + +// --------------------------------------------------------------------------- +// F4 review additions — adversarial test gaps +// --------------------------------------------------------------------------- + +describe('review fix F4 — adversarial test gaps', () => { + let tmpDir; + afterEach(() => cleanup(tmpDir)); + + test('001.10-PLAN.md multi-decimal leading-zero ID is captured fully and wave-assigned correctly', (t) => { + // Guards regression of Bug 3: `[\w-]+?` would stop at the first `.` and + // capture `001` instead of `001.10`, which never matches any planData entry. + // Post-fix `[\w.-]+?` must capture `001.10` in full. + const roadmap = [ + '# Roadmap', + '', + '### Phase 001.10: Extended decimal phase', + '', + 'Plans: 2 plans across 2 waves', + '- [ ] 001.10-01-PLAN.md — First task', + '- [ ] 001.10-02-PLAN.md — Second task', + '', + ].join('\n'); + + tmpDir = makePlanProject({ + '.planning/ROADMAP.md': roadmap, + '.planning/phases/001.10-extended/001.10-01-PLAN.md': makePlan({ phase: '001.10', plan: '001.10-01', wave: 1 }), + '.planning/phases/001.10-extended/001.10-02-PLAN.md': makePlan({ phase: '001.10', plan: '001.10-02', wave: 2, dependsOn: ['001.10-01'] }), + }); + + const result = runGsdTools('roadmap annotate-dependencies 001.10', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, true, + 'multi-decimal leading-zero plan ID must be captured fully and annotated'); + assert.strictEqual(out.waves, 2, + 'wave 2 dependency must be resolved from full 001.10-02 ID (not truncated to 001)'); + }); + + test('**Plans:** (bold, no trailing text) is matched and checklist is processed', (t) => { + // Guards the bare-bold variant: `**Plans:**` with nothing after the colon. + // The `[^\n]*` quantifier accepts zero chars so this should already work, + // but this test would fail if `\*{0,2}Plans\*{0,2}` regressed to require no stars. + const roadmap = [ + '# Roadmap', + '', + '### Phase 1: Foundation', + '', + '**Plans:**', + '- [ ] 01-01-PLAN.md — Task A', + '- [ ] 01-02-PLAN.md — Task B', + '', + ].join('\n'); + + tmpDir = makePlanProject({ + '.planning/ROADMAP.md': roadmap, + '.planning/phases/01-foundation/01-01-PLAN.md': makePlan({ phase: '1', plan: '01-01', wave: 1 }), + '.planning/phases/01-foundation/01-02-PLAN.md': makePlan({ phase: '1', plan: '01-02', wave: 2, dependsOn: ['01-01'] }), + }); + + const result = runGsdTools('roadmap annotate-dependencies 1', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, true, + '**Plans:** bare-bold variant (no trailing text) must be detected and annotated'); + assert.strictEqual(out.waves, 2, + 'wave assignment must resolve correctly from planData for bare-bold variant'); }); });