fix(core): replace negative-heuristic stripShippedMilestones with positive milestone lookup (#1145) (#1146)
stripShippedMilestones() uses a negative heuristic: strip all <details> blocks, assume what remains is the current milestone. This breaks when agents accidentally wrap the current milestone in <details> for collapsibility — all downstream consumers then see an empty milestone. Observed failure: cmdPhaseComplete() returns is_last_phase: true and next_phase: null for non-final phases because the current milestone's phases were stripped along with shipped ones. Added extractCurrentMilestone(content, cwd) — a positive lookup that: 1. Reads the current milestone version from STATE.md frontmatter 2. Falls back to 🚧 in-progress marker in ROADMAP.md 3. Finds the section heading matching that version 4. Returns only that section's content 5. Falls back to stripShippedMilestones() if version can't be determined Updated 12 call sites across 6 files to use extractCurrentMilestone: - core.cjs: getRoadmapPhaseInternal(), getMilestonePhaseFilter() - phase.cjs: cmdPhaseAdd(), cmdPhaseInsert(), cmdPhaseComplete() (2 sites) - roadmap.cjs: cmdRoadmapGetPhase(), cmdRoadmapAnalyze() - commands.cjs: stats/progress display - verify.cjs: phase verification (2 sites) - init.cjs: project initialization Kept stripShippedMilestones() for: - getMilestoneInfo() — determines the version itself, can't use positive lookup - replaceInCurrentMilestone() — write operations, conservative boundary - extractCurrentMilestone() fallback — when no version available All 755 tests pass. Fixes #1145
This commit is contained in:
Tom Boucher
@@ -4,7 +4,7 @@
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { execSync } = require('child_process');
|
||||
const { safeReadFile, loadConfig, isGitIgnored, execGit, normalizePhaseName, comparePhaseNum, getArchivedPhaseDirs, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, resolveModelInternal, stripShippedMilestones, toPosixPath, output, error, findPhaseInternal } = require('./core.cjs');
|
||||
const { safeReadFile, loadConfig, isGitIgnored, execGit, normalizePhaseName, comparePhaseNum, getArchivedPhaseDirs, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, resolveModelInternal, stripShippedMilestones, extractCurrentMilestone, toPosixPath, output, error, findPhaseInternal } = require('./core.cjs');
|
||||
const { extractFrontmatter } = require('./frontmatter.cjs');
|
||||
const { MODEL_PROFILES } = require('./model-profiles.cjs');
|
||||
|
||||
@@ -547,7 +547,7 @@ function cmdStats(cwd, format, raw) {
|
||||
let totalSummaries = 0;
|
||||
|
||||
try {
|
||||
const roadmapContent = stripShippedMilestones(fs.readFileSync(roadmapPath, 'utf-8'));
|
||||
const roadmapContent = extractCurrentMilestone(fs.readFileSync(roadmapPath, 'utf-8'), cwd);
|
||||
const headingPattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:\s*([^\n]+)/gi;
|
||||
let match;
|
||||
while ((match = headingPattern.exec(roadmapContent)) !== null) {
|
||||
|
||||
@@ -420,6 +420,91 @@ function stripShippedMilestones(content) {
|
||||
return content.replace(/<details>[\s\S]*?<\/details>/gi, '');
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the current milestone section from ROADMAP.md by positive lookup.
|
||||
*
|
||||
* Instead of stripping <details> blocks (negative heuristic that breaks if
|
||||
* agents wrap the current milestone in <details>), this finds the section
|
||||
* matching the current milestone version and returns only that content.
|
||||
*
|
||||
* Falls back to stripShippedMilestones() if:
|
||||
* - cwd is not provided
|
||||
* - STATE.md doesn't exist or has no milestone field
|
||||
* - Version can't be found in ROADMAP.md
|
||||
*
|
||||
* @param {string} content - Full ROADMAP.md content
|
||||
* @param {string} [cwd] - Working directory for reading STATE.md
|
||||
* @returns {string} Content scoped to current milestone
|
||||
*/
|
||||
function extractCurrentMilestone(content, cwd) {
|
||||
if (!cwd) return stripShippedMilestones(content);
|
||||
|
||||
// 1. Get current milestone version from STATE.md frontmatter
|
||||
let version = null;
|
||||
try {
|
||||
const statePath = path.join(cwd, '.planning', 'STATE.md');
|
||||
if (fs.existsSync(statePath)) {
|
||||
const stateRaw = fs.readFileSync(statePath, 'utf-8');
|
||||
const milestoneMatch = stateRaw.match(/^milestone:\s*(.+)/m);
|
||||
if (milestoneMatch) {
|
||||
version = milestoneMatch[1].trim();
|
||||
}
|
||||
}
|
||||
} catch {}
|
||||
|
||||
// 2. Fallback: derive version from getMilestoneInfo pattern in ROADMAP.md itself
|
||||
if (!version) {
|
||||
// Check for 🚧 in-progress marker
|
||||
const inProgressMatch = content.match(/🚧\s*\*\*v(\d+\.\d+)\s/);
|
||||
if (inProgressMatch) {
|
||||
version = 'v' + inProgressMatch[1];
|
||||
}
|
||||
}
|
||||
|
||||
if (!version) return stripShippedMilestones(content);
|
||||
|
||||
// 3. Find the section matching this version
|
||||
// Match headings like: ## Roadmap v3.0: Name, ## v3.0 Name, etc.
|
||||
const escapedVersion = escapeRegex(version);
|
||||
const sectionPattern = new RegExp(
|
||||
`(^#{1,3}\\s+.*${escapedVersion}[^\\n]*)`,
|
||||
'mi'
|
||||
);
|
||||
const sectionMatch = content.match(sectionPattern);
|
||||
|
||||
if (!sectionMatch) return stripShippedMilestones(content);
|
||||
|
||||
const sectionStart = sectionMatch.index;
|
||||
|
||||
// Find the end: next milestone heading at same or higher level, or EOF
|
||||
// Milestone headings look like: ## v2.0, ## Roadmap v2.0, ## ✅ v1.0, etc.
|
||||
const headingLevel = sectionMatch[1].match(/^(#{1,3})\s/)[1].length;
|
||||
const restContent = content.slice(sectionStart + sectionMatch[0].length);
|
||||
const nextMilestonePattern = new RegExp(
|
||||
`^#{1,${headingLevel}}\\s+(?:.*v\\d+\\.\\d+|✅|📋|🚧)`,
|
||||
'mi'
|
||||
);
|
||||
const nextMatch = restContent.match(nextMilestonePattern);
|
||||
|
||||
let sectionEnd;
|
||||
if (nextMatch) {
|
||||
sectionEnd = sectionStart + sectionMatch[0].length + nextMatch.index;
|
||||
} else {
|
||||
sectionEnd = content.length;
|
||||
}
|
||||
|
||||
// Return everything before the current milestone section (non-milestone content
|
||||
// like title, overview) plus the current milestone section
|
||||
const beforeMilestones = content.slice(0, sectionStart);
|
||||
const currentSection = content.slice(sectionStart, sectionEnd);
|
||||
|
||||
// Also include any content before the first milestone heading (title, overview, etc.)
|
||||
// but strip any <details> blocks in it (these are definitely shipped)
|
||||
const preamble = beforeMilestones.replace(/<details>[\s\S]*?<\/details>/gi, '');
|
||||
|
||||
return preamble + currentSection;
|
||||
}
|
||||
|
||||
/**
|
||||
* Replace a pattern only in the current milestone section of ROADMAP.md
|
||||
* (everything after the last </details> close tag). Used for write operations
|
||||
@@ -444,7 +529,7 @@ function getRoadmapPhaseInternal(cwd, phaseNum) {
|
||||
if (!fs.existsSync(roadmapPath)) return null;
|
||||
|
||||
try {
|
||||
const content = stripShippedMilestones(fs.readFileSync(roadmapPath, 'utf-8'));
|
||||
const content = extractCurrentMilestone(fs.readFileSync(roadmapPath, 'utf-8'), cwd);
|
||||
const escapedPhase = escapeRegex(phaseNum.toString());
|
||||
const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+${escapedPhase}:\\s*([^\\n]+)`, 'i');
|
||||
const headerMatch = content.match(phasePattern);
|
||||
@@ -549,7 +634,7 @@ function getMilestoneInfo(cwd) {
|
||||
function getMilestonePhaseFilter(cwd) {
|
||||
const milestonePhaseNums = new Set();
|
||||
try {
|
||||
const roadmap = stripShippedMilestones(fs.readFileSync(path.join(cwd, '.planning', 'ROADMAP.md'), 'utf-8'));
|
||||
const roadmap = extractCurrentMilestone(fs.readFileSync(path.join(cwd, '.planning', 'ROADMAP.md'), 'utf-8'), cwd);
|
||||
const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:/gi;
|
||||
let m;
|
||||
while ((m = phasePattern.exec(roadmap)) !== null) {
|
||||
@@ -597,6 +682,7 @@ module.exports = {
|
||||
getMilestoneInfo,
|
||||
getMilestonePhaseFilter,
|
||||
stripShippedMilestones,
|
||||
extractCurrentMilestone,
|
||||
replaceInCurrentMilestone,
|
||||
toPosixPath,
|
||||
};
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { execSync } = require('child_process');
|
||||
const { loadConfig, resolveModelInternal, findPhaseInternal, getRoadmapPhaseInternal, pathExistsInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, stripShippedMilestones, normalizePhaseName, toPosixPath, output, error } = require('./core.cjs');
|
||||
const { loadConfig, resolveModelInternal, findPhaseInternal, getRoadmapPhaseInternal, pathExistsInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, normalizePhaseName, toPosixPath, output, error } = require('./core.cjs');
|
||||
|
||||
function cmdInitExecutePhase(cwd, phase, raw) {
|
||||
if (!phase) {
|
||||
@@ -633,8 +633,8 @@ function cmdInitProgress(cwd, raw) {
|
||||
const roadmapPhaseNums = new Set();
|
||||
const roadmapPhaseNames = new Map();
|
||||
try {
|
||||
const roadmapContent = stripShippedMilestones(
|
||||
fs.readFileSync(path.join(cwd, '.planning', 'ROADMAP.md'), 'utf-8')
|
||||
const roadmapContent = extractCurrentMilestone(
|
||||
fs.readFileSync(path.join(cwd, '.planning', 'ROADMAP.md'), 'utf-8'), cwd
|
||||
);
|
||||
const headingPattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:\s*([^\n]+)/gi;
|
||||
let hm;
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { escapeRegex, normalizePhaseName, comparePhaseNum, findPhaseInternal, getArchivedPhaseDirs, generateSlugInternal, getMilestonePhaseFilter, stripShippedMilestones, replaceInCurrentMilestone, toPosixPath, output, error } = require('./core.cjs');
|
||||
const { escapeRegex, normalizePhaseName, comparePhaseNum, findPhaseInternal, getArchivedPhaseDirs, generateSlugInternal, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, toPosixPath, output, error } = require('./core.cjs');
|
||||
const { extractFrontmatter } = require('./frontmatter.cjs');
|
||||
const { writeStateMd } = require('./state.cjs');
|
||||
|
||||
@@ -319,7 +319,7 @@ function cmdPhaseAdd(cwd, description, raw) {
|
||||
}
|
||||
|
||||
const rawContent = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
const content = stripShippedMilestones(rawContent);
|
||||
const content = extractCurrentMilestone(rawContent, cwd);
|
||||
const slug = generateSlugInternal(description);
|
||||
|
||||
// Find highest integer phase number (in current milestone only)
|
||||
@@ -376,7 +376,7 @@ function cmdPhaseInsert(cwd, afterPhase, description, raw) {
|
||||
}
|
||||
|
||||
const rawContent = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
const content = stripShippedMilestones(rawContent);
|
||||
const content = extractCurrentMilestone(rawContent, cwd);
|
||||
const slug = generateSlugInternal(description);
|
||||
|
||||
// Normalize input then strip leading zeros for flexible matching
|
||||
@@ -760,7 +760,7 @@ function cmdPhaseComplete(cwd, phaseNum, raw) {
|
||||
if (fs.existsSync(reqPath)) {
|
||||
// Extract the current phase section from roadmap (scoped to avoid cross-phase matching)
|
||||
const phaseEsc = escapeRegex(phaseNum);
|
||||
const currentMilestoneRoadmap = stripShippedMilestones(roadmapContent);
|
||||
const currentMilestoneRoadmap = extractCurrentMilestone(roadmapContent, cwd);
|
||||
const phaseSectionMatch = currentMilestoneRoadmap.match(
|
||||
new RegExp(`(#{2,4}\\s*Phase\\s+${phaseEsc}[:\\s][\\s\\S]*?)(?=#{2,4}\\s*Phase\\s+|$)`, 'i')
|
||||
);
|
||||
@@ -824,7 +824,7 @@ function cmdPhaseComplete(cwd, phaseNum, raw) {
|
||||
// for phases that are defined but not yet planned (no directory on disk)
|
||||
if (isLastPhase && fs.existsSync(roadmapPath)) {
|
||||
try {
|
||||
const roadmapForPhases = stripShippedMilestones(fs.readFileSync(roadmapPath, 'utf-8'));
|
||||
const roadmapForPhases = extractCurrentMilestone(fs.readFileSync(roadmapPath, 'utf-8'), cwd);
|
||||
const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:\s*([^\n]+)/gi;
|
||||
let pm;
|
||||
while ((pm = phasePattern.exec(roadmapForPhases)) !== null) {
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { escapeRegex, normalizePhaseName, output, error, findPhaseInternal, stripShippedMilestones, replaceInCurrentMilestone } = require('./core.cjs');
|
||||
const { escapeRegex, normalizePhaseName, output, error, findPhaseInternal, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone } = require('./core.cjs');
|
||||
|
||||
function cmdRoadmapGetPhase(cwd, phaseNum, raw) {
|
||||
const roadmapPath = path.join(cwd, '.planning', 'ROADMAP.md');
|
||||
@@ -15,7 +15,7 @@ function cmdRoadmapGetPhase(cwd, phaseNum, raw) {
|
||||
}
|
||||
|
||||
try {
|
||||
const content = stripShippedMilestones(fs.readFileSync(roadmapPath, 'utf-8'));
|
||||
const content = extractCurrentMilestone(fs.readFileSync(roadmapPath, 'utf-8'), cwd);
|
||||
|
||||
// Escape special regex chars in phase number, handle decimal
|
||||
const escapedPhase = escapeRegex(phaseNum);
|
||||
@@ -99,7 +99,7 @@ function cmdRoadmapAnalyze(cwd, raw) {
|
||||
}
|
||||
|
||||
const rawContent = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
const content = stripShippedMilestones(rawContent);
|
||||
const content = extractCurrentMilestone(rawContent, cwd);
|
||||
const phasesDir = path.join(cwd, '.planning', 'phases');
|
||||
|
||||
// Extract all phase headings: ## Phase N: Name or ### Phase N: Name
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const os = require('os');
|
||||
const { safeReadFile, normalizePhaseName, execGit, findPhaseInternal, getMilestoneInfo, stripShippedMilestones, output, error } = require('./core.cjs');
|
||||
const { safeReadFile, normalizePhaseName, execGit, findPhaseInternal, getMilestoneInfo, stripShippedMilestones, extractCurrentMilestone, output, error } = require('./core.cjs');
|
||||
const { extractFrontmatter, parseMustHavesBlock } = require('./frontmatter.cjs');
|
||||
const { writeStateMd } = require('./state.cjs');
|
||||
|
||||
@@ -409,7 +409,7 @@ function cmdValidateConsistency(cwd, raw) {
|
||||
}
|
||||
|
||||
const roadmapContentRaw = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
const roadmapContent = stripShippedMilestones(roadmapContentRaw);
|
||||
const roadmapContent = extractCurrentMilestone(roadmapContentRaw, cwd);
|
||||
|
||||
// Extract phases from ROADMAP (archived milestones already stripped)
|
||||
const roadmapPhases = new Set();
|
||||
@@ -695,7 +695,7 @@ function cmdValidateHealth(cwd, options, raw) {
|
||||
// Inline subset of cmdValidateConsistency
|
||||
if (fs.existsSync(roadmapPath)) {
|
||||
const roadmapContentRaw = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
const roadmapContent = stripShippedMilestones(roadmapContentRaw);
|
||||
const roadmapContent = extractCurrentMilestone(roadmapContentRaw, cwd);
|
||||
const roadmapPhases = new Set();
|
||||
const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:/gi;
|
||||
let m;
|
||||
|
||||
Reference in New Issue
Block a user