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

fix(#2641): recognize <details><summary> as active-milestone anchor

Browse Source 
`extractCurrentMilestone` only matched markdown headings (## v0.9, ### v0.9)
to find the active milestone slice. Projects that wrap their active
milestone's phase details inside `<details><summary>vX.Y …</summary>`
(a common GitHub-friendly collapse pattern, e.g. FAMP) fell through to
`stripShippedMilestones`, which strips ALL `<details>` blocks indiscriminately.

Net effect: `roadmapGetPhase` returned `{found:false}` for phases that ARE in
the active ROADMAP. The `init.phase-op` safety guard at `init.ts:133`
('drop archived disk match when phase is in current ROADMAP') depends on
`roadmapPhase.found`, so it didn't fire. `init.phase-op` then returned a
`phase_dir` pointing at an ARCHIVED milestone's same-numbered phase —
silently routing downstream workflows (e.g. /gsd-discuss-phase) into
completed phases.

Fix: when no markdown heading matches the active version, try matching
`<details\b[^>]*><summary>...vX.Y...</summary>`. Returns the inner content
of the matching block. Purely additive — `stripShippedMilestones` behavior
and its tests are unchanged.

The `\b[^>]*>` form tolerates attributes like `<details open>` or
`<details class="...">` (GitHub commonly emits `<details open>` for
default-expanded sections). Lazy `[\s\S]*?` matches up to the first
`</details>`; nested `<details>` inside the active milestone are not
expected and would mis-anchor (acceptable; falls through to the existing
`stripShippedMilestones` path with no regression vs. today's behavior).

Closes #2641. Distinct from the closed #2642 which bundled three orthogonal
changes (parser fix + checkbox-scan fix + STATE.md counting auth) into one
PR; this PR addresses only the parser anchoring bug, leaving
`stripShippedMilestones`, `roadmapAnalyze`, and `initMilestoneOp` untouched.

Tests added (3, all in `roadmap.test.ts`):
- `bug-2641: finds active milestone wrapped in <details><summary>vX.Y …</summary>`
- `bug-2641: finds active milestone in <details open><summary>vX.Y …</summary>`
- `bug-2641: returns found:true for phase inside <details>-wrapped active milestone` (end-to-end via `roadmapGetPhase`)

All existing `roadmap.test.ts` tests pass (39/39). Real-world repro
verified against an FAMP-style ROADMAP: before the fix,
`gsd-sdk query roadmap.get-phase 3` returned `{found:false}` despite the
phase being at line 113 of the active ROADMAP; after the fix, it returns
the correct phase metadata, and `init.phase-op 3` no longer returns the
v0.8 archived `phase_dir`.
This commit is contained in:
Ben Lamm
2026-05-02 22:56:00 -04:00
committed by Tom Boucher
parent 94f835af40
commit 592b676414
2 changed files with 143 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

109
sdk/src/query/roadmap.test.ts
View File

@@ -389,6 +389,80 @@ describe('extractCurrentMilestone', () => {
expect(result).toContain('### Phase 19: Security Audit');
});
// ─── Bug #2641: <details><summary>vX.Y …</summary> not recognized as anchor ───
it('bug-2641: finds active milestone wrapped in <details><summary>vX.Y …</summary>', async () => {
// Many projects (GitHub-friendly collapse) wrap the active milestone's
// phase details inside <details><summary>v0.9 …</summary>. Without the
// <details>-aware fallback, extractCurrentMilestone misses the heading
// anchor (because <summary> is HTML), falls through to
// stripShippedMilestones, and loses all <details> blocks — including
// the active one. Result: roadmapGetPhase returns {found:false} for
// phases that ARE in the active ROADMAP.
const roadmapWithActiveDetails = `# Roadmap
## Milestones
- ✅ **v0.8 Foundation** — shipped
- 📋 **v0.9 Local-First Bus** — active
## Phases
<details>
<summary>✅ v0.8 Foundation — SHIPPED 2026-04-15</summary>
### Phase 1: Old phase
**Goal:** Old goal.
</details>
<details>
<summary>v0.9 Local-First Bus (active) — Phase Details</summary>
### Phase 1: Library
**Goal:** Build the library.
### Phase 3: Polish
**Goal:** Add polish.
</details>
`;
const state = `---\nmilestone: v0.9\n---\n# State\n`;
await writeFile(join(tmpDir, '.planning', 'STATE.md'), state);
await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmapWithActiveDetails);
const result = await extractCurrentMilestone(roadmapWithActiveDetails, tmpDir);
// Active milestone's phases must survive
expect(result).toContain('### Phase 1: Library');
expect(result).toContain('### Phase 3: Polish');
expect(result).toContain('Add polish.');
// Shipped milestone phases must not bleed in
expect(result).not.toContain('Old phase');
});
// ─── Bug #2641: tolerate attributes on <details> tag (e.g. <details open>) ───
it('bug-2641: finds active milestone in <details open><summary>vX.Y …</summary>', async () => {
// GitHub auto-renders <details open> for sections that should default to
// expanded. The <details>-aware fallback regex must use <details\b[^>]*>
// (not literal <details>) so attribute-bearing tags also anchor correctly.
const roadmapWithDetailsOpen = `# Roadmap
## Phases
<details open>
<summary>v0.9 Local-First Bus (active) — Phase Details</summary>
### Phase 3: Polish
**Goal:** Add polish.
</details>
`;
const state = `---\nmilestone: v0.9\n---\n# State\n`;
await writeFile(join(tmpDir, '.planning', 'STATE.md'), state);
await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmapWithDetailsOpen);
const result = await extractCurrentMilestone(roadmapWithDetailsOpen, tmpDir);
expect(result).toContain('### Phase 3: Polish');
expect(result).toContain('Add polish.');
});
// ─── Bug #2422: same-version sub-heading truncation ───────────────────
it('bug-2422: does not truncate at same-version sub-heading (## v2.0 Phase Details)', async () => {
const roadmapWithDetails = `# ROADMAP
@@ -456,6 +530,41 @@ describe('roadmapGetPhase', () => {
expect(data.found).toBe(false);
expect(data.error).toBe('ROADMAP.md not found');
});
// ─── Bug #2641 (regression): end-to-end via roadmapGetPhase ───
it('bug-2641: returns found:true for phase inside <details>-wrapped active milestone', async () => {
// End-to-end coverage: roadmapGetPhase calls extractCurrentMilestone
// internally. Without the <details>-aware fallback, the active
// milestone's phases were stripped before the phase-heading lookup,
// and roadmapGetPhase returned {found:false} for phases that exist.
const roadmap = `# Roadmap
## Milestones
- 📋 **v0.9 Local-First Bus** — active
<details>
<summary>v0.9 Local-First Bus (active) — Phase Details</summary>
### Phase 3: Polish
**Goal:** Add polish.
**Success Criteria**:
1. Polish applied
2. Tests pass
</details>
`;
const state = `---\nmilestone: v0.9\n---\n# State\n`;
await writeFile(join(tmpDir, '.planning', 'STATE.md'), state);
await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmap);
const result = await roadmapGetPhase(['3'], tmpDir);
const data = result.data as Record<string, unknown>;
expect(data.found).toBe(true);
expect(data.phase_number).toBe('3');
expect(data.phase_name).toBe('Polish');
expect(data.goal).toBe('Add polish.');
});
});
// ─── roadmapAnalyze ───────────────────────────────────────────────────────

35
sdk/src/query/roadmap.ts
View File

@@ -181,7 +181,40 @@ export async function extractCurrentMilestone(content: string, projectDir: strin
);
const sectionMatch = content.match(sectionPattern);
if (!sectionMatch || sectionMatch.index === undefined) return stripShippedMilestones(content);
if (!sectionMatch || sectionMatch.index === undefined) {
// Fallback: <details><summary> matching the active version (issue #2641).
//
// Many projects (GitHub-friendly collapse pattern) wrap the active
// milestone's phase details inside a collapsible block whose <summary>
// names the version, e.g.:
//
// <details>
// <summary>v0.9 Local-First Bus (active) — Phase Details</summary>
// ### Phase 1: ...
// </details>
//
// The markdown-heading lookup above misses this because <summary> is HTML,
// not a heading. Without this fallback, control falls through to
// stripShippedMilestones() which removes ALL <details> blocks
// indiscriminately — including the active milestone's — causing
// roadmapGetPhase() to return {found:false} for phases that ARE in the
// active ROADMAP. The init.phase-op safety guard then misfires and can
// route phase lookups into archived milestones.
//
// <details\b[^>]*> tolerates attributes like <details open> and
// <details class="...">. The lazy [\s\S]*? terminates on the first
// </details>; nested <details> inside the active milestone are not
// expected and would mis-anchor (acceptable; FAMP-style ROADMAPs do not
// nest, and any project that does will fall through to the existing
// stripShippedMilestones path with no regression vs. today's behavior).
const detailsPattern = new RegExp(
`<details\\b[^>]*>\\s*<summary>[^<]*${escapedVersion}[^<]*</summary>([\\s\\S]*?)</details>`,
'i'
);
const detailsMatch = content.match(detailsPattern);
if (detailsMatch) return detailsMatch[1];
return stripShippedMilestones(content);
}
const sectionStart = sectionMatch.index;