`extractCurrentMilestone()` scoped the current-milestone window to its `## Phases` checklist subsection and terminated at the milestone's own `## Milestone … (Phase Details)` heading, so the `### Phase N:` detail headers fell outside scope. Every parser-backed command — `init.phase-op` (and thus `/gsd:discuss-phase`, `/gsd:plan-phase`), `state`, `roadmap list`, and `validate health` (W006) — therefore could not resolve phases of any milestone after the first until a `.planning/phases/` directory already existed, blocking discuss/plan. The parser now additionally includes the current milestone's `(Phase Details)` section in scope, located via the already-computed version matches and anchored (boundary-aware) to the selected milestone's version token so sibling sub-milestones sharing a version prefix do not cross-pollinate. The existing heading selection and primary window are unchanged. Adds tests/bug-730-milestone-phase-details-scope.test.cjs covering the two-milestone reproduction, first-milestone non-regression, direct getRoadmapPhaseInternal resolution, validate-health W006 visibility, a three-milestone roadmap, and the closed-sibling sub-milestone case. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/730-roadmap-milestone-phase-details-scope.md
Normal file
5
.changeset/730-roadmap-milestone-phase-details-scope.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 748
|
||||
---
|
||||
The roadmap parser now resolves fresh phases of the current milestone in multi-milestone roadmaps. `extractCurrentMilestone()` scoped the current-milestone window to its `## Phases` checklist subsection and stopped at the milestone's own `## Milestone … (Phase Details)` heading, so the `### Phase N:` detail headers fell out of scope. Any command backed by the parser — `init.phase-op` (and therefore `/gsd:discuss-phase` and `/gsd:plan-phase`), `state`, `roadmap list`, and `validate health` (W006) — could not resolve phases of any milestone after the first until a `.planning/phases/` directory already existed, blocking discuss/plan. The parser now also includes the current milestone's `(Phase Details)` section in scope, anchored to the selected milestone's version token so sibling sub-milestones do not cross-pollinate. (#730)
|
||||
@@ -43,6 +43,7 @@
|
||||
},
|
||||
"milestone": {
|
||||
"files": [
|
||||
"bug-730-milestone-phase-details-scope.test.cjs",
|
||||
"milestone-archive.test.cjs",
|
||||
"milestone-helper.test.cjs",
|
||||
"milestone-prefixed-convention.test.cjs",
|
||||
|
||||
97
src/core.cts
97
src/core.cts
@@ -1085,37 +1085,40 @@ function extractCurrentMilestone(content: string, cwd?: string): string {
|
||||
|
||||
const sectionStart = selected.index;
|
||||
|
||||
const sectionMatch = selected;
|
||||
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+(?!Phase\\s+\\S)(?:.*v\\d+\\.\\d+|✅|📋|🚧)`,
|
||||
'i'
|
||||
);
|
||||
|
||||
let sectionEnd = content.length;
|
||||
let fenceChar: string | null = null;
|
||||
let fenceLen = 0;
|
||||
let charOffset = 0;
|
||||
for (const line of restContent.split('\n')) {
|
||||
const fenceMatch = line.match(/^\s{0,3}((?:`{3,}|~{3,}))(.*)/);
|
||||
if (fenceMatch) {
|
||||
const char = fenceMatch[1][0];
|
||||
const len = fenceMatch[1].length;
|
||||
const trailing = fenceMatch[2] || '';
|
||||
if (!fenceChar) {
|
||||
fenceChar = char;
|
||||
fenceLen = len;
|
||||
} else if (char === fenceChar && len >= fenceLen && /^\s*$/.test(trailing)) {
|
||||
fenceChar = null;
|
||||
fenceLen = 0;
|
||||
const computeSectionEnd = (headingText: string, headingStart: number): number => {
|
||||
const level = (headingText.match(/^(#{1,3})\s/) ?? ['', '#'])[1].length;
|
||||
const rest = content.slice(headingStart + headingText.length);
|
||||
const stopPattern = new RegExp(
|
||||
`^#{1,${level}}\\s+(?!Phase\\s+\\S)(?:.*v\\d+\\.\\d+|✅|📋|🚧)`,
|
||||
'i',
|
||||
);
|
||||
let end = content.length;
|
||||
let fc: string | null = null;
|
||||
let fl = 0;
|
||||
let off = 0;
|
||||
for (const line of rest.split('\n')) {
|
||||
const fm = line.match(/^\s{0,3}((?:`{3,}|~{3,}))(.*)/);
|
||||
if (fm) {
|
||||
const ch = fm[1][0];
|
||||
const ln = fm[1].length;
|
||||
const trailing = fm[2] || '';
|
||||
if (!fc) {
|
||||
fc = ch;
|
||||
fl = ln;
|
||||
} else if (ch === fc && ln >= fl && /^\s*$/.test(trailing)) {
|
||||
fc = null;
|
||||
fl = 0;
|
||||
}
|
||||
} else if (!fc && stopPattern.test(line)) {
|
||||
end = headingStart + headingText.length + off;
|
||||
break;
|
||||
}
|
||||
} else if (!fenceChar && nextMilestonePattern.test(line)) {
|
||||
sectionEnd = sectionStart + sectionMatch[0].length + charOffset;
|
||||
break;
|
||||
off += line.length + 1;
|
||||
}
|
||||
charOffset += line.length + 1;
|
||||
}
|
||||
return end;
|
||||
};
|
||||
|
||||
const sectionEnd = computeSectionEnd(selected[0], sectionStart);
|
||||
|
||||
const anyMilestonePattern = /^#{1,3}\s+(?!Phase\s+\S)(?:.*v\d+\.\d+|✅|📋|🚧)/im;
|
||||
const firstMilestoneMatch = content.match(anyMilestonePattern);
|
||||
@@ -1125,12 +1128,46 @@ function extractCurrentMilestone(content: string, cwd?: string): string {
|
||||
const beforeMilestones = content.slice(0, preambleCutoff);
|
||||
const currentSection = content.slice(sectionStart, sectionEnd);
|
||||
|
||||
// Multi-milestone roadmaps split each added milestone across two version-bearing
|
||||
// headings: a `## Phases` checklist subsection (early) and a dedicated
|
||||
// `## Milestone … (Phase Details)` section (late) holding the `### Phase N:`
|
||||
// detail headers. The scope window above stops at the next version-bearing
|
||||
// heading — the current milestone's OWN Phase Details heading — leaving those
|
||||
// detail headers outside `currentSection`. Append that section so phase
|
||||
// resolution and counting see the current milestone's phases. Anchor the lookup
|
||||
// to the SELECTED heading's specific version token (boundary-aware, so a
|
||||
// `v3.0` state does not match a `v3.0-A` sub-milestone) so sibling milestones
|
||||
// that share a version prefix do not cross-pollinate. (#730)
|
||||
const selectedVersionToken = selected[1].match(
|
||||
/v\d+(?:\.\d+)+(?:[-.][A-Za-z0-9]+)*/i,
|
||||
)?.[0];
|
||||
const detailsVersionBoundary = selectedVersionToken
|
||||
? new RegExp(`${escapeRegex(selectedVersionToken)}(?![\\w.-])`, 'i')
|
||||
: null;
|
||||
let detailsSection = '';
|
||||
const detailsMatch = allMatches.find(
|
||||
(m) =>
|
||||
/\(Phase\s+Details\)/i.test(m[1]) &&
|
||||
!isClosed(m[1]) &&
|
||||
(!detailsVersionBoundary || detailsVersionBoundary.test(m[1])) &&
|
||||
(m.index ?? 0) >= sectionEnd,
|
||||
);
|
||||
if (detailsMatch) {
|
||||
const detailsStart = detailsMatch.index ?? 0;
|
||||
detailsSection = content.slice(
|
||||
detailsStart,
|
||||
computeSectionEnd(detailsMatch[0], detailsStart),
|
||||
);
|
||||
}
|
||||
|
||||
const preamble = beforeMilestones
|
||||
.replace(/<details>[\s\S]*?<\/details>/gi, '')
|
||||
.replace(/^#{2,4}\s*Phase\s+[\w][\w.-]*\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim, '')
|
||||
.replace(/^#{1,4}\s*Phase Details\b[^\n]*\n?/gim, '');
|
||||
|
||||
return preamble + currentSection;
|
||||
return detailsSection
|
||||
? preamble + currentSection + '\n' + detailsSection
|
||||
: preamble + currentSection;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
275
tests/bug-730-milestone-phase-details-scope.test.cjs
Normal file
275
tests/bug-730-milestone-phase-details-scope.test.cjs
Normal file
@@ -0,0 +1,275 @@
|
||||
/**
|
||||
* Regression test for bug #730: phase details defined under a milestone-scoped
|
||||
* "## Milestone vX.Y — … (Phase Details)" section are invisible to phase
|
||||
* resolution (getRoadmapPhaseInternal / init phase-op) when the flat shared
|
||||
* "## Phase Details" section for an earlier milestone sits between the shared
|
||||
* ## Phases checklist and the per-milestone Phase Details section.
|
||||
*
|
||||
* The bug manifests ONLY before any .planning/phases/ directory exists because
|
||||
* findPhaseInternal masks it once the dir is created. RED step — tests 1 and 3
|
||||
* are expected to fail against current code.
|
||||
*/
|
||||
|
||||
const { test, describe, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const os = require('os');
|
||||
const { runGsdTools, cleanup } = require('./helpers.cjs');
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Shared fixture content
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const STATE_CONTENT = `---
|
||||
milestone: v1.1
|
||||
---
|
||||
`;
|
||||
|
||||
const ROADMAP_CONTENT = `# Roadmap: Example
|
||||
|
||||
## Phases
|
||||
|
||||
- [x] **Phase 1: Setup** — initial scaffold
|
||||
|
||||
### Milestone v1.1 — Second milestone (added 2026-01-01)
|
||||
|
||||
- [ ] **Phase 2: Feature** — the new thing
|
||||
|
||||
## Phase Details
|
||||
|
||||
### Phase 1: Setup
|
||||
**Goal:** scaffold the app.
|
||||
|
||||
## Milestone v1.1 — Second milestone (Phase Details)
|
||||
|
||||
### Phase 2: Feature
|
||||
**Goal:** build the new thing.
|
||||
`;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Helper: create a bare project with .planning/ but NO .planning/phases/ dir
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
function createBareProject() {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-test-730-'));
|
||||
fs.mkdirSync(path.join(tmpDir, '.planning'), { recursive: true });
|
||||
return tmpDir;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Suite
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('bug #730 — milestone (Phase Details) section scope resolution', () => {
|
||||
let dir;
|
||||
|
||||
beforeEach(() => {
|
||||
dir = createBareProject();
|
||||
fs.writeFileSync(path.join(dir, '.planning', 'STATE.md'), STATE_CONTENT, 'utf-8');
|
||||
fs.writeFileSync(path.join(dir, '.planning', 'ROADMAP.md'), ROADMAP_CONTENT, 'utf-8');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(dir);
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Test 1 (AC1): init phase-op resolves phase defined only under its
|
||||
// per-milestone "(Phase Details)" section
|
||||
// -------------------------------------------------------------------------
|
||||
test('init phase-op resolves a current-milestone phase defined only under its (Phase Details) section', () => {
|
||||
const r = runGsdTools('init phase-op 2', dir);
|
||||
assert.ok(r.success, `init phase-op 2 failed: ${r.error}`);
|
||||
|
||||
const out = JSON.parse(r.output);
|
||||
assert.strictEqual(out.phase_found, true, `phase_found should be true; got phase_found=${out.phase_found}, expected_phase_dir=${out.expected_phase_dir}`);
|
||||
assert.strictEqual(out.phase_name, 'Feature', `phase_name should be 'Feature'; got '${out.phase_name}'`);
|
||||
assert.strictEqual(out.padded_phase, '02', `padded_phase should be '02'; got '${out.padded_phase}'`);
|
||||
assert.strictEqual(out.expected_phase_dir, '.planning/phases/02-feature', `expected_phase_dir should be '.planning/phases/02-feature'; got '${out.expected_phase_dir}'`);
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Test 2 (AC4): first-milestone phase still resolves via the flat
|
||||
// "## Phase Details" section — no regression
|
||||
// -------------------------------------------------------------------------
|
||||
test('init phase-op still resolves a first-milestone phase (no regression on flat Phase Details)', () => {
|
||||
const r = runGsdTools('init phase-op 1', dir);
|
||||
assert.ok(r.success, `init phase-op 1 failed: ${r.error}`);
|
||||
|
||||
const out = JSON.parse(r.output);
|
||||
assert.strictEqual(out.phase_found, true, `phase_found should be true for phase 1; got ${out.phase_found}`);
|
||||
assert.strictEqual(out.phase_name, 'Setup', `phase_name should be 'Setup'; got '${out.phase_name}'`);
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Test 3 (AC5): getRoadmapPhaseInternal resolves the current-milestone phase
|
||||
// directly before any phases/ dir exists
|
||||
// -------------------------------------------------------------------------
|
||||
test('getRoadmapPhaseInternal resolves the current-milestone phase directly before any dir exists', () => {
|
||||
const core = require('../gsd-core/bin/lib/core.cjs');
|
||||
|
||||
const res = core.getRoadmapPhaseInternal(dir, '2');
|
||||
assert.ok(res !== null && res !== undefined, `getRoadmapPhaseInternal returned null/undefined for phase 2`);
|
||||
assert.strictEqual(res.found, true, `res.found should be true; got ${JSON.stringify(res)}`);
|
||||
assert.strictEqual(res.phase_name, 'Feature', `res.phase_name should be 'Feature'; got '${res.phase_name}'`);
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Test 4 (AC3): validate health raises W006 for a current-milestone phase
|
||||
// defined under (Phase Details) with no directory on disk.
|
||||
//
|
||||
// Before the fix, extractCurrentMilestone's slice stopped before the
|
||||
// "## Milestone v1.1 — … (Phase Details)" section, so phase 2's
|
||||
// "### Phase 2: Feature" header was invisible and W006 was never raised.
|
||||
// After the fix the slice includes that section and W006 is emitted.
|
||||
//
|
||||
// This test uses its OWN local fixture (separate tmpdir) so it does not
|
||||
// disturb the shared beforeEach/afterEach fixture used by tests 1–3.
|
||||
// -------------------------------------------------------------------------
|
||||
test('validate health raises W006 for a started current-milestone phase defined under (Phase Details) with no directory', () => {
|
||||
const localDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-test-730-t4-'));
|
||||
try {
|
||||
const planning = path.join(localDir, '.planning');
|
||||
fs.mkdirSync(planning, { recursive: true });
|
||||
|
||||
// STATE.md — milestone: v1.1
|
||||
fs.writeFileSync(
|
||||
path.join(planning, 'STATE.md'),
|
||||
`---\nmilestone: v1.1\n---\n`,
|
||||
'utf-8',
|
||||
);
|
||||
|
||||
// ROADMAP.md — phase 2 is [x] (started/complete) so the not-started
|
||||
// guard does NOT suppress W006. Phase 2's details live exclusively in
|
||||
// the per-milestone "(Phase Details)" section (the blind-spot pre-fix).
|
||||
fs.writeFileSync(
|
||||
path.join(planning, 'ROADMAP.md'),
|
||||
`# Roadmap: Example\n\n## Phases\n\n- [x] **Phase 1: Setup** — initial scaffold\n\n### Milestone v1.1 — Second milestone (added 2026-01-01)\n\n- [x] **Phase 2: Feature** — the new thing\n\n## Phase Details\n\n### Phase 1: Setup\n**Goal:** scaffold the app.\n\n## Milestone v1.1 — Second milestone (Phase Details)\n\n### Phase 2: Feature\n**Goal:** build the new thing.\n`,
|
||||
'utf-8',
|
||||
);
|
||||
|
||||
// Create the phase 1 directory so phase 1 does NOT trigger W006.
|
||||
// Phase 2 has NO directory — that's the missing-dir condition under test.
|
||||
fs.mkdirSync(path.join(planning, 'phases', '01-setup'), { recursive: true });
|
||||
|
||||
const result = runGsdTools(['validate', 'health'], localDir);
|
||||
const payload = JSON.parse(result.output);
|
||||
const warnings = payload.warnings || [];
|
||||
|
||||
// Find a W006 entry whose message references phase 2 (by number or name).
|
||||
const w006ForPhase2 = warnings.find(
|
||||
(w) =>
|
||||
w.code === 'W006' &&
|
||||
(/\b2\b/.test(w.message) || /\b02\b/.test(w.message) || /Feature/i.test(w.message)),
|
||||
);
|
||||
|
||||
assert.ok(
|
||||
w006ForPhase2 != null,
|
||||
`Expected a W006 warning referencing phase 2 (Feature) — phase 2 is started ([x]) and has no directory on disk, ` +
|
||||
`but its ### Phase 2: header lives in the Milestone v1.1 (Phase Details) section which was invisible before the fix. ` +
|
||||
`Got warnings: ${JSON.stringify(warnings)}`,
|
||||
);
|
||||
} finally {
|
||||
cleanup(localDir);
|
||||
}
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Test 5: three-milestone roadmap, current = latest (v1.2)
|
||||
// -------------------------------------------------------------------------
|
||||
test('init phase-op resolves the latest milestone phase in a 3-milestone roadmap', () => {
|
||||
const localDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-test-730-t5-'));
|
||||
try {
|
||||
const planning = path.join(localDir, '.planning');
|
||||
fs.mkdirSync(planning, { recursive: true });
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(planning, 'STATE.md'),
|
||||
`---\nmilestone: v1.2\n---\n`,
|
||||
'utf-8',
|
||||
);
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(planning, 'ROADMAP.md'),
|
||||
`# Roadmap: Example\n\n## Phases\n\n- [x] **Phase 1: Setup** — done\n\n### Milestone v1.1 — Second (added 2026-01-01)\n\n- [x] **Phase 2: Feature** — done\n\n### Milestone v1.2 — Third (added 2026-02-01)\n\n- [ ] **Phase 3: Polish** — current\n\n## Phase Details\n\n### Phase 1: Setup\n**Goal:** scaffold.\n\n## Milestone v1.1 — Second (Phase Details)\n\n### Phase 2: Feature\n**Goal:** build.\n\n## Milestone v1.2 — Third (Phase Details)\n\n### Phase 3: Polish\n**Goal:** refine.\n`,
|
||||
'utf-8',
|
||||
);
|
||||
|
||||
const r = runGsdTools('init phase-op 3', localDir);
|
||||
assert.ok(r.success, `init phase-op 3 failed: ${r.error}`);
|
||||
|
||||
const out = JSON.parse(r.output);
|
||||
assert.strictEqual(out.phase_found, true, `phase_found should be true; got phase_found=${out.phase_found}`);
|
||||
assert.strictEqual(out.phase_name, 'Polish', `phase_name should be 'Polish'; got '${out.phase_name}'`);
|
||||
assert.strictEqual(out.padded_phase, '03', `padded_phase should be '03'; got '${out.padded_phase}'`);
|
||||
assert.strictEqual(out.expected_phase_dir, '.planning/phases/03-polish', `expected_phase_dir should be '.planning/phases/03-polish'; got '${out.expected_phase_dir}'`);
|
||||
} finally {
|
||||
cleanup(localDir);
|
||||
}
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Test 6: sub-milestone sharing a version prefix — closed sibling must NOT
|
||||
// cross-pollinate into the active milestone's Phase Details lookup (#730)
|
||||
// -------------------------------------------------------------------------
|
||||
test('init phase-op anchors Phase Details to the selected sub-milestone, not a closed same-prefix sibling', () => {
|
||||
const localDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-test-730-t6-'));
|
||||
try {
|
||||
const planning = path.join(localDir, '.planning');
|
||||
fs.mkdirSync(planning, { recursive: true });
|
||||
|
||||
// STATE.md — milestone: v3.0 (matches v3.0-B active slice)
|
||||
fs.writeFileSync(
|
||||
path.join(planning, 'STATE.md'),
|
||||
`---\nmilestone: v3.0\n---\n`,
|
||||
'utf-8',
|
||||
);
|
||||
|
||||
// ROADMAP.md — v3.0-A is SHIPPED (closed), v3.0-B is active.
|
||||
// The Phase Details for v3.0-A comes FIRST — without version-boundary
|
||||
// anchoring the old code would grab it (first non-closed (Phase Details)
|
||||
// heading outside the window), returning phase_name='Alpha' instead of 'Beta'.
|
||||
fs.writeFileSync(
|
||||
path.join(planning, 'ROADMAP.md'),
|
||||
[
|
||||
'# Roadmap: Example',
|
||||
'',
|
||||
'## Phases',
|
||||
'',
|
||||
'### Milestone v3.0-A — First slice (added 2026-01-01) ✅ SHIPPED',
|
||||
'',
|
||||
'- [x] **Phase 1: Alpha** — done',
|
||||
'',
|
||||
'### Milestone v3.0-B — Second slice (added 2026-02-01)',
|
||||
'',
|
||||
'- [ ] **Phase 2: Beta** — current',
|
||||
'',
|
||||
'## Phase Details',
|
||||
'',
|
||||
'## Milestone v3.0-A — First slice (Phase Details)',
|
||||
'',
|
||||
'### Phase 1: Alpha',
|
||||
'**Goal:** alpha goal.',
|
||||
'',
|
||||
'## Milestone v3.0-B — Second slice (Phase Details)',
|
||||
'',
|
||||
'### Phase 2: Beta',
|
||||
'**Goal:** beta goal.',
|
||||
'',
|
||||
].join('\n'),
|
||||
'utf-8',
|
||||
);
|
||||
|
||||
const r = runGsdTools('init phase-op 2', localDir);
|
||||
assert.ok(r.success, `init phase-op 2 failed: ${r.error}`);
|
||||
|
||||
const out = JSON.parse(r.output);
|
||||
assert.strictEqual(out.phase_found, true, `phase_found should be true; got phase_found=${out.phase_found}, output=${JSON.stringify(out)}`);
|
||||
assert.strictEqual(out.phase_name, 'Beta', `phase_name should be 'Beta' (v3.0-B section), not '${out.phase_name}' (would indicate v3.0-A cross-pollination)`);
|
||||
assert.strictEqual(out.expected_phase_dir, '.planning/phases/02-beta', `expected_phase_dir should be '.planning/phases/02-beta'; got '${out.expected_phase_dir}'`);
|
||||
} finally {
|
||||
cleanup(localDir);
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user