From f77179248d6c80b9fe82e1c035a73ad55444b929 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 5 Jul 2026 17:39:11 -0400 Subject: [PATCH] fix(#2012): scope phase.complete Progress-row regex to ## Progress section (#2032) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2012): scope Progress-row regex to ## Progress section (was binding to earlier table) The Progress-row writer used a non-global regex that matched ANY table row starting with the phase number. When an earlier table (e.g. Requirements coverage | Phase | Requirements | Count |) preceded ## Progress, the regex bound to the wrong row (3-column), no-op'd, and never reached the real Progress row. roadmap_updated stayed true (it's existsSync), masking the failure. - src/phase.cts: scope the tableRowPattern regex to the ## Progress section (indexOf + slice) so it only matches Progress-table rows. - tests/phase.test.cjs: regression test — ROADMAP with a phase-numbered Requirements table before ## Progress → Progress row updated, Requirements row untouched. Closes #2012 * docs(#2012): backfill changeset pr 2032 --- .../2012-phase-complete-progress-row.md | 5 ++ src/phase.cts | 15 +++++- tests/phase.test.cjs | 47 +++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 .changeset/2012-phase-complete-progress-row.md diff --git a/.changeset/2012-phase-complete-progress-row.md b/.changeset/2012-phase-complete-progress-row.md new file mode 100644 index 000000000..3bc84044c --- /dev/null +++ b/.changeset/2012-phase-complete-progress-row.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2032 +--- +**`phase.complete` now updates the `## Progress` rollup row even when an earlier phase-numbered table precedes it** — the Progress-row writer used a non-global regex that matched *any* table row starting with the phase number, so it bound to the first such row (e.g. a `| Phase | Requirements | Count |` coverage table), no-op'd on the wrong 3-column row, and never reached the real Progress row. The regex is now scoped to the `## Progress` section so it binds to the correct table. The command still returned `roadmap_updated: true` (that field is `fs.existsSync(ROADMAP.md)`), masking the silent failure. (#2012) diff --git a/src/phase.cts b/src/phase.cts index 6dca595d1..f2ca162e4 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -1460,7 +1460,10 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { `^(\\|\\s*${phaseEscaped}\\.?\\s[^|]*(?:\\|[^\\n]*))$`, 'im', ); - roadmapContent = roadmapContent.replace(tableRowPattern, (fullRow) => { + // Scope the Progress-row search to the ## Progress section so the regex + // doesn't bind to an earlier table (e.g. | Phase | Requirements | Count |) + // whose rows also start with the phase number. (#2012) + const updateProgressRow = (fullRow: string): string => { const cells = fullRow.split('|').slice(1, -1); const dateShape = /^\d{4}-\d{2}-\d{2}$/; if (cells.length === 5) { @@ -1477,7 +1480,15 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { cells[3] = dateShape.test(existingDate4) ? cells[3] : ` ${today} `; } return '|' + cells.join('|') + '|'; - }); + }; + const progressIdx = roadmapContent.indexOf('## Progress'); + if (progressIdx >= 0) { + const beforeProgress = roadmapContent.slice(0, progressIdx); + const progressSection = roadmapContent.slice(progressIdx); + roadmapContent = beforeProgress + progressSection.replace(tableRowPattern, updateProgressRow); + } else { + roadmapContent = roadmapContent.replace(tableRowPattern, updateProgressRow); + } const planCountPattern = new RegExp( `(#{2,4}\\s*Phase\\s+${phaseEscaped}[\\s\\S]*?\\*\\*Plans:\\*\\*\\s*)[^\\n]+`, diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 1dad00845..6681c40cd 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -2530,6 +2530,53 @@ describe('phase complete command', () => { assert.ok(roadmap.includes('completed'), 'completion date should be added'); }); + test('#2012 — Progress row updated even when an earlier phase-numbered table precedes ## Progress', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +- [ ] Phase 3: Build + +### Phase 3: Build +**Goal:** Build stuff + +## Requirements Coverage + +| Phase | Requirements | Count | +|-------|-------------|-------| +| 3. Build | R-01 | 5 | + +## Progress + +| Phase | Milestone | Plans Complete | Status | Completed | +|-------|-----------|----------------|--------|-----------| +| 3. Build | v1.0 | 0/1 | Planned | - | +` + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# State\n\n**Current Phase:** 03\n**Status:** In progress\n**Last Activity:** 2025-01-01\n` + ); + + const p3 = path.join(tmpDir, '.planning', 'phases', '03-build'); + fs.mkdirSync(p3, { recursive: true }); + fs.writeFileSync(path.join(p3, '03-01-PLAN.md'), '# Plan'); + fs.writeFileSync(path.join(p3, '03-01-SUMMARY.md'), '# Summary'); + + const result = runVerifiedPhaseComplete('phase complete 3', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + + // The Requirements coverage table row must be UNCHANGED (3 columns, not a Progress row). + const reqRow = roadmap.match(/^\| 3\. Build \| R-01 \| 5 \|$/m); + assert.ok(reqRow, 'Requirements coverage row must be untouched'); + + // The Progress row must be updated to Complete with a date. + const progressRow = roadmap.match(/^\| 3\. Build \| v1\.0 \| 1\/1 \| Complete\s+\| \d{4}-\d{2}-\d{2} \|/m); + assert.ok(progressRow, 'Progress row must be updated to Complete with a date'); + }); + test('detects last phase in milestone', () => { fs.writeFileSync( path.join(tmpDir, '.planning', 'ROADMAP.md'),