diff --git a/.changeset/calm-deer-run.md b/.changeset/calm-deer-run.md new file mode 100644 index 000000000..36db54de9 --- /dev/null +++ b/.changeset/calm-deer-run.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2916 +--- +**roadmap.update-plan-progress no longer deletes hand-written annotations** — bumping the plan count used to swallow the rest of the Plans line, silently deleting any prose a human wrote after the count. The verb now replaces only the count token and leaves trailing text intact. (#2853) diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index f4843ddaa..e80643d59 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -194,15 +194,25 @@ function extractCurrentMilestone(content: string, cwd?: string): string { /** * Replace a pattern only in the current milestone section of ROADMAP.md. */ -function replaceInCurrentMilestone(content: string, pattern: RegExp, replacement: string): string { +type RoadmapReplacer = (match: string, ...captures: string[]) => string; + +function replaceInCurrentMilestone( + content: string, + pattern: RegExp, + replacement: string | RoadmapReplacer, +): string { + const apply = (src: string): string => + typeof replacement === 'function' + ? src.replace(pattern, replacement) + : src.replace(pattern, replacement); const lastDetailsClose = content.lastIndexOf(''); if (lastDetailsClose === -1) { - return content.replace(pattern, replacement); + return apply(content); } const offset = lastDetailsClose + ''.length; const before = content.slice(0, offset); const after = content.slice(offset); - return before + after.replace(pattern, replacement); + return before + apply(after); } /** diff --git a/src/roadmap.cts b/src/roadmap.cts index 310161756..1c8d4f5f8 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -615,14 +615,31 @@ function cmdRoadmapUpdatePlanProgress(cwd: string, phaseNum: string | null | und // `**Plans**: N plans` — bold word + outer colon (gsd-core/templates/roadmap.md) // `**Plans:** N plans` — bold "Plans:" (colon inside bold) // `Plans: N plans` — plain text header + // + // #2853: the verb owns the count token ONLY — it must not destroy hand-written + // prose a human placed after the count (e.g. "(11-16 are gap closure ...)"). + // Group $1 = phase header → `Plans:` label + trailing whitespace (unchanged). + // Group $2 = the existing count token to replace: matches `N/N plans complete`, + // `N/N plans executed`, or the bare template `N plans` form. Group $3 = whatever + // else is on the line (`[^\r\n]*`, so CRLF `\r` is preserved). + // + // Trailing text is preserved ONLY when a real count token ($2) was present — + // i.e. an annotation a human wrote after a real count. When $2 is absent the + // line is the fresh-template bracketed placeholder (`[Number of plans…]`) or + // other freeform guidance, not user prose: the count replaces the whole token, + // preserving the pre-#2853 clean-output behaviour on the template path. const planCountPattern = new RegExp( - `(#{2,4}\\s*Phase\\s+${phasePattern}${OPTIONAL_PHASE_TAG_SOURCE}(?=[:\\s])(?:(?!\\n#{1,4}\\s)[\\s\\S])*?(?:\\*\\*Plans\\*\\*:|\\*\\*Plans:\\*\\*|(?:^|\\n)Plans:)\\s*)[^\\n]+`, + `(#{2,4}\\s*Phase\\s+${phasePattern}${OPTIONAL_PHASE_TAG_SOURCE}(?=[:\\s])(?:(?!\\n#{1,4}\\s)[\\s\\S])*?(?:\\*\\*Plans\\*\\*:|\\*\\*Plans:\\*\\*|(?:^|\\n)Plans:)\\s*)(\\d+\\s*\\/\\s*\\d+\\s+plans(?:\\s+(?:complete|executed))?|\\d+\\s+plans)?([^\\r\\n]*)`, 'i' ); const planCountText = isComplete ? `${summaryCount}/${planCount} plans complete` : `${summaryCount}/${planCount} plans executed`; - roadmapContent = replaceInCurrentMilestone(roadmapContent, planCountPattern, `$1${planCountText}`); + roadmapContent = replaceInCurrentMilestone(roadmapContent, planCountPattern, (_match, label, existingCount, trailing) => { + // Preserve trailing text only when a real count preceded it. + const suffix = existingCount ? trailing : ''; + return `${label}${planCountText}${suffix}`; + }); // If complete: check checkbox if (isComplete) { diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index bbb81cb5b..97d85c092 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -7541,6 +7541,274 @@ describe('bug #3537: phase verbs accept padded ids against un-padded ROADMAP pro } +// ──────────────────────────────────────────────────────────────────────── +// Regression: bug #2853 — roadmap.update-plan-progress deletes hand-written +// annotations after the plan count. The count-bump regex's trailing `[^\n]+` +// swallowed the whole line and the replacement wrote back only the regenerated +// count, truncating any human prose after it. +// ──────────────────────────────────────────────────────────────────────── +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe("folded:bug-2853-plan-progress-annotation-preservation", () => { +'use strict'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); +const { execFileSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); + +const gsdTools2853 = path.resolve(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); + +function run2853(args, cwd) { + try { + return { + stdout: execFileSync('node', [gsdTools2853, ...args], { + cwd, timeout: 15000, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], + }), + ok: true, + }; + } catch (e) { + return { + stdout: (e.stdout && e.stdout.toString()) || '', + stderr: (e.stderr && e.stderr.toString()) || '', + ok: false, code: e.status, + }; + } +} + +/** + * Build a phase-10 fixture with one plan + matching summary + verification + * `passed`, so the phase is "complete" for update-plan-progress. The ROADMAP + * `Plans` summary line is supplied verbatim so each test pins a specific shape. + */ +function setupFixture2853(tmpDir, plansSummaryLine, opts = {}) { + const { incomplete = false, eol = '\n' } = opts; + const planningDir = path.join(tmpDir, '.planning'); + const phaseDir = path.join(planningDir, 'phases', 'CK-10-test-phase'); + fs.mkdirSync(phaseDir, { recursive: true }); + + fs.writeFileSync(path.join(planningDir, 'config.json'), JSON.stringify({ project_code: 'CK' })); + fs.writeFileSync( + path.join(planningDir, 'STATE.md'), + `---\ncurrent_phase: 10\nstatus: executing\n---\n# State\n` + ); + + fs.writeFileSync( + path.join(phaseDir, '10-01-PLAN.md'), + `---\nphase: 10\nplan: 1\nwave: 1\n---\n# Plan 1\n` + ); + fs.writeFileSync( + path.join(phaseDir, '10-01-SUMMARY.md'), + '---\nstatus: complete\n---\n# Summary\nDone.\n' + ); + fs.writeFileSync( + path.join(phaseDir, '10-VERIFICATION.md'), + incomplete + ? '---\nstatus: pending\n---\n# Verification\nPending.\n' + : '---\nstatus: passed\nscore: "1/1"\n---\n# Verification\nPassed.\n' + ); + + const roadmap = [ + '# Roadmap', + '', + '## v1.0 Milestone', + '', + '- [ ] **Phase 10: Test Phase**', + '', + '## Progress', + '', + '| Phase | Plans | Status | Completed |', + '|-------|-------|--------|-----------|', + '| 10 Test Phase | 0/1 | Planned | - |', + '', + '### Phase 10: Test Phase', + '', + '**Goal:** reproduce', + plansSummaryLine, + '', + 'Plans:', + '- [ ] 10-01-PLAN.md', + '', + ].join('\n') + '\n'; + + fs.writeFileSync(path.join(planningDir, 'ROADMAP.md'), roadmap.replace(/\n/g, eol)); + + return { planningDir, roadmapPath: path.join(planningDir, 'ROADMAP.md') }; +} + +describe('bug #2853: update-plan-progress preserves hand-written annotations', () => { + test('row 1 — bold colon-outside form: annotation after count survives a complete bump', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2853-r1-')); + try { + const annotation = '(11-16 are gap closure from VERIFICATION)'; + const { roadmapPath } = setupFixture2853( + tmp, + `**Plans**: 0/1 plans executed ${annotation}` + ); + run2853(['roadmap', 'update-plan-progress', '10'], tmp); + + const result = fs.readFileSync(roadmapPath, 'utf-8'); + const line = result.split(/\r?\n/).find((l) => l.includes('**Plans**')); + assert.ok(line, 'Plans summary line must exist'); + assert.ok( + line.includes(annotation), + `annotation must survive; got: ${line}` + ); + assert.match(line, /1\/1 plans complete/, 'count must still bump to complete'); + } finally { + cleanup(tmp); + } + }); + + test('row 2 — **Plans:** colon-inside-bold form: annotation survives', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2853-r2-')); + try { + const annotation = '(gap closure wave)'; + const { roadmapPath } = setupFixture2853( + tmp, + `**Plans:** 0/1 plans executed ${annotation}` + ); + run2853(['roadmap', 'update-plan-progress', '10'], tmp); + const line = fs.readFileSync(roadmapPath, 'utf-8').split(/\r?\n/).find((l) => l.includes('**Plans:**')); + assert.ok(line && line.includes(annotation), `annotation must survive; got: ${line}`); + } finally { + cleanup(tmp); + } + }); + + test('row 3 — plain Plans: header form: annotation survives', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2853-r3-')); + try { + const annotation = '(see verification notes)'; + const { roadmapPath } = setupFixture2853(tmp, `Plans: 0/1 plans executed ${annotation}`); + run2853(['roadmap', 'update-plan-progress', '10'], tmp); + const result = fs.readFileSync(roadmapPath, 'utf-8'); + const line = result.split(/\r?\n/).find((l) => /^Plans:/.test(l)); + assert.ok(line && line.includes(annotation), `annotation must survive; got: ${line}`); + } finally { + cleanup(tmp); + } + }); + + test('row 4 — no trailing text: count updates with no whitespace regression', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2853-r4-')); + try { + const { roadmapPath } = setupFixture2853(tmp, '**Plans:** 0/1 plans executed'); + run2853(['roadmap', 'update-plan-progress', '10'], tmp); + const line = fs.readFileSync(roadmapPath, 'utf-8').split(/\r?\n/).find((l) => l.includes('**Plans:**')); + assert.ok(line, 'Plans line must exist'); + // Exact line: count bumped, no stray trailing space or duplicated text. + assert.equal(line, '**Plans:** 1/1 plans complete'); + } finally { + cleanup(tmp); + } + }); + + test('row 5 — bare template form `N plans` (no slash) still gets count inserted', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2853-r5-')); + try { + const { roadmapPath } = setupFixture2853(tmp, '**Plans:** 0 plans'); + run2853(['roadmap', 'update-plan-progress', '10'], tmp); + const line = fs.readFileSync(roadmapPath, 'utf-8').split(/\r?\n/).find((l) => l.includes('**Plans:**')); + assert.ok(line, 'Plans line must exist'); + assert.match(line, /1\/1 plans complete/, 'bare form must still receive the canonical count'); + } finally { + cleanup(tmp); + } + }); + + test('row 6 — executed (non-complete) path: annotation survives', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2853-r6-')); + try { + const annotation = '(half-done, gap closure pending)'; + const { roadmapPath } = setupFixture2853( + tmp, + `**Plans:** 0/1 plans executed ${annotation}`, + { incomplete: true } + ); + run2853(['roadmap', 'update-plan-progress', '10'], tmp); + const line = fs.readFileSync(roadmapPath, 'utf-8').split(/\r?\n/).find((l) => l.includes('**Plans:**')); + assert.ok(line, 'Plans line must exist'); + assert.ok(line.includes(annotation), `annotation must survive on executed path; got: ${line}`); + assert.match(line, /1\/1 plans executed/, 'count must reflect executed (not complete) path'); + } finally { + cleanup(tmp); + } + }); + + test('row 7 — CRLF input: annotation text preserved (line-ending normalization is pre-existing)', () => { + // The verb has always normalized ROADMAP.md line endings on its read-modify-write + // (a pre-existing trait, not #2853's concern). What #2853 guarantees is that the + // annotation TEXT survives the count bump on whatever line ending the verb emits. + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2853-r7-')); + try { + const annotation = '(CRLF note)'; + const { roadmapPath } = setupFixture2853( + tmp, + `**Plans:** 0/1 plans executed ${annotation}`, + { eol: '\r\n' } + ); + run2853(['roadmap', 'update-plan-progress', '10'], tmp); + const result = fs.readFileSync(roadmapPath, 'utf-8'); + const plansLine = result.split(/\r?\n/).find((l) => l.includes('**Plans:**')); + assert.ok(plansLine, 'Plans summary line must exist'); + assert.ok( + plansLine.includes(annotation), + `annotation text must survive on a CRLF-origin file; got: ${plansLine}` + ); + assert.match(plansLine, /1\/1 plans complete/, 'count must still bump'); + } finally { + cleanup(tmp); + } + }); + + test('row 8 — idempotent re-run does not drop annotation', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2853-r8-')); + try { + const annotation = '(idempotent check)'; + const { roadmapPath } = setupFixture2853( + tmp, + `**Plans:** 0/1 plans executed ${annotation}` + ); + run2853(['roadmap', 'update-plan-progress', '10'], tmp); + run2853(['roadmap', 'update-plan-progress', '10'], tmp); + const line = fs.readFileSync(roadmapPath, 'utf-8').split(/\r?\n/).find((l) => l.includes('**Plans:**')); + assert.ok(line && line.includes(annotation), `annotation must survive a no-op re-run; got: ${line}`); + } finally { + cleanup(tmp); + } + }); + + test('row 9 — fresh-template placeholder is replaced, not glued to the new count', () => { + // The canonical template ships `**Plans**: [Number of plans, e.g., "3 plans" or "TBD"]`. + // A count bump must NOT preserve that bracketed guidance verbatim glued after the + // count (pre-#2853 produced a clean `N/N plans complete`). The count replaces the + // placeholder because there is no real count token for $2 to anchor a trailing-text + // preservation to. + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2853-r9-')); + try { + const placeholder = '[Number of plans, e.g., "3 plans" or "TBD"]'; + const { roadmapPath } = setupFixture2853(tmp, `**Plans**: ${placeholder}`); + run2853(['roadmap', 'update-plan-progress', '10'], tmp); + const line = fs.readFileSync(roadmapPath, 'utf-8').split(/\r?\n/).find((l) => l.includes('**Plans**')); + assert.ok(line, 'Plans line must exist'); + assert.equal( + line, + '**Plans**: 1/1 plans complete', + `placeholder must be replaced cleanly, not glued; got: ${line}` + ); + } finally { + cleanup(tmp); + } + }); +}); + }); +} + + // ──────────────────────────────────────────────────────────────────────── // Folded from tests/bug-2268-parallel-discuss.test.cjs — consolidation epic #1969 (B2 #1971) // ────────────────────────────────────────────────────────────────────────