feat(#39): milestone-prefixed phase IDs (M-NN convention) + migration tool + validation (#565)

* feat(#39): milestone-prefixed phase IDs (M-NN convention) + migration tool + validation

- Add getMilestoneFromPhaseId() / getPhaseDirFromPhaseId() helpers to core.cjs
- Fix isDirInMilestone to match M-NN-style dirs (02-01-setup) against M-NN ROADMAP headings
- Extend heading regex to tolerate [bracket-token] scope prefix on phase headings
- Add W021 validation rule for milestone prefix mismatch
- Add gsd-tools roadmap validate + roadmap upgrade --convention milestone-prefixed
- Add phase_id_convention config field (null default, backwards-compatible)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(#39): address 4 Codex review findings in milestone-prefixed phase ID implementation

- getMilestoneFromPhaseId: tighten regex to require a digit after the hyphen (rejects '1-' and '1-abc')
- isDirInMilestone: use convention-aware regex — only capture M-NN segments when ROADMAP itself uses hyphenated phase IDs, preventing legacy dirs like '01-02-setup' from being misread as phase '1-02'
- checkW021: add UNPREFIXED_PHASE_RE path so unprefixed headings (### Phase 1:) also fire W021 when convention is milestone-prefixed
- roadmap-upgrade: remove isMigratedDirName dir-name check (false-positive for legacy dirs); config + ROADMAP heading checks at lines 194 and 212 are sufficient

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore: update changeset pr reference to #565

* fix(#39): restore phaseDirNameRe 2-digit minimum; add roadmap-upgrade to inventory

- validate.cjs: \d{1,} → \d{2,} to keep single-digit prefix rejection per W005 contract
- docs/INVENTORY.md: 79 → 80, add roadmap-upgrade.cjs row
- docs/INVENTORY-MANIFEST.json: regenerated (roadmap-upgrade.cjs entry)

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
2026-05-31 23:15:55 -04:00
committed by GitHub
parent 48c1827028
commit 0a12b06381
21 changed files with 1593 additions and 63 deletions

View File

@@ -0,0 +1,187 @@
/**
* Backwards-compatibility tests for legacy phase ID conventions.
*
* Covers:
* 1. Legacy 'Phase N' ROADMAP entries still work when phase_id_convention
* is null (the default — no config key set).
* 2. Deprecated warning fires for free-form roadmaps (non-fatal).
* 3. No automatic migration happens when a free-form roadmap is loaded.
* 4. isDirInMilestone still works for old-style dirs ('02-setup') against
* ROADMAP entries 'Phase 2:'.
* 5. isDirInMilestone works for new-style dirs ('GSD-02-01-setup') against
* ROADMAP entries 'Phase 2-01:'.
* 6. Heading regex matches both '### Phase 2-01: Setup' and
* '### [GSD] Phase 2-01: Setup'.
*
* Tests 1-3 exercise new behavior and will FAIL until implemented.
* Tests 4-6 exercise existing/new behavior and should pass once wired.
*/
'use strict';
const { describe, test, beforeEach, afterEach } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const { createTempProject, cleanup, runGsdTools, captureConsole } = require('./helpers.cjs');
const { getMilestonePhaseFilter } = require('../get-shit-done/bin/lib/core.cjs');
// ─── helpers ─────────────────────────────────────────────────────────────────
function writeRoadmap(tmpDir, content) {
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), content);
}
function writeConfig(tmpDir, obj) {
fs.writeFileSync(
path.join(tmpDir, '.planning', 'config.json'),
JSON.stringify(obj)
);
}
// ─── suite ───────────────────────────────────────────────────────────────────
describe('backwards-compat: legacy Phase N roadmap entries', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject();
});
afterEach(() => {
cleanup(tmpDir);
});
// ── test 1: legacy entries work with null phase_id_convention ──────────────
test('Phase N ROADMAP entries work when phase_id_convention is null (default)', () => {
// No phase_id_convention key → default (null) must still honour Phase N headings.
writeRoadmap(tmpDir, [
'## Roadmap v1.0: Current',
'',
'### Phase 1: Setup',
'**Goal:** initial setup',
'',
'### Phase 2: Build',
'**Goal:** build the thing',
].join('\n'));
const filter = getMilestonePhaseFilter(tmpDir);
assert.strictEqual(filter('01-setup'), true, 'old-style dir must match Phase 1');
assert.strictEqual(filter('02-build'), true, 'old-style dir must match Phase 2');
assert.strictEqual(filter('03-deploy'), false, 'unlisted phase must not match');
});
// ── test 2: deprecated warning fires for free-form roadmaps ───────────────
test('deprecated warning fires (non-fatal) when roadmap has no versioned milestone headings', () => {
// A "free-form" roadmap: phase headings but no ## vX.Y milestone section.
writeRoadmap(tmpDir, [
'### Phase 1: Setup',
'**Goal:** setup',
'',
'### Phase 2: Build',
'**Goal:** build',
].join('\n'));
const { stderr } = captureConsole(() => {
getMilestonePhaseFilter(tmpDir);
});
// Warning must fire but must not throw — non-fatal.
assert.match(
stderr,
/deprecated|free.form|phase_id_convention/i,
'a deprecation warning must be emitted for free-form roadmaps'
);
});
// ── test 3: no automatic migration ────────────────────────────────────────
test('loading a free-form roadmap does not rewrite ROADMAP.md on disk', () => {
const roadmapContent = [
'### Phase 1: Setup',
'**Goal:** setup',
].join('\n');
writeRoadmap(tmpDir, roadmapContent);
const roadmapPath = path.join(tmpDir, '.planning', 'ROADMAP.md');
const before = fs.readFileSync(roadmapPath, 'utf-8');
// Trigger a load — must not silently migrate the file.
getMilestonePhaseFilter(tmpDir);
const after = fs.readFileSync(roadmapPath, 'utf-8');
assert.equal(after, before, 'ROADMAP.md must not be rewritten during load');
});
// ── test 4: old-style dirs ('02-setup') match 'Phase 2:' ─────────────────
test('isDirInMilestone: old-style dir "02-setup" matches ROADMAP "Phase 2:"', () => {
writeRoadmap(tmpDir, [
'## Roadmap v1.0: Current',
'',
'### Phase 2: Setup',
'**Goal:** setup',
].join('\n'));
const filter = getMilestonePhaseFilter(tmpDir);
assert.strictEqual(filter('02-setup'), true, '"02-setup" must match "Phase 2:"');
assert.strictEqual(filter('2-setup'), true, '"2-setup" must also match "Phase 2:"');
assert.strictEqual(filter('03-other'), false, 'unlisted dir must not match');
});
// ── test 5: new-style dirs ('GSD-02-01-setup') match 'Phase 2-01:' ───────
test('isDirInMilestone: new-style dir "GSD-02-01-setup" matches ROADMAP "Phase 2-01:"', () => {
writeRoadmap(tmpDir, [
'## Roadmap v1.0: Current',
'',
'### Phase 2-01: Setup',
'**Goal:** setup',
].join('\n'));
writeConfig(tmpDir, { project_code: 'GSD' });
const filter = getMilestonePhaseFilter(tmpDir);
assert.strictEqual(
filter('GSD-02-01-setup'),
true,
'"GSD-02-01-setup" must match "Phase 2-01:"'
);
assert.strictEqual(
filter('02-01-setup'),
true,
'"02-01-setup" must match "Phase 2-01:" without project prefix'
);
});
// ── test 6: heading regex matches both plain and [GSD]-prefixed headings ──
test('phase heading regex matches "### Phase 2-01: Setup" and "### [GSD] Phase 2-01: Setup"', () => {
const plain = '### Phase 2-01: Setup';
const bracketed = '### [GSD] Phase 2-01: Setup';
// Both heading variants must be captured by the phasePattern used internally.
// We exercise this via getMilestonePhaseFilter with a roadmap containing each form.
const plainRoadmap = ['## Roadmap v1.0: Current', '', plain, '**Goal:** g'].join('\n');
const bracketedRoadmap = ['## Roadmap v1.0: Current', '', bracketed, '**Goal:** g'].join('\n');
writeRoadmap(tmpDir, plainRoadmap);
const filterPlain = getMilestonePhaseFilter(tmpDir);
assert.strictEqual(
filterPlain('02-01-setup'),
true,
'plain heading "### Phase 2-01:" must be matched'
);
writeRoadmap(tmpDir, bracketedRoadmap);
const filterBracketed = getMilestonePhaseFilter(tmpDir);
assert.strictEqual(
filterBracketed('02-01-setup'),
true,
'"### [GSD] Phase 2-01:" must also be matched by the heading regex'
);
});
});

View File

@@ -0,0 +1,90 @@
// Tests for getMilestoneFromPhaseId and getPhaseDirFromPhaseId helpers (issue #39).
'use strict';
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const {
getMilestoneFromPhaseId,
getPhaseDirFromPhaseId,
} = require('../get-shit-done/bin/lib/core.cjs');
// ─── getMilestoneFromPhaseId ────────────────────────────────────────────────
describe('getMilestoneFromPhaseId', () => {
test('maps milestone integer 1 to v1.0', () => {
assert.strictEqual(getMilestoneFromPhaseId('1-01'), 'v1.0');
});
test('uses only the top-level integer: 2-4-1 → v2.0', () => {
assert.strictEqual(getMilestoneFromPhaseId('2-4-1'), 'v2.0');
});
test('handles double-digit milestone: 10-01 → v10.0', () => {
assert.strictEqual(getMilestoneFromPhaseId('10-01'), 'v10.0');
});
test('returns null for sentinel 999 (backlog)', () => {
assert.strictEqual(getMilestoneFromPhaseId('999-1'), null);
});
test('returns null for sentinel 0 (pre-milestone spike)', () => {
assert.strictEqual(getMilestoneFromPhaseId('0-1'), null);
});
test('returns null when there is no hyphen separator', () => {
assert.strictEqual(getMilestoneFromPhaseId('1'), null);
});
test('strips project_code prefix: CK-2-01 → v2.0', () => {
assert.strictEqual(getMilestoneFromPhaseId('CK-2-01'), 'v2.0');
});
test('strips longer project_code prefix: GSD-10-01 → v10.0', () => {
assert.strictEqual(getMilestoneFromPhaseId('GSD-10-01'), 'v10.0');
});
test('returns null for fully non-numeric input', () => {
assert.strictEqual(getMilestoneFromPhaseId('invalid'), null);
});
});
// ─── getPhaseDirFromPhaseId ─────────────────────────────────────────────────
describe('getPhaseDirFromPhaseId', () => {
test('produces zero-padded dir with project code', () => {
assert.strictEqual(
getPhaseDirFromPhaseId('2-01', 'Setup Database', 'GSD'),
'GSD-02-01-setup-database',
);
});
test('omits project code when not provided', () => {
assert.strictEqual(
getPhaseDirFromPhaseId('2-01', 'Setup Database'),
'02-01-setup-database',
);
});
test('handles double-digit milestone with project code', () => {
assert.strictEqual(
getPhaseDirFromPhaseId('10-01', 'Build Feature', 'CK'),
'CK-10-01-build-feature',
);
});
test('produces zero-padded dir without project code: 1-01 → 01-01-setup', () => {
assert.strictEqual(
getPhaseDirFromPhaseId('1-01', 'Setup'),
'01-01-setup',
);
});
test('returns null for phase IDs without the M-NN hyphen form', () => {
assert.strictEqual(
getPhaseDirFromPhaseId('nohyphen', 'Some Title', 'GSD'),
null,
);
});
});

View File

@@ -0,0 +1,235 @@
'use strict';
/**
* W021 validation rule — milestone-prefixed phase ID convention.
*
* W021 fires when a phase ID's integer prefix doesn't match its enclosing
* milestone section (e.g. phase '1-01' listed under ## v2.0 is a mismatch).
*
* Also covers: `gsd-tools roadmap validate` subcommand shape.
*
* These features do NOT exist yet — this file is written TDD-first.
*/
const { describe, test, beforeEach, afterEach } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
// ---------------------------------------------------------------------------
// Fixture builder
// ---------------------------------------------------------------------------
/**
* Build a ROADMAP.md with milestone-prefixed sections at
* `tmpDir/.planning/ROADMAP.md`.
*
* @param {string} tmpDir - Temp project root returned by createTempProject().
* @param {Array<{version: string, label: string, phases: Array<{id: string, name: string}>}>} milestones
* Each milestone maps to a `## [GSD] vX.Y — Label` section; each phase maps
* to a `### Phase <id>: <name>` heading inside that section.
* @param {object} [opts]
* @param {string|null} [opts.conventionValue] - Value for the `phase_id_convention`
* front-matter field. Pass `null` to emit the key with a null/absent value.
* Omit (undefined) to use the default ('milestone-prefixed').
*/
function buildRoadmap(tmpDir, milestones, opts = {}) {
const { conventionValue } = opts;
let conventionLine;
if (conventionValue === null) {
conventionLine = 'phase_id_convention: null';
} else if (conventionValue === undefined) {
conventionLine = 'phase_id_convention: milestone-prefixed';
} else {
conventionLine = `phase_id_convention: ${conventionValue}`;
}
const frontmatter = `---\n${conventionLine}\n---\n\n`;
const sections = milestones
.map(({ version, label, phases }) => {
const phaseBlocks = phases
.map(({ id, name }) => `### Phase ${id}: ${name}\n**Goal:** Placeholder goal\n`)
.join('\n');
return `## [GSD] ${version} — ${label}\n\n${phaseBlocks}`;
})
.join('\n\n');
const content = `${frontmatter}# Roadmap\n\n${sections}\n`;
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), content);
}
// ---------------------------------------------------------------------------
// Suite
// ---------------------------------------------------------------------------
describe('W021 — milestone-prefixed phase ID convention', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject();
});
afterEach(() => {
cleanup(tmpDir);
});
// ── 1. Mismatch fires W021 ────────────────────────────────────────────────
test('W021 fires when phase 1-01 is listed under ## v2.0 (mismatch)', () => {
buildRoadmap(tmpDir, [
{
version: 'v2.0',
label: 'Expansion',
phases: [{ id: '1-01', name: 'Setup' }],
},
]);
const result = runGsdTools(['roadmap', 'validate'], tmpDir);
assert.ok(result.success, `roadmap validate should exit 0 even with warnings: ${result.error}`);
const out = JSON.parse(result.output);
assert.ok(Array.isArray(out.warnings), 'output.warnings should be an array');
const w021 = out.warnings.filter(w => w.code === 'W021');
assert.ok(w021.length > 0, 'at least one W021 warning expected for prefix mismatch');
const warning = w021[0];
assert.ok(warning.message, 'W021 entry should have a message field');
});
// ── 2. Match does NOT fire W021 ───────────────────────────────────────────
test('W021 does NOT fire when phase 2-01 is under ## v2.0 (match)', () => {
buildRoadmap(tmpDir, [
{
version: 'v2.0',
label: 'Expansion',
phases: [{ id: '2-01', name: 'New thing' }],
},
]);
const result = runGsdTools(['roadmap', 'validate'], tmpDir);
assert.ok(result.success, `roadmap validate failed: ${result.error}`);
const out = JSON.parse(result.output);
assert.ok(Array.isArray(out.warnings), 'output.warnings should be an array');
const w021 = out.warnings.filter(w => w.code === 'W021');
assert.strictEqual(w021.length, 0, 'no W021 warnings expected when prefix matches milestone');
});
// ── 3. Sentinel ranges are exempt ────────────────────────────────────────
test('W021 does NOT fire for sentinel range: phase 999-01 (backlog)', () => {
buildRoadmap(tmpDir, [
{
version: 'v1.0',
label: 'Foundation',
phases: [{ id: '999-01', name: 'Backlog item' }],
},
]);
const result = runGsdTools(['roadmap', 'validate'], tmpDir);
assert.ok(result.success, `roadmap validate failed: ${result.error}`);
const out = JSON.parse(result.output);
const w021 = (out.warnings || []).filter(w => w.code === 'W021');
assert.strictEqual(w021.length, 0, 'backlog sentinel (999-xx) should be exempt from W021');
});
test('W021 does NOT fire for sentinel range: phase 0-01 (pre-milestone)', () => {
buildRoadmap(tmpDir, [
{
version: 'v1.0',
label: 'Foundation',
phases: [{ id: '0-01', name: 'Pre-milestone work' }],
},
]);
const result = runGsdTools(['roadmap', 'validate'], tmpDir);
assert.ok(result.success, `roadmap validate failed: ${result.error}`);
const out = JSON.parse(result.output);
const w021 = (out.warnings || []).filter(w => w.code === 'W021');
assert.strictEqual(w021.length, 0, 'pre-milestone sentinel (0-xx) should be exempt from W021');
});
// ── 4. null convention disables W021 ─────────────────────────────────────
test('W021 does NOT fire when phase_id_convention is null (free-form roadmap)', () => {
buildRoadmap(
tmpDir,
[
{
version: 'v2.0',
label: 'Expansion',
// Deliberately mismatched prefix to confirm the rule is disabled
phases: [{ id: '1-01', name: 'Setup' }],
},
],
{ conventionValue: null }
);
const result = runGsdTools(['roadmap', 'validate'], tmpDir);
assert.ok(result.success, `roadmap validate failed: ${result.error}`);
const out = JSON.parse(result.output);
const w021 = (out.warnings || []).filter(w => w.code === 'W021');
assert.strictEqual(w021.length, 0, 'W021 must not fire when convention is null');
});
// ── 5. `roadmap validate` returns JSON with warnings array ───────────────
test("'gsd-tools roadmap validate' subcommand returns JSON with warnings array", () => {
buildRoadmap(tmpDir, [
{
version: 'v1.0',
label: 'Foundation',
phases: [{ id: '1-01', name: 'Setup' }],
},
]);
const result = runGsdTools(['roadmap', 'validate'], tmpDir);
assert.ok(result.success, `roadmap validate should succeed: ${result.error}`);
let out;
try {
out = JSON.parse(result.output);
} catch {
assert.fail(`roadmap validate output is not valid JSON: ${result.output}`);
}
assert.ok(typeof out === 'object' && out !== null, 'output should be a JSON object');
assert.ok(Array.isArray(out.warnings), 'output should have a warnings array');
});
// ── 6. W021 message includes migration command ────────────────────────────
test('W021 warning text includes the migration command', () => {
buildRoadmap(tmpDir, [
{
version: 'v2.0',
label: 'Expansion',
phases: [{ id: '1-01', name: 'Mismatched phase' }],
},
]);
const result = runGsdTools(['roadmap', 'validate'], tmpDir);
assert.ok(result.success, `roadmap validate failed: ${result.error}`);
const out = JSON.parse(result.output);
const w021 = (out.warnings || []).filter(w => w.code === 'W021');
assert.ok(w021.length > 0, 'W021 warning expected');
const migrationCmd = 'gsd-tools roadmap upgrade --convention milestone-prefixed';
const hasMigration = w021.some(w => typeof w.message === 'string' && w.message.includes(migrationCmd));
assert.ok(
hasMigration,
`W021 warning message should include "${migrationCmd}". Got: ${JSON.stringify(w021.map(w => w.message))}`
);
});
});

View File

@@ -82,6 +82,6 @@ describe('roadmap-command-router', () => {
},
});
assert.equal(message, 'Unknown roadmap subcommand. Available: analyze, get-phase, update-plan-progress, annotate-dependencies');
assert.equal(message, 'Unknown roadmap subcommand. Available: analyze, get-phase, update-plan-progress, annotate-dependencies, validate, upgrade');
});
});