fix: zero-padded phase numbers bypass archived-phase guard; stale current_milestone (#2458)
* fix(sdk): extractCurrentMilestone Backlog leak + state.begin-phase flag parsing Closes #2422 Closes #2420 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sdk): skip stateVersion early-return for shipped milestones When STATE.md has a stale `milestone: v1.0` entry but v1.0 is already shipped (heading contains ✅ in ROADMAP.md), the stateVersion early-return path in getMilestoneInfo was returning v1.0 instead of detecting the new active milestone. Two-part fix: 1. In the stateVersion block: skip the early-return when the matched heading line includes ✅ (shipped marker). Fall through to normal detection instead. 2. In the heading-format fallback regex: add a negative lookahead `(?!.*✅)` so the regex never matches a ✅ heading regardless of whether stateVersion was present. This handles the no-STATE.md case and ensures fallthrough from part 1 actually finds the next milestone. Adds two regression tests covering both ✅-suffix (`## v1.0 ✅ Name`) and ✅-prefix (`## ✅ v1.0 Name`) heading formats. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(core): allow padded-and-unpadded phase headings in getRoadmapPhaseInternal The zero-strip normalization (01→1) fixed the archived-phase guard but broke lookup against ROADMAP headings that still use zero-padded numbers like "Phase 01:". Change the regex to use 0*<normalized> so both formats match, making the fix robust regardless of ROADMAP heading style. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -600,6 +600,108 @@ describe('getMilestoneInfo', () => {
|
||||
assert.strictEqual(info.version, 'v1.0');
|
||||
assert.strictEqual(info.name, 'milestone');
|
||||
});
|
||||
|
||||
// Bug #2409: getMilestoneInfo must prefer STATE.md milestone: field over regex matching
|
||||
test('uses STATE.md milestone frontmatter when 🚧 is inside <summary> tag without bold (bug #2409)', () => {
|
||||
// STATE.md says v2.9, ROADMAP has 🚧 v2.9 inside <summary> (not bolded) — no bold regex match
|
||||
const roadmap = [
|
||||
'# Milestones',
|
||||
'',
|
||||
'- ✅ v2.2 Old Features — shipped 2026-04-03',
|
||||
'- 🚧 v2.9 Full-Pass Verification',
|
||||
'',
|
||||
'<details>',
|
||||
'<summary>🚧 v2.9 Full-Pass Verification & Bug Fixing — IN PROGRESS</summary>',
|
||||
'',
|
||||
'## Roadmap v2.9: Full-Pass Verification & Bug Fixing',
|
||||
'',
|
||||
'### Phase 1: Verification',
|
||||
'',
|
||||
'</details>',
|
||||
'',
|
||||
'## Phase Details (v2.2 — Old Features)',
|
||||
'',
|
||||
'### Phase 1: Old Stuff',
|
||||
].join('\n');
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap);
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'STATE.md'),
|
||||
'---\nmilestone: v2.9\n---\n\n# State\n'
|
||||
);
|
||||
|
||||
const info = getMilestoneInfo(tmpDir);
|
||||
assert.strictEqual(info.version, 'v2.9',
|
||||
'should return v2.9 from STATE.md, not v2.2 from a stale heading match');
|
||||
assert.ok(info.name.includes('Full-Pass') || info.name.includes('Verification'),
|
||||
`name should reference the v2.9 milestone, got: "${info.name}"`);
|
||||
});
|
||||
|
||||
test('STATE.md milestone takes precedence over first ## heading match (bug #2409)', () => {
|
||||
// ROADMAP with multiple ## headings — without STATE.md anchoring, first match wins
|
||||
const roadmap = [
|
||||
'## Phase Details (v1.5–v2.1)',
|
||||
'',
|
||||
'## Roadmap v2.2: Old Milestone',
|
||||
'',
|
||||
'## Roadmap v2.9: Current Milestone',
|
||||
'',
|
||||
'### Phase 1: Alpha',
|
||||
].join('\n');
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap);
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'STATE.md'),
|
||||
'---\nmilestone: v2.9\n---\n\n# State\n'
|
||||
);
|
||||
|
||||
const info = getMilestoneInfo(tmpDir);
|
||||
assert.strictEqual(info.version, 'v2.9',
|
||||
'should read v2.9 from STATE.md, not v2.2 from first ## heading');
|
||||
assert.strictEqual(info.name, 'Current Milestone');
|
||||
});
|
||||
|
||||
// Bug found in code review of PR #2458: stateVersion early-return doesn't check if shipped
|
||||
test('falls through to new active milestone when STATE.md version is already shipped (✅ heading)', () => {
|
||||
// STATE.md still says v1.0 (stale), but v1.0 is marked ✅ in ROADMAP.md.
|
||||
// getMilestoneInfo must NOT return v1.0; it must fall through and detect v2.0.
|
||||
const roadmap = [
|
||||
'## v1.0 ✅ Initial Release: Done',
|
||||
'',
|
||||
'### Phase 1: Setup',
|
||||
'',
|
||||
'## v2.0: Active Milestone',
|
||||
'',
|
||||
'### Phase 2: Build',
|
||||
].join('\n');
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap);
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'STATE.md'),
|
||||
'---\nmilestone: v1.0\n---\n\n# State\n'
|
||||
);
|
||||
|
||||
const info = getMilestoneInfo(tmpDir);
|
||||
assert.strictEqual(info.version, 'v2.0',
|
||||
'should return v2.0 (active milestone), not v1.0 (stale shipped milestone from STATE.md)');
|
||||
assert.strictEqual(info.name, 'Active Milestone');
|
||||
});
|
||||
|
||||
test('falls through when STATE.md version matches ✅ heading in alternate position formats', () => {
|
||||
// ✅ can appear before the version: ## ✅ v1.0 Old Name
|
||||
const roadmap = [
|
||||
'## ✅ v1.0 Old Name',
|
||||
'',
|
||||
'## v2.0: New Stuff',
|
||||
].join('\n');
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap);
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'STATE.md'),
|
||||
'---\nmilestone: v1.0\n---\n\n# State\n'
|
||||
);
|
||||
|
||||
const info = getMilestoneInfo(tmpDir);
|
||||
assert.strictEqual(info.version, 'v2.0',
|
||||
'should return v2.0, not stale v1.0 with ✅ prefix in heading');
|
||||
assert.strictEqual(info.name, 'New Stuff');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── searchPhaseInDir ──────────────────────────────────────────────────────────
|
||||
@@ -790,6 +892,41 @@ describe('getRoadmapPhaseInternal', () => {
|
||||
// Should not include Phase 2 content
|
||||
assert.ok(!result.section.includes('Phase 2: API'));
|
||||
});
|
||||
|
||||
// Bug #2391: zero-padded phase numbers ("03") must match unpadded ROADMAP headings ("Phase 3:")
|
||||
test('matches zero-padded phase number against unpadded ROADMAP heading (bug #2391)', () => {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'ROADMAP.md'),
|
||||
'### Phase 3: Rotation Engine\n**Goal**: Build rotation\n**Requirements**: ROTA-01\n'
|
||||
);
|
||||
const result = getRoadmapPhaseInternal(tmpDir, '03');
|
||||
assert.ok(result !== null, 'should find the phase with zero-padded input "03"');
|
||||
assert.strictEqual(result.found, true);
|
||||
assert.strictEqual(result.phase_name, 'Rotation Engine');
|
||||
assert.strictEqual(result.goal, 'Build rotation');
|
||||
});
|
||||
|
||||
test('matches double-zero-padded phase number against unpadded ROADMAP heading (bug #2391)', () => {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'ROADMAP.md'),
|
||||
'### Phase 7: Final\n**Goal**: Ship it\n'
|
||||
);
|
||||
const result = getRoadmapPhaseInternal(tmpDir, '007');
|
||||
assert.ok(result !== null, 'should find the phase with "007"');
|
||||
assert.strictEqual(result.found, true);
|
||||
assert.strictEqual(result.phase_name, 'Final');
|
||||
});
|
||||
|
||||
test('unpadded lookup still works after fix (regression check)', () => {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'ROADMAP.md'),
|
||||
'### Phase 3: Rotation Engine\n**Goal**: Build rotation\n'
|
||||
);
|
||||
const result = getRoadmapPhaseInternal(tmpDir, '3');
|
||||
assert.ok(result !== null);
|
||||
assert.strictEqual(result.found, true);
|
||||
assert.strictEqual(result.phase_name, 'Rotation Engine');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── getMilestonePhaseFilter ────────────────────────────────────────────────────
|
||||
|
||||
@@ -422,6 +422,66 @@ describe('init commands ignore archived phases from prior milestones sharing a n
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Bug #2391: zero-padded phase numbers must not bypass archived-phase guard
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('init plan-phase zero-padded phase number (bug #2391)', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempProject();
|
||||
// Current milestone ROADMAP has Phase 3 (unpadded heading)
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'ROADMAP.md'),
|
||||
'# v2.0 Roadmap\n\n### Phase 3: Rotation Engine + Availability\n**Goal**: Rotation\n**Requirements**: ROTA-01, ROTA-02\n**Plans:** TBD\n'
|
||||
);
|
||||
// Prior milestone archive has a shipped Phase 3 with different content
|
||||
const archivedDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '03-plant-collection-and-rooms');
|
||||
fs.mkdirSync(archivedDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(archivedDir, '03-CONTEXT.md'), '# OLD v1.0 Phase 3 context');
|
||||
fs.writeFileSync(path.join(archivedDir, '03-RESEARCH.md'), '# OLD v1.0 Phase 3 research');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('zero-padded "03" returns current ROADMAP phase, not archived v1.0 phase', () => {
|
||||
const result = runGsdTools('init plan-phase 03', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.strictEqual(output.phase_found, true);
|
||||
assert.strictEqual(output.phase_name, 'Rotation Engine + Availability',
|
||||
'phase_name must come from current ROADMAP.md, not the archived v1.0 phase');
|
||||
assert.strictEqual(output.phase_dir, null,
|
||||
'phase_dir must be null — current milestone has no directory yet');
|
||||
assert.strictEqual(output.has_context, false,
|
||||
'has_context must not inherit archived v1.0 artifacts');
|
||||
assert.strictEqual(output.has_research, false,
|
||||
'has_research must not inherit archived v1.0 artifacts');
|
||||
assert.ok(!output.context_path || !output.context_path.includes('v1.0'),
|
||||
'context_path must not point at archived v1.0 file');
|
||||
assert.strictEqual(output.phase_req_ids, 'ROTA-01, ROTA-02');
|
||||
});
|
||||
|
||||
test('unpadded "3" and zero-padded "03" return identical phase identity', () => {
|
||||
const result3 = runGsdTools('init plan-phase 3', tmpDir);
|
||||
const result03 = runGsdTools('init plan-phase 03', tmpDir);
|
||||
assert.ok(result3.success && result03.success, 'both commands must succeed');
|
||||
|
||||
const out3 = JSON.parse(result3.output);
|
||||
const out03 = JSON.parse(result03.output);
|
||||
assert.strictEqual(out03.phase_name, out3.phase_name,
|
||||
'phase_name must be identical regardless of padding');
|
||||
assert.strictEqual(out03.phase_slug, out3.phase_slug,
|
||||
'phase_slug must be identical regardless of padding');
|
||||
assert.strictEqual(out03.phase_req_ids, out3.phase_req_ids,
|
||||
'phase_req_ids must be identical regardless of padding');
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// cmdInitTodos (INIT-01)
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user