golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(3815): phase.insert handles checked-bullet ROADMAP format (#79)

Browse Source 
* fix(3815): phase.insert parser handles checked-bullet ROADMAP format

phaseInsert (TS) and cmdPhaseInsert (CJS) previously used a heading-only
regex (#{2,4}\s*Phase\s+N:) to locate the target phase.  On projects whose
ROADMAP uses the checked-bullet format (- [ ] **Phase N: name** or
- [ ] Phase N: name), the lookup always failed with "Phase N not found".

Extend the locator to also accept the bullet form — mirroring the patterns
already used by phaseRemove and phaseComplete.  When bullet-style is
detected, insert a new bullet entry after the matched line (preserving
bold/plain style to match surrounding entries).  The heading-style code
path is unchanged.

Also fix a pre-existing test timeout: the first registry-integration test in
phase-lifecycle.test.ts was failing with STACK_TRACE_ERROR (masked timeout)
because the cold import of index.js takes >5 s.  Added { timeout: 30_000 }.

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

* fix(3815): refine hybrid-ROADMAP detection, preserve #3098 parity

Tighten the bullet-style branch guard: only treat a ROADMAP as
bullet-style (and apply the bullet-insert path) when it contains
ZERO heading-style phase entries (anyHeadingPattern test).  A
mixed (hybrid) ROADMAP — headings for some phases, bullet summaries
for others — is the #3098 case where the detail section is absent;
that path must still error with "missing a detail section".

Adds a regression test (#3098 preserved) in both TS and CJS to
confirm that a heading-style ROADMAP with a bullet-only entry for
the target phase still fires the "missing a detail section" error,
not the bullet-insert path.

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

* chore: add changeset for #3815 phase.insert bullet-roadmap fix

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-22 11:39:45 -04:00
committed by GitHub
parent 619b5c42e3
commit 4a19d4db2b
5 changed files with 288 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/3815-phase-insert-bullet-roadmap.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3815
---
**`phase.insert` now handles checked-bullet ROADMAP format** — `gsd-sdk query phase.insert` (and `gsd-tools phase insert`) previously threw "Phase N not found" on ROADMAPs that use the `- [ ] **Phase N: name**` checklist format instead of `### Phase N: name` headings. The parser now detects purely bullet-style ROADMAPs and inserts the new decimal phase entry between the target bullet and the next phase bullet (preserving bold vs plain formatting). Hybrid ROADMAPs that mix heading-style phases with bullet summaries continue to produce the "missing a detail section" error from #3098, since a bullet-only entry in that context means the `### Phase N:` detail section is absent. (#3815)

102
get-shit-done/bin/lib/phase.cjs
View File

@@ -759,8 +759,32 @@ function cmdPhaseInsert(cwd, afterPhase, description, raw) {
const normalizedAfter = normalizePhaseName(afterPhase);
const afterPhaseEscaped = phaseMarkdownRegexSource(normalizedAfter);
const targetPattern = new RegExp(`#{2,4}\\s*Phase\\s+${afterPhaseEscaped}:`, 'i');
if (!targetPattern.test(content)) {
const checklistPattern = new RegExp(`-\\s*\\[[ x]\\]\\s*\\*\\*Phase\\s+${afterPhaseEscaped}:`, 'i');
const headingMatch = targetPattern.test(content);
// #3815: also recognise the checked-bullet phase format used by projects
// that list phases as `- [ ] **Phase N: name**` or `- [ ] Phase N: name`
// (both bold and plain variants). Mirrors phaseRemove / phaseComplete.
//
// Bullet-style only activates when there are NO heading-style phases in the
// milestone content. A bullet entry in a hybrid (headings + bullets) ROADMAP
// means the detail section is missing — that is the #3098 case and must keep
// producing the "missing a detail section" error.
const bulletPattern = new RegExp(
`-\\s*\\[[ x]\\]\\s*(?:\\*\\*)?Phase\\s+${afterPhaseEscaped}[:\\s]`,
'i',
);
const anyHeadingPattern = /#{2,4}\s*Phase\s+\d/i;
const roadmapHasHeadingPhases = anyHeadingPattern.test(content);
const isBulletStyle = !headingMatch && bulletPattern.test(content) && !roadmapHasHeadingPhases;
if (!headingMatch && !isBulletStyle) {
// Bug #3098 parity: when the ROADMAP uses heading-style phases and only
// the summary checklist exists for this phase (no `### Phase N:` detail
// section), point the user at the missing detail section.
const checklistPattern = new RegExp(
`-\\s*\\[[ x]\\]\\s*(?:\\*\\*)?Phase\\s+${afterPhaseEscaped}[:\\s]`,
'i',
);
if (checklistPattern.test(content)) {
error(`Phase ${afterPhase} exists in roadmap summary but is missing a detail section (### Phase ${afterPhase}: ...).`);
}
@@ -806,28 +830,70 @@ function cmdPhaseInsert(cwd, afterPhase, description, raw) {
platformEnsureDir(dirPath);
platformWriteSync(path.join(dirPath, '.gitkeep'), '');
// Build phase entry
const phaseEntry = `\n### Phase ${_decimalPhase}: ${description} (INSERTED)\n\n**Goal:** [Urgent work - to be planned]\n**Requirements**: TBD\n**Depends on:** Phase ${afterPhase}\n**Plans:** 0 plans\n\nPlans:\n- [ ] TBD (run ${formatGsdSlash('plan-phase', resolveRuntime(cwd))} ${_decimalPhase} to break down)\n`;
let updatedContent;
// Insert after the target phase section
const headerPattern = new RegExp(`(#{2,4}\\s*Phase\\s+${afterPhaseEscaped}:[^\\n]*\\n)`, 'i');
const headerMatch = rawContent.match(headerPattern);
if (!headerMatch) {
error(`Could not find Phase ${afterPhase} header`);
}
if (isBulletStyle) {
// #3815: Insert in checked-bullet format, mirroring the style of the
// surrounding entries. Detect whether the matched bullet uses bold
// (`**Phase N: …**`) to preserve file-internal format consistency.
const boldBulletPattern = new RegExp(
`-\\s*\\[[ x]\\]\\s*\\*\\*Phase\\s+${afterPhaseEscaped}:`,
'i',
);
const useBold = boldBulletPattern.test(content);
const phaseLabel = useBold
? `**Phase ${_decimalPhase}: ${description}**`
: `Phase ${_decimalPhase}: ${description}`;
const bulletEntry = `\n- [ ] ${phaseLabel}`;
const headerIdx = rawContent.indexOf(headerMatch[0]);
const afterHeader = rawContent.slice(headerIdx + headerMatch[0].length);
const nextPhaseMatch = afterHeader.match(/\n#{2,4}\s+Phase\s+\d/i);
// Locate the target bullet line in the raw content
const targetBulletPattern = new RegExp(
`(-\\s*\\[[ x]\\]\\s*(?:\\*\\*)?Phase\\s+${afterPhaseEscaped}[:\\s][^\\n]*)`,
'i',
);
const bulletMatchResult = rawContent.match(targetBulletPattern);
if (!bulletMatchResult) {
error(`Could not find Phase ${afterPhase} bullet line`);
}
let insertIdx;
if (nextPhaseMatch) {
insertIdx = headerIdx + headerMatch[0].length + nextPhaseMatch.index;
const bulletLineEnd = rawContent.indexOf(bulletMatchResult[0]) + bulletMatchResult[0].length;
const afterBullet = rawContent.slice(bulletLineEnd);
const nextBulletMatch = afterBullet.match(/\n-\s*\[[ x]\]\s*(?:\*\*)?Phase\s+\d/i);
let insertIdx;
if (nextBulletMatch) {
insertIdx = bulletLineEnd + nextBulletMatch.index;
} else {
insertIdx = bulletLineEnd;
}
updatedContent = rawContent.slice(0, insertIdx) + bulletEntry + rawContent.slice(insertIdx);
} else {
insertIdx = rawContent.length;
// Heading-style insert (original path)
// Build phase entry
const phaseEntry = `\n### Phase ${_decimalPhase}: ${description} (INSERTED)\n\n**Goal:** [Urgent work - to be planned]\n**Requirements**: TBD\n**Depends on:** Phase ${afterPhase}\n**Plans:** 0 plans\n\nPlans:\n- [ ] TBD (run ${formatGsdSlash('plan-phase', resolveRuntime(cwd))} ${_decimalPhase} to break down)\n`;
// Insert after the target phase section
const headerPattern = new RegExp(`(#{2,4}\\s*Phase\\s+${afterPhaseEscaped}:[^\\n]*\\n)`, 'i');
const headerMatch = rawContent.match(headerPattern);
if (!headerMatch) {
error(`Could not find Phase ${afterPhase} header`);
}
const headerIdx = rawContent.indexOf(headerMatch[0]);
const afterHeader = rawContent.slice(headerIdx + headerMatch[0].length);
const nextPhaseMatch = afterHeader.match(/\n#{2,4}\s+Phase\s+\d/i);
let insertIdx;
if (nextPhaseMatch) {
insertIdx = headerIdx + headerMatch[0].length + nextPhaseMatch.index;
} else {
insertIdx = rawContent.length;
}
updatedContent = rawContent.slice(0, insertIdx) + phaseEntry + rawContent.slice(insertIdx);
}
const updatedContent = rawContent.slice(0, insertIdx) + phaseEntry + rawContent.slice(insertIdx);
platformWriteSync(roadmapPath, updatedContent);
return { decimalPhase: _decimalPhase, dirName: _dirName };
});

124
sdk/src/query/phase-lifecycle.test.ts
View File

@@ -705,6 +705,123 @@ describe('phaseInsert', () => {
await expect(phaseInsert([], tmpDir)).rejects.toThrow('after-phase and description required');
});
it('#3098 preserved: throws when bullet-only entry in a heading-style ROADMAP (hybrid = missing detail section)', async () => {
// A hybrid ROADMAP: phase 9 has a heading but phase 10 only has a bullet
// summary entry. Insert must still reject with "missing a detail section"
// because the surrounding ROADMAP uses heading-style phases.
const { phaseInsert } = await import('./phase-lifecycle.js');
const hybridRoadmap = `# Roadmap\n\n## Current Milestone\n\n### Phase 9: Foundation\n\n- [ ] **Phase 10: Queries**\n`;
await setupTestProject(tmpDir, {
roadmap: hybridRoadmap,
phases: ['09-foundation'],
});
await expect(phaseInsert(['10', 'Hotfix'], tmpDir)).rejects.toThrow('missing a detail section');
});
// ─── #3815: checked-bullet ROADMAP format ─────────────────────────────
const BULLET_ONLY_ROADMAP = `# Roadmap
## Current Milestone: v2.0 Agent Skills
- [x] **Phase 01: bootstrap** — completed 2026-04-01
- [ ] **Phase 02: core-loop**
- [ ] **Phase 03: persistence**
---
*Last updated: 2026-05-01*
`;
it('#3815: inserts decimal phase between bullets in a checked-bullet ROADMAP', async () => {
const { phaseInsert } = await import('./phase-lifecycle.js');
await setupTestProject(tmpDir, {
roadmap: BULLET_ONLY_ROADMAP,
phases: ['01-bootstrap', '02-core-loop', '03-persistence'],
});
const result = await phaseInsert(['02', 'hot patch'], tmpDir);
const data = result.data as Record<string, unknown>;
expect(data.phase_number).toBe('02.1');
expect(data.after_phase).toBe('02');
expect(data.name).toBe('hot patch');
const roadmap = await readFile(join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
// New bullet must be present
expect(roadmap).toMatch(/- \[ \].*Phase 02\.1:.*hot patch/i);
// New bullet must appear AFTER the Phase 02 bullet
const phase02Idx = roadmap.indexOf('Phase 02: core-loop');
const insertedIdx = roadmap.search(/Phase 02\.1:/i);
expect(phase02Idx).toBeGreaterThanOrEqual(0);
expect(insertedIdx).toBeGreaterThan(phase02Idx);
// New bullet must appear BEFORE the Phase 03 bullet (positional assertion)
const phase03Idx = roadmap.indexOf('Phase 03: persistence');
expect(insertedIdx).toBeLessThan(phase03Idx);
// Must NOT corrupt the surrounding bullet structure
expect(roadmap).toContain('- [x] **Phase 01: bootstrap**');
expect(roadmap).toContain('- [ ] **Phase 02: core-loop**');
expect(roadmap).toContain('- [ ] **Phase 03: persistence**');
});
it('#3815: inserts at end of bullet list when target is the last phase', async () => {
const { phaseInsert } = await import('./phase-lifecycle.js');
await setupTestProject(tmpDir, {
roadmap: BULLET_ONLY_ROADMAP,
phases: ['01-bootstrap', '02-core-loop', '03-persistence'],
});
const result = await phaseInsert(['03', 'final extra'], tmpDir);
const data = result.data as Record<string, unknown>;
expect(data.phase_number).toBe('03.1');
const roadmap = await readFile(join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
expect(roadmap).toMatch(/- \[ \].*Phase 03\.1:.*final extra/i);
// Must appear after Phase 03 bullet
const phase03Idx = roadmap.indexOf('Phase 03: persistence');
const insertedIdx = roadmap.search(/Phase 03\.1:/i);
expect(insertedIdx).toBeGreaterThan(phase03Idx);
});
it('#3815: plain bullet format (no bold) also works', async () => {
const { phaseInsert } = await import('./phase-lifecycle.js');
const PLAIN_BULLET_ROADMAP = `# Roadmap
## Current Milestone: v2.0
- [ ] Phase 01: foo
- [ ] Phase 02: bar
- [ ] Phase 03: baz
---
*Last updated: 2026-05-01*
`;
await setupTestProject(tmpDir, {
roadmap: PLAIN_BULLET_ROADMAP,
phases: ['01-foo', '02-bar', '03-baz'],
});
const result = await phaseInsert(['02', 'inserted'], tmpDir);
const data = result.data as Record<string, unknown>;
expect(data.phase_number).toBe('02.1');
const roadmap = await readFile(join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
expect(roadmap).toMatch(/- \[ \].*Phase 02\.1:.*inserted/i);
const phase02Idx = roadmap.indexOf('Phase 02: bar');
const insertedIdx = roadmap.search(/Phase 02\.1:/i);
const phase03Idx = roadmap.indexOf('Phase 03: baz');
expect(insertedIdx).toBeGreaterThan(phase02Idx);
expect(insertedIdx).toBeLessThan(phase03Idx);
});
});
// ─── phaseScaffold ──────────────────────────────────────────────────────
@@ -1599,6 +1716,11 @@ describe('milestoneComplete help-flag defense', () => {
// ─── Registry integration ──────────────────────────────────────────────────
describe('lifecycle handlers in registry', () => {
// Registry assembly (invariant checks + large handler map) routinely takes
// 5–10 s on a cold ESM module cache. The second test below reuses the
// cached import and completes instantly, but the first import is the cold
// path. Raise the per-test timeout so the warm-up doesn't surface as a
// spurious "Test timed out" failure (pre-existing: STACK_TRACE_ERROR mask).
it('registers all 7 lifecycle handlers with dot notation', async () => {
const { createRegistry } = await import('./index.js');
const registry = createRegistry();
@@ -1612,7 +1734,7 @@ describe('lifecycle handlers in registry', () => {
const handler = registry.getHandler(cmd);
expect(handler, `${cmd} should be registered`).toBeDefined();
}
});
}, 30_000);
it('registers space-delimited aliases', async () => {
const { createRegistry } = await import('./index.js');

72
sdk/src/query/phase-lifecycle.ts
View File

@@ -409,12 +409,34 @@ export const phaseInsert: QueryHandler = async (args, projectDir, workstream) =>
const unpadded = normalizedAfter.replace(/^0+/, '');
const afterPhaseEscaped = unpadded.replace(/\./g, '\\.');
const targetPattern = new RegExp(`#{2,4}\\s*Phase\\s+0*${afterPhaseEscaped}:`, 'i');
if (!targetPattern.test(content)) {
// Bug #3098 parity: when only the summary checklist exists for this
// phase (no `### Phase N:` detail section), point the user at the
// missing detail section rather than implying the phase is absent.
const headingMatch = targetPattern.test(content);
// #3815: also recognise the checked-bullet phase format used by projects
// that list phases as `- [ ] **Phase N: name**` or `- [ ] Phase N: name`
// (both bold and plain variants). This mirrors the patterns already used
// by phaseRemove / phaseComplete so insert is consistent with its siblings.
//
// Bullet-style is only used when there are NO heading-style phases at all in
// the milestone content. If the ROADMAP mixes headings + bullets (hybrid
// format), a bullet-only match means the detail section is missing — that
// is the #3098 case and must continue to produce the "missing a detail
// section" error. Only a purely bullet-style ROADMAP (zero heading-style
// phase entries in the milestone) goes through the bullet insert path.
const bulletPattern = new RegExp(
`-\\s*\\[[ x]\\]\\s*(?:\\*\\*)?Phase\\s+0*${afterPhaseEscaped}[:\\s]`,
'i',
);
const anyHeadingPattern = /#{2,4}\s*Phase\s+\d/i;
const roadmapHasHeadingPhases = anyHeadingPattern.test(content);
const isBulletStyle = !headingMatch && bulletPattern.test(content) && !roadmapHasHeadingPhases;
if (!headingMatch && !isBulletStyle) {
// Bug #3098 parity: when the ROADMAP uses heading-style phases and only
// the summary checklist exists for this phase (no `### Phase N:` detail
// section), point the user at the missing detail section rather than
// implying the phase is absent.
const checklistPattern = new RegExp(
`-\\s*\\[[ x]\\]\\s*\\*\\*Phase\\s+0*${afterPhaseEscaped}:`,
`-\\s*\\[[ x]\\]\\s*(?:\\*\\*)?Phase\\s+0*${afterPhaseEscaped}[:\\s]`,
'i',
);
if (checklistPattern.test(content)) {
@@ -459,6 +481,46 @@ export const phaseInsert: QueryHandler = async (args, projectDir, workstream) =>
// Create directory with .gitkeep
await ensureDirectoryWithGitkeep(dirPath);
if (isBulletStyle) {
// #3815: Insert in checked-bullet format, mirroring the style of the
// surrounding entries. Detect whether the matched bullet uses bold
// (`**Phase N: …**`) to preserve file-internal format consistency.
const boldBulletPattern = new RegExp(
`-\\s*\\[[ x]\\]\\s*\\*\\*Phase\\s+0*${afterPhaseEscaped}:`,
'i',
);
const useBold = boldBulletPattern.test(content);
const phaseLabel = useBold
? `**Phase ${decimalPhase}: ${description}**`
: `Phase ${decimalPhase}: ${description}`;
const bulletEntry = `\n- [ ] ${phaseLabel}`;
// Locate the target bullet line in the raw content
const targetBulletPattern = new RegExp(
`(-\\s*\\[[ x]\\]\\s*(?:\\*\\*)?Phase\\s+0*${afterPhaseEscaped}[:\\s][^\\n]*)`,
'i',
);
const bulletMatchResult = rawContent.match(targetBulletPattern);
if (!bulletMatchResult) {
throw new GSDError(`Could not find Phase ${afterPhase} bullet line`, ErrorClassification.Execution);
}
const bulletLineEnd = rawContent.indexOf(bulletMatchResult[0]) + bulletMatchResult[0].length;
// Find where the next phase bullet starts (or use end of content)
const afterBullet = rawContent.slice(bulletLineEnd);
const nextBulletMatch = afterBullet.match(/\n-\s*\[[ x]\]\s*(?:\*\*)?Phase\s+\d/i);
let insertIdx: number;
if (nextBulletMatch && nextBulletMatch.index !== undefined) {
insertIdx = bulletLineEnd + nextBulletMatch.index;
} else {
insertIdx = bulletLineEnd;
}
return rawContent.slice(0, insertIdx) + bulletEntry + rawContent.slice(insertIdx);
}
// Heading-style insert (original path)
// Build phase entry
const phaseEntry = `\n### Phase ${decimalPhase}: ${description} (INSERTED)\n\n**Goal:** [Urgent work - to be planned]\n**Requirements**: TBD\n**Depends on:** Phase ${afterPhase}\n**Plans:** 0 plans\n\nPlans:\n- [ ] TBD (run /gsd-plan-phase ${decimalPhase} to break down)\n`;

11
tests/phase.test.cjs
View File

@@ -1357,13 +1357,20 @@ describe('phase insert command', () => {
});
test('reports actionable error for summary-only placeholder phase without detail section (#3098)', () => {
// #3098: a hybrid ROADMAP that has heading-style phases for some phases
// but only a bullet summary entry for phase 5 (the detail section is
// missing). Insert must fail with "missing a detail section" rather than
// silently inserting in bullet-style — because the surrounding ROADMAP
// uses headings, so the absent `### Phase 5:` is a genuine omission.
// (Compare with the #3815 case below: a purely bullet-style ROADMAP that
// has NO heading-style phases at all is valid and insert should succeed.)
fs.writeFileSync(
path.join(tmpDir, '.planning', 'ROADMAP.md'),
`# Roadmap\n\n- [ ] **Phase 5: Placeholder**\n`
`# Roadmap\n\n### Phase 4: Foundation\n**Goal:** Setup\n\n- [ ] **Phase 5: Placeholder**\n`
);
const result = runGsdTools('phase insert 5 Hotfix', tmpDir);
assert.ok(!result.success, 'should fail when phase is summary-only placeholder');
assert.ok(!result.success, 'should fail when phase is summary-only placeholder in a heading-style ROADMAP');
assert.ok(result.error.includes('missing a detail section'));
});