diff --git a/.changeset/brave-cats-leap.md b/.changeset/brave-cats-leap.md new file mode 100644 index 000000000..29401c9dc --- /dev/null +++ b/.changeset/brave-cats-leap.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1218 +--- +**Legacy ROADMAP projects no longer get deprecation-warning spam** — the free-form ROADMAP warning fired on every command regardless of phase_id_convention; it now only warns when the milestone-prefixed convention is explicitly set and unmet. (#1218) diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index c89f90e52..64c315ef1 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -377,8 +377,17 @@ type MilestonePhaseFilter = ((dirName: string) => boolean) & { /** * Returns a filter function that checks whether a phase directory belongs * to the current milestone based on ROADMAP.md phase headings. + * + * @param cwd - Project working directory. + * @param versionOverride - Optional version string to scope the phase filter + * to a specific milestone (e.g. 'v1.2'). + * @param phaseIdConvention - The resolved `phase_id_convention` config value. + * When `'milestone-prefixed'`, a deprecation warning is emitted for + * free-form ROADMAPs that lack versioned milestone headings. When absent or + * any other value, the warning is suppressed — legacy/default projects must + * never see spurious warnings. */ -function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null): MilestonePhaseFilter { +function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, phaseIdConvention?: string | null): MilestonePhaseFilter { const milestonePhaseNums = new Set(); let missingExplicitVersion = false; try { @@ -389,10 +398,11 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null): const hasVersionedMilestonesGlobal = /^#{1,3}\s+.*v\d+\.\d+/mi.test(roadmapContent); const hasPhaseHeadings = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+[\w]/i.test(roadmapContent); - if (!hasVersionedMilestonesGlobal && hasPhaseHeadings) { + if (!hasVersionedMilestonesGlobal && hasPhaseHeadings && phaseIdConvention === 'milestone-prefixed') { console.warn( '[gsd] Deprecated: free-form ROADMAP.md detected (no versioned milestone headings). ' + - 'Set phase_id_convention in config.json to suppress this warning.' + 'The project has phase_id_convention set to "milestone-prefixed" in config.json but the ' + + 'ROADMAP does not use versioned milestone headings. Run `gsd-tools roadmap upgrade --convention milestone-prefixed` to migrate (dry-run by default).' ); } diff --git a/tests/backwards-compat-phase-id.test.cjs b/tests/backwards-compat-phase-id.test.cjs index 20536b978..d1f343323 100644 --- a/tests/backwards-compat-phase-id.test.cjs +++ b/tests/backwards-compat-phase-id.test.cjs @@ -4,7 +4,10 @@ * 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). + * 2a. Deprecated warning is SUPPRESSED when phase_id_convention is not set + * (legacy/default projects must see ZERO warnings). [regression guard] + * 2b. Deprecated warning FIRES when phase_id_convention is explicitly + * 'milestone-prefixed' and the roadmap doesn't conform (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:'. @@ -13,8 +16,8 @@ * 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. + * Tests 1-3 exercise new/regression behavior. + * Tests 4-6 exercise existing/new behavior. */ 'use strict'; @@ -73,10 +76,14 @@ describe('backwards-compat: legacy Phase N roadmap entries', () => { assert.strictEqual(filter('03-deploy'), false, 'unlisted phase must not match'); }); - // ── test 2: deprecated warning fires for free-form roadmaps ─────────────── + // ── test 2a: NO warning for legacy/default projects (regression guard) ─────── + // This is the PRIMARY regression guard: a project with no phase_id_convention + // must never receive the deprecation warning. Before the fix this test FAILS + // because the warning fires unconditionally. - 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. + test('no deprecation warning when phase_id_convention is not set (legacy default)', () => { + // Free-form roadmap (no versioned milestone headings) AND no config file at + // all — the warning must be fully suppressed. writeRoadmap(tmpDir, [ '### Phase 1: Setup', '**Goal:** setup', @@ -89,11 +96,35 @@ describe('backwards-compat: legacy Phase N roadmap entries', () => { getMilestonePhaseFilter(tmpDir); }); + assert.strictEqual( + stderr, + '', + 'no deprecation warning must be emitted when phase_id_convention is not set' + ); + }); + + // ── test 2b: deprecated warning fires when convention is milestone-prefixed ── + + test('deprecated warning fires (non-fatal) when phase_id_convention is milestone-prefixed and roadmap lacks versioned milestones', () => { + // A "free-form" roadmap (no ## vX.Y milestone section) combined with the + // explicit milestone-prefixed convention — the warning is actionable here. + writeRoadmap(tmpDir, [ + '### Phase 1: Setup', + '**Goal:** setup', + '', + '### Phase 2: Build', + '**Goal:** build', + ].join('\n')); + + const { stderr } = captureConsole(() => { + getMilestonePhaseFilter(tmpDir, null, 'milestone-prefixed'); + }); + // 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' + 'a deprecation warning must be emitted when milestone-prefixed convention is set but roadmap is free-form' ); });