diff --git a/.changeset/fix-3-milestone-complete-noise.md b/.changeset/fix-3-milestone-complete-noise.md new file mode 100644 index 000000000..979b4f38a --- /dev/null +++ b/.changeset/fix-3-milestone-complete-noise.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 146 +--- +milestone.complete no longer produces a doubled version in the MILESTONES.md header (`v1.0 v1.0`), leaks phases declared via checkbox bullets into the "all phases" fallback, or extracts one-liner text from sections beyond the first heading boundary. (#3) diff --git a/sdk/src/query/phase-lifecycle-policy.ts b/sdk/src/query/phase-lifecycle-policy.ts index 4232d27e5..b1db23ed6 100644 --- a/sdk/src/query/phase-lifecycle-policy.ts +++ b/sdk/src/query/phase-lifecycle-policy.ts @@ -51,11 +51,32 @@ export function parseMultiwordArg(args: string[], flag: string): string | null { return tokens.length > 0 ? tokens.join(' ') : null; } +/** + * Extract a one-liner from the summary body when it is not in frontmatter. + * + * Scope: content from the first heading to the next heading of ANY level. + * Per GFM ATX headings (https://github.github.com/gfm/#atx-headings), + * a heading of any level terminates the scope -- including sub-headings + * even when the title heading is H1. + * + * #3 defect-3: the original regex matched the first bold in the entire body + * after any heading, leaking bold text from later sections (e.g. deviation + * entries) into the one-liner. + */ export function extractOneLinerFromBody(content: string): string | null { if (!content) return null; const body = content.replace(/^---\r?\n[\s\S]*?\r?\n---\r?\n*/, ''); - const match = body.match(/^#[^\n]*\n+\*\*([^*]+)\*\*/m); - return match ? match[1]!.trim() : null; + // Find the first heading of any level (GFM section ATX headings) + const headingMatch = body.match(/^(#{1,6}\s[^\n]+\n)/m); + if (!headingMatch || headingMatch.index === undefined) return null; + const afterHeading = body.slice(headingMatch.index + headingMatch[0].length); + // Bound to the first section: truncate at the next heading of any level + const nextHeadingMatch = afterHeading.match(/^#{1,6}\s/m); + const sectionScope = nextHeadingMatch && nextHeadingMatch.index !== undefined + ? afterHeading.slice(0, nextHeadingMatch.index) + : afterHeading; + const boldMatch = sectionScope.match(/\*\*([^*]+)\*\*/); + return boldMatch ? boldMatch[1]!.trim() : null; } /** diff --git a/sdk/src/query/phase-lifecycle.test.ts b/sdk/src/query/phase-lifecycle.test.ts index 44f8221b5..d5a4a7d2c 100644 --- a/sdk/src/query/phase-lifecycle.test.ts +++ b/sdk/src/query/phase-lifecycle.test.ts @@ -1969,3 +1969,132 @@ describe('collectDecimalSuffixesFromDirNames — CR-3267 finding 3: alphanumeric expect(result.has(2)).toBe(true); }); }); + +// ─── #3 defect-1: milestone.complete header duplication ─────────────────── +// +// Historical evidence: `## v1.8.0 Quick Mode (Shipped: 2026-01-19)` +// Canonical template: `## ${version}${nameOpt ? ` ${nameOpt}` : ''} (Shipped: ${today})` +// GFM § ATX headings: https://github.github.com/gfm/#atx-headings + +describe('milestoneComplete (#3 defect-1): header version/name rendering', () => { + it('no --name: header is "## v1.0 (Shipped: )" — version appears exactly once', async () => { + const { milestoneComplete } = await import('./phase-lifecycle.js'); + const today = new Date().toISOString().split('T')[0]!; + const roadmap = `# Roadmap\n\n## Current Milestone: v1.0\n\n### Phase 1: Foundation\n\n**Goal:** TBD\n**Plans:** 1 plan\n\nPlans:\n- [ ] 01-01\n`; + const state = `---\ngsd_state_version: 1.0\nmilestone: v1.0\nstatus: executing\n---\n\n# Project State\n\n## Current Position\n\nPhase: 1\nStatus: Executing\n`; + await setupTestProject(tmpDir, { roadmap, state }); + const milestonesPath = join(tmpDir, '.planning', 'MILESTONES.md'); + await writeFile(milestonesPath, '# Milestones\n\n', 'utf-8'); + await mkdir(join(tmpDir, '.planning', 'phases', '01-foundation'), { recursive: true }); + + await milestoneComplete(['v1.0'], tmpDir); + + const content = await readFile(milestonesPath, 'utf-8'); + // Must match exactly "## v1.0 (Shipped: )" — version must NOT be doubled + expect(content).toContain(`## v1.0 (Shipped: ${today})`); + expect(content).not.toMatch(/## v1\.0 v1\.0/); + }); + + it('with --name "Foundation Release": header is "## v1.0 Foundation Release (Shipped: )"', async () => { + const { milestoneComplete } = await import('./phase-lifecycle.js'); + const today = new Date().toISOString().split('T')[0]!; + const roadmap = `# Roadmap\n\n## Current Milestone: v1.0\n\n### Phase 1: Foundation\n\n**Goal:** TBD\n**Plans:** 1 plan\n\nPlans:\n- [ ] 01-01\n`; + const state = `---\ngsd_state_version: 1.0\nmilestone: v1.0\nstatus: executing\n---\n\n# Project State\n\n## Current Position\n\nPhase: 1\nStatus: Executing\n`; + await setupTestProject(tmpDir, { roadmap, state }); + const milestonesPath = join(tmpDir, '.planning', 'MILESTONES.md'); + await writeFile(milestonesPath, '# Milestones\n\n', 'utf-8'); + await mkdir(join(tmpDir, '.planning', 'phases', '01-foundation'), { recursive: true }); + + await milestoneComplete(['v1.0', '--name', 'Foundation', 'Release'], tmpDir); + + const content = await readFile(milestonesPath, 'utf-8'); + // version + name appended: "## v1.0 Foundation Release (Shipped: ...)" + expect(content).toContain(`## v1.0 Foundation Release (Shipped: ${today})`); + expect(content).not.toMatch(/## v1\.0 v1\.0/); + }); +}); + +// ─── #3 defect-2: bullet-declared phases leak phase 99 into milestone ───── +// +// ROADMAP declares phases via checkbox bullets ONLY (GFM task list items: +// https://github.github.com/gfm/#task-list-items-extension-). +// When there are no `### Phase N:` headings in the milestone section, +// getMilestonePhaseFilter falls back to passAll (milestonePhaseNums.size === 0), +// admitting unrelated phase dirs like 99-unrelated. +// The fix: extend the phase-extraction regex to also match checkbox-bullet +// declarations so passAll is not triggered spuriously. + +describe('milestoneComplete (#3 defect-2): checkbox-bullet phase declarations filter correctly', () => { + it('collects only phases 1 and 2 from bullet-only declarations — never phase 99', async () => { + const { milestoneComplete } = await import('./phase-lifecycle.js'); + // Roadmap declares phases via GFM task-list bullets ONLY — no ### Phase N: + // headings exist anywhere in the document. This is the exact shape that + // triggers passAll in the current code (milestonePhaseNums.size === 0). + const roadmap = `# Roadmap + +## Current Milestone: v1.0 Foundation + +- [ ] **Phase 1: Foundation** +- [ ] **Phase 2: API** + +--- +*Last updated: 2026-01-01* +`; + const state = `---\ngsd_state_version: 1.0\nmilestone: v1.0\nstatus: executing\n---\n\n# Project State\n\n## Current Position\n\nPhase: 1\nStatus: Executing\n`; + await setupTestProject(tmpDir, { roadmap, state }); + + const phasesDir = join(tmpDir, '.planning', 'phases'); + await mkdir(join(phasesDir, '01-foundation'), { recursive: true }); + await mkdir(join(phasesDir, '02-api'), { recursive: true }); + await mkdir(join(phasesDir, '99-unrelated'), { recursive: true }); + + const summaryContent = `# Phase Summary\n\n**Shipped the feature.**\n`; + await writeFile(join(phasesDir, '01-foundation', 'SUMMARY.md'), summaryContent, 'utf-8'); + await writeFile(join(phasesDir, '02-api', 'SUMMARY.md'), summaryContent, 'utf-8'); + // Phase 99 has a summary too — should NOT appear in accomplishments + await writeFile(join(phasesDir, '99-unrelated', 'SUMMARY.md'), '# Phase Summary\n\n**Should NOT appear.**\n', 'utf-8'); + + const milestonesPath = join(tmpDir, '.planning', 'MILESTONES.md'); + await writeFile(milestonesPath, '# Milestones\n\n', 'utf-8'); + + const result = await milestoneComplete(['v1.0'], tmpDir); + const data = result.data as Record; + const accomplishments = data.accomplishments as string[]; + + // phases 1 and 2 should be counted, 99 must be excluded + expect(data.phases).toBe(2); + expect(accomplishments.some(a => a.includes('Should NOT appear'))).toBe(false); + }); +}); + +// ─── #3 defect-3: extractOneLinerFromBody section-scope leak ────────────── +// +// extractOneLinerFromBody must be bounded to the content between the first +// heading and the next heading of ANY level (GFM § ATX headings: +// https://github.github.com/gfm/#atx-headings). +// A bold match inside a subsequent subsection (e.g. "### Deviation 1 — bar") +// must NOT be returned. + +describe('extractOneLinerFromBody (#3 defect-3): bounded to first heading section', () => { + it('returns the real one-liner and NOT the bold inside a following sub-heading', async () => { + const { extractOneLinerFromBody } = await import('./phase-lifecycle-policy.js'); + const body = [ + '# Phase Summary', + '', + '> This is a blockquote intro — not a one-liner.', + '', + '**Real one-liner: shipped the foo system.**', + '', + 'Some prose.', + '', + '### Deviation 1 — bar', + '', + '**This bold should NOT be picked up.**', + '', + ].join('\n'); + + const result = extractOneLinerFromBody(body); + expect(result).toBe('Real one-liner: shipped the foo system.'); + expect(result).not.toContain('should NOT be picked up'); + }); +}); diff --git a/sdk/src/query/phase-lifecycle.ts b/sdk/src/query/phase-lifecycle.ts index 5dfc9a959..f40e0a621 100644 --- a/sdk/src/query/phase-lifecycle.ts +++ b/sdk/src/query/phase-lifecycle.ts @@ -2039,7 +2039,11 @@ export const milestoneComplete: QueryHandler = async (args, projectDir, workstre const archiveDir = join(paths.planning, 'milestones'); const phasesDir = paths.phases; const today = new Date().toISOString().split('T')[0]!; - const milestoneName = nameOpt || version; + // #3 defect-1: preserve version in header, append optional name only when + // explicitly provided. Historical template: `## v1.8.0 Quick Mode (Shipped: ...)`. + // When no --name is given, milestoneName is undefined so the header is + // `## ${version} (Shipped: ...)` — version appears exactly once. + const milestoneName = nameOpt ?? undefined; await mkdir(archiveDir, { recursive: true }); @@ -2095,8 +2099,10 @@ export const milestoneComplete: QueryHandler = async (args, projectDir, workstre if (existsSync(reqPath)) { const reqContent = await readFile(reqPath, 'utf-8'); + // #3 defect-1: canonical header: version + optional name appended only when provided. + const reqTitle = milestoneName ? `${version} ${milestoneName}` : version; const archiveHeader = - `# Requirements Archive: ${version} ${milestoneName}\n\n` + + `# Requirements Archive: ${reqTitle}\n\n` + `**Archived:** ${today}\n**Status:** SHIPPED\n\n` + `For current requirements, see \`.planning/REQUIREMENTS.md\`.\n\n---\n\n`; await writeFile(join(archiveDir, `${version}-REQUIREMENTS.md`), archiveHeader + reqContent, 'utf-8'); @@ -2108,8 +2114,12 @@ export const milestoneComplete: QueryHandler = async (args, projectDir, workstre } const accomplishmentsList = accomplishments.map((a) => `- ${a}`).join('\n'); + // #3 defect-1 (GFM § ATX headings: https://github.github.com/gfm/#atx-headings) + // Canonical template: `## ${version}${nameOpt ? ` ${nameOpt}` : ''} (Shipped: ${today})` + // Historical evidence: `## v1.8.0 Quick Mode (Shipped: 2026-01-19)` + const milestoneTitle = milestoneName ? `${version} ${milestoneName}` : version; const milestoneEntry = - `## ${version} ${milestoneName} (Shipped: ${today})\n\n` + + `## ${milestoneTitle} (Shipped: ${today})\n\n` + `**Phases completed:** ${phaseCount} phases, ${totalPlans} plans, ${totalTasks} tasks\n\n` + `**Key accomplishments:**\n${accomplishmentsList || '- (none recorded)'}\n\n---\n\n`; diff --git a/sdk/src/query/state.ts b/sdk/src/query/state.ts index 2b6cc59ae..3e6095d86 100644 --- a/sdk/src/query/state.ts +++ b/sdk/src/query/state.ts @@ -47,7 +47,13 @@ export async function getMilestonePhaseFilter(projectDir: string, workstream?: s try { const roadmapContent = await readFile(planningPaths(projectDir, workstream).roadmap, 'utf-8'); const roadmap = await extractCurrentMilestone(roadmapContent, projectDir, workstream); - const phasePattern = /#{2,4}\s*Phase\s+([\w][\w.-]*)\s*:/gi; + // Match both heading-style (### Phase N:) and GFM task-list bullet-style + // (- [ ] **Phase N: title**) declarations. + // GFM § ATX headings: https://github.github.com/gfm/#atx-headings + // GFM § Task list items: https://github.github.com/gfm/#task-list-items-extension- + // #3 defect-2: the original regex matched only heading-style declarations; + // bullet-only ROADMAPs caused milestonePhaseNums.size === 0 -> passAll -> phase 99 leakage. + const phasePattern = /(?:#{2,4}\s*|-\s*(?:\[[x ]\]\s*)?\*{0,2}\s*)Phase\s+([\w][\w.-]*)\s*:/gi; let m: RegExpExecArray | null; while ((m = phasePattern.exec(roadmap)) !== null) { milestonePhaseNums.add(m[1]); diff --git a/sdk/src/query/summary.test.ts b/sdk/src/query/summary.test.ts index e066a7c35..4019b18b3 100644 --- a/sdk/src/query/summary.test.ts +++ b/sdk/src/query/summary.test.ts @@ -59,6 +59,29 @@ describe('summaryExtract', () => { expect(Array.isArray(data.decisions)).toBe(true); }); + it('summaryExtract: one-liner extraction is bounded to first heading scope', async () => { + const rel = '.planning/phases/01-x/01-SUMMARY.md'; + const fixture = [ + '# Phase Summary', + '', + '> Blockquote intro — should NOT be picked up.', + '', + '**Real one-liner: shipped the foo system.**', + '', + '### Deviation 1 — bar', + '', + '**This bold MUST NOT be picked up by the bounded extractor.**', + ].join('\n'); + await writeFile( + join(tmpDir, '.planning', 'phases', '01-x', '01-SUMMARY.md'), + fixture, + 'utf-8', + ); + const r = await summaryExtract([rel], tmpDir); + const data = r.data as Record; + expect(data.one_liner).toBe('Real one-liner: shipped the foo system.'); + }); + it('filters with --fields', async () => { const rel = '.planning/phases/01-x/01-SUMMARY.md'; await writeFile( diff --git a/sdk/src/query/summary.ts b/sdk/src/query/summary.ts index 6eabc8019..936c8e7b8 100644 --- a/sdk/src/query/summary.ts +++ b/sdk/src/query/summary.ts @@ -26,12 +26,30 @@ import type { QueryHandler } from './utils.js'; /** * Extract a one-liner from the summary body when it is not in frontmatter. * Port of `extractOneLinerFromBody` from `get-shit-done/bin/lib/core.cjs`. + * + * Scope: content from the first heading to the next heading of ANY level. + * Per GFM ATX headings (https://github.github.com/gfm/#atx-headings), + * a heading of any level terminates the scope — including sub-headings + * even when the title heading is H1. + * + * #3 defect-3: the original regex matched the first bold in the entire body + * after any heading, leaking bold text from later sections (e.g. deviation + * entries) into the one-liner. */ function extractOneLinerFromBody(content: string): string | null { if (!content) return null; const body = content.replace(/^---\r?\n[\s\S]*?\r?\n---\r?\n*/, ''); - const match = body.match(/^#[^\n]*\n+\*\*([^*]+)\*\*/m); - return match ? match[1].trim() : null; + // Find the first heading of any level (GFM section ATX headings) + const headingMatch = body.match(/^(#{1,6}\s[^\n]+\n)/m); + if (!headingMatch || headingMatch.index === undefined) return null; + const afterHeading = body.slice(headingMatch.index + headingMatch[0].length); + // Bound to the first section: truncate at the next heading of any level + const nextHeadingMatch = afterHeading.match(/^#{1,6}\s/m); + const sectionScope = nextHeadingMatch && nextHeadingMatch.index !== undefined + ? afterHeading.slice(0, nextHeadingMatch.index) + : afterHeading; + const boldMatch = sectionScope.match(/\*\*([^*]+)\*\*/); + return boldMatch ? boldMatch[1]!.trim() : null; } /** Normalize frontmatter list fields — scalars become single-element arrays. */