fix(3600): count project-code-prefixed phase dirs in milestone filter
`init.new-milestone` reported `phase_dir_count: 0` for projects whose
phase directories carry a project_code prefix (`.planning/phases/CK-01-name`)
when the ROADMAP used numeric `### Phase N:` headings. Verified via a
temp-project repro that mirrors the reporter's setup.
Root cause: `getMilestonePhaseFilter` builds an `isDirInMilestone(dirName)`
predicate that tries two paths:
1) Numeric — requires the dir name to START with a digit. `CK-01-name`
starts with `C`, so this skips.
2) Custom-ID — captures the leading kebab token (`CK-01-name` as a
whole) and compares it to the normalised milestone phase IDs
(`{"1"}`). No match.
There was no path that stripped the project_code prefix before retrying
the numeric match. Added a third path that strips the same shape
`normalizePhaseName` already recognises (`^[A-Z]{1,6}-(?=\d)`) and retries
the numeric match. This runs AFTER the custom-ID path so a ROADMAP that
uses `### Phase PROJ-42:` continues to win via the custom-ID match for
a `PROJ-42` directory; the new branch only fires when the milestone is
keyed on the bare numeric form.
The fix lands in both:
- get-shit-done/bin/lib/core.cjs:isDirInMilestone (active CJS runtime)
- sdk/src/query/state.ts:isDirInMilestone (SDK twin)
`getMilestonePhaseFilter` is shared by multiple callers — init.new-milestone,
phase complete, verify-work, validate-health — so the fix benefits every
caller that walks `.planning/phases/` against a numeric ROADMAP.
Regression test
(tests/bug-3600-milestone-phase-filter-project-code-prefix.test.cjs):
1. Reporter's case: CK-01-name + CK-02-build dirs against Phase 1 / 2
headings → phase_dir_count === 2.
2. Existing contract: 01-first dir against Phase 1 heading still counts.
3. Custom-ID contract: PROJ-42 dir against `### Phase PROJ-42:` still
counts via the existing custom-ID match (no regression).
4. Counter-test: CK-99-backlog and CK-100-future dirs MUST NOT count
against a milestone with only Phase 1 — the strip-and-retry must
still respect the milestone's actual phase set.
All assertions go through `init new-milestone --json` (typed payload —
`phase_dir_count`). No raw text matching.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
5
.changeset/3600-milestone-phase-filter-project-code.md
Normal file
5
.changeset/3600-milestone-phase-filter-project-code.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
issue: 3600
|
||||
---
|
||||
**`init.new-milestone` now counts project-code-prefixed phase directories** — `getMilestonePhaseFilter` previously skipped `.planning/phases/CK-01-name` against a numeric ROADMAP heading like `### Phase 1:`: the numeric matcher required the directory name to start with a digit and the custom-ID matcher compared the full prefixed name against the bare milestone token. Added a strip-and-retry path that strips the same `^[A-Z]{1,6}-(?=\d)` prefix `normalizePhaseName` already recognises and retries the numeric match. The fix lands in both the CJS runtime (`get-shit-done/bin/lib/core.cjs:isDirInMilestone`) and the SDK twin (`sdk/src/query/state.ts:isDirInMilestone`), and is shared by every caller of `getMilestonePhaseFilter` — including `init.new-milestone`, `phase complete`, `verify-work`, and `validate-health`.
|
||||
@@ -1748,6 +1748,18 @@ function getMilestonePhaseFilter(cwd, versionOverride) {
|
||||
// Try custom ID match (e.g. PROJ-42-description → PROJ-42)
|
||||
const customMatch = dirName.match(/^([A-Za-z][A-Za-z0-9]*(?:-[A-Za-z0-9]+)*)/);
|
||||
if (customMatch && normalized.has(customMatch[1].toLowerCase())) return true;
|
||||
// #3600: project-code-prefixed directory (`CK-01-name`) against a
|
||||
// numeric ROADMAP heading (`### Phase 1:`). Strip the same prefix
|
||||
// shape `normalizePhaseName` recognises (`^[A-Z]{1,6}-(?=\d)`) and
|
||||
// retry the numeric match. This runs AFTER the custom-ID match so
|
||||
// a roadmap that uses `Phase PROJ-42:` continues to win via the
|
||||
// existing custom-ID path; the strip-and-retry only fires when the
|
||||
// milestone is keyed on the bare numeric form.
|
||||
const stripped = dirName.replace(/^[A-Z]{1,6}-(?=\d)/i, '');
|
||||
if (stripped !== dirName) {
|
||||
const sm = stripped.match(/^0*(\d+[A-Za-z]?(?:\.\d+)*)/);
|
||||
if (sm && normalized.has(sm[1].toLowerCase())) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
isDirInMilestone.phaseCount = milestonePhaseNums.size;
|
||||
|
||||
@@ -71,6 +71,18 @@ export async function getMilestonePhaseFilter(projectDir: string, workstream?: s
|
||||
// Try custom ID match
|
||||
const customMatch = dirName.match(/^([A-Za-z][A-Za-z0-9]*(?:-[A-Za-z0-9]+)*)/);
|
||||
if (customMatch && normalized.has(customMatch[1].toLowerCase())) return true;
|
||||
// #3600: project-code-prefixed directory (`CK-01-name`) against a
|
||||
// numeric ROADMAP heading (`### Phase 1:`). Strip the same prefix
|
||||
// shape `normalizePhaseName` recognises (`^[A-Z]{1,6}-(?=\d)`) and
|
||||
// retry the numeric match. This runs AFTER the custom-ID match so
|
||||
// a roadmap that uses `Phase PROJ-42:` continues to win via the
|
||||
// existing custom-ID path; the strip-and-retry only fires when the
|
||||
// milestone is keyed on the bare numeric form.
|
||||
const stripped = dirName.replace(/^[A-Z]{1,6}-(?=\d)/i, '');
|
||||
if (stripped !== dirName) {
|
||||
const sm = stripped.match(/^0*(\d+[A-Za-z]?(?:\.\d+)*)/);
|
||||
if (sm && normalized.has(sm[1].toLowerCase())) return true;
|
||||
}
|
||||
return false;
|
||||
}) as ((dirName: string) => boolean) & { phaseCount: number };
|
||||
|
||||
|
||||
@@ -0,0 +1,177 @@
|
||||
/**
|
||||
* Bug #3600: `init.new-milestone` reports `phase_dir_count: 0` for
|
||||
* project-code-prefixed phase directories (e.g. `.planning/phases/CK-01-name`)
|
||||
* against a ROADMAP that uses numeric `Phase 1:` headings.
|
||||
*
|
||||
* Root cause: `getMilestonePhaseFilter` builds an `isDirInMilestone(dirName)`
|
||||
* predicate that tries two paths:
|
||||
* 1) Numeric match — requires the directory name to START with a digit;
|
||||
* `CK-01-name` starts with `C`, so this path skips.
|
||||
* 2) Custom-ID match — captures the leading token (`CK-01-name` as a
|
||||
* whole) and compares it to the normalised milestone phase IDs
|
||||
* (`1`). No match.
|
||||
*
|
||||
* The predicate has no path that strips a project-code prefix before the
|
||||
* numeric match. This fix adds that third path: when both existing
|
||||
* matches fail, strip an optional `^[A-Z]{1,6}-(?=\d)` prefix (the same
|
||||
* shape `normalizePhaseName` already strips) and retry the numeric match.
|
||||
*
|
||||
* The fix is shared between the CJS impl in `core.cjs` and the SDK twin
|
||||
* in `sdk/src/query/state.ts`. The behavioural test exercises the CJS
|
||||
* surface (the active runtime).
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
process.env.GSD_TEST_MODE = '1';
|
||||
|
||||
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');
|
||||
|
||||
function writeRoadmap(tmpDir, body) {
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), body);
|
||||
}
|
||||
function writeState(tmpDir, version) {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'STATE.md'),
|
||||
`---\nmilestone: ${version}\n---\n`,
|
||||
);
|
||||
}
|
||||
function writeConfig(tmpDir, configObj) {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'config.json'),
|
||||
JSON.stringify(configObj, null, 2),
|
||||
);
|
||||
}
|
||||
function ensurePhaseDir(tmpDir, name) {
|
||||
fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', name), { recursive: true });
|
||||
}
|
||||
|
||||
describe('bug #3600: milestone phase filter understands project-code-prefixed directories', () => {
|
||||
let tmpDir;
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempProject('bug-3600-');
|
||||
});
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('init.new-milestone counts CK-NN-name dirs against numeric `Phase N:` headings', () => {
|
||||
writeConfig(tmpDir, { project_code: 'CK' });
|
||||
writeState(tmpDir, 'v1.0.0');
|
||||
writeRoadmap(
|
||||
tmpDir,
|
||||
[
|
||||
'# Roadmap',
|
||||
'',
|
||||
'## Current Milestone: v1.0.0 - Test',
|
||||
'',
|
||||
'### Phase 1: Discovery',
|
||||
'**Goal:** GoalOne',
|
||||
'',
|
||||
'### Phase 2: Build',
|
||||
'**Goal:** GoalTwo',
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
ensurePhaseDir(tmpDir, 'CK-01-discovery');
|
||||
ensurePhaseDir(tmpDir, 'CK-02-build');
|
||||
|
||||
const r = runGsdTools(['init', 'new-milestone', '--json'], tmpDir);
|
||||
assert.ok(r.success, `init new-milestone failed: ${r.error || r.output}`);
|
||||
const payload = JSON.parse(r.output);
|
||||
assert.strictEqual(
|
||||
payload.phase_dir_count,
|
||||
2,
|
||||
`expected phase_dir_count=2 for two CK-NN-name dirs against Phase 1/Phase 2 headings, got ${payload.phase_dir_count}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('unprefixed directories continue to count (#3537 / existing contract)', () => {
|
||||
writeState(tmpDir, 'v1.0.0');
|
||||
writeRoadmap(
|
||||
tmpDir,
|
||||
[
|
||||
'# Roadmap',
|
||||
'',
|
||||
'## Current Milestone: v1.0.0 - Test',
|
||||
'',
|
||||
'### Phase 1: First',
|
||||
'**Goal:** g',
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
ensurePhaseDir(tmpDir, '01-first');
|
||||
|
||||
const r = runGsdTools(['init', 'new-milestone', '--json'], tmpDir);
|
||||
assert.ok(r.success);
|
||||
const payload = JSON.parse(r.output);
|
||||
assert.strictEqual(payload.phase_dir_count, 1);
|
||||
});
|
||||
|
||||
test('custom-ID match for PROJ-42 directory + Phase PROJ-42: heading still works', () => {
|
||||
// Existing custom-ID path: directory name exactly equals the custom
|
||||
// phase ID (no slug suffix). The PROJ-42 → Phase PROJ-42: match must
|
||||
// continue to fire via the second branch of `isDirInMilestone`, not
|
||||
// get pre-empted by the new strip-and-retry branch.
|
||||
writeConfig(tmpDir, { project_code: 'PROJ' });
|
||||
writeState(tmpDir, 'v1.0.0');
|
||||
writeRoadmap(
|
||||
tmpDir,
|
||||
[
|
||||
'# Roadmap',
|
||||
'',
|
||||
'## Current Milestone: v1.0.0 - Test',
|
||||
'',
|
||||
'### Phase PROJ-42: Custom',
|
||||
'**Goal:** g',
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
ensurePhaseDir(tmpDir, 'PROJ-42');
|
||||
|
||||
const r = runGsdTools(['init', 'new-milestone', '--json'], tmpDir);
|
||||
assert.ok(r.success);
|
||||
const payload = JSON.parse(r.output);
|
||||
assert.strictEqual(
|
||||
payload.phase_dir_count,
|
||||
1,
|
||||
'PROJ-42 directory must still match Phase PROJ-42: via the custom-ID path',
|
||||
);
|
||||
});
|
||||
|
||||
test('directories that do not match the milestone do NOT count', () => {
|
||||
// Counter-test: a 999-backlog directory or a totally-unrelated phase
|
||||
// must NOT be counted in the milestone tally.
|
||||
writeConfig(tmpDir, { project_code: 'CK' });
|
||||
writeState(tmpDir, 'v1.0.0');
|
||||
writeRoadmap(
|
||||
tmpDir,
|
||||
[
|
||||
'# Roadmap',
|
||||
'',
|
||||
'## Current Milestone: v1.0.0 - Test',
|
||||
'',
|
||||
'### Phase 1: First',
|
||||
'**Goal:** g',
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
ensurePhaseDir(tmpDir, 'CK-01-first');
|
||||
// Future / backlog phases that should not be counted in this milestone.
|
||||
ensurePhaseDir(tmpDir, 'CK-99-backlog');
|
||||
ensurePhaseDir(tmpDir, 'CK-100-future');
|
||||
|
||||
const r = runGsdTools(['init', 'new-milestone', '--json'], tmpDir);
|
||||
assert.ok(r.success);
|
||||
const payload = JSON.parse(r.output);
|
||||
assert.strictEqual(
|
||||
payload.phase_dir_count,
|
||||
1,
|
||||
'only CK-01-first should match Phase 1; CK-99 and CK-100 must be excluded',
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user