fix(sdk): extractCurrentMilestone Backlog leak + state.begin-phase flag parsing (#2455)

* fix(sdk): extractCurrentMilestone Backlog leak + state.begin-phase flag parsing

Closes #2422
Closes #2420

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

* fix: patch-version semver in milestone boundary regex + flag-parser validation

Two follow-on correctness issues identified in code review:

1. roadmap.ts: currentVersionMatch and nextMilestoneRegex only captured
   major.minor (v(\d+\.\d+)), collapsing v2.0.1 to "2.0". A sub-heading
   "## v2.0.2 Phase Details" would match the same prefix and be incorrectly
   skipped. Both patterns updated to v(\d+(?:\.\d+)+) to capture full semver.

2. state-mutation.ts: pair-wise flag parsing loop advanced i by 2 unconditionally,
   so a missing flag value caused the next flag token to be assigned as the value
   (e.g. flags['phase'] = '--name'). Fix: iterate with i++ and validate that the
   candidate value exists and does not start with '--' before assigning; throw
   GSDError('missing value for --<key>') on invalid input. Added regression test.

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

sdk/src/query/roadmap.test.ts

@@ -144,6 +144,57 @@ describe('extractCurrentMilestone', () => {
     const result = await extractCurrentMilestone(content, tmpDir);
     expect(result).toBe('current content');
   });
+
+  // ─── Bug #2422: preamble Backlog leak ─────────────────────────────────
+  it('bug-2422: does not include ## Backlog section before the current milestone', async () => {
+    const roadmapWithBacklog = `# ROADMAP
+
+## Backlog
+### Phase 999.1: Parking lot item A
+### Phase 999.2: Parking lot item B
+
+### 🚧 v2.0 My Milestone (In Progress)
+- [ ] **Phase 100: Real work**
+
+## v2.0 Phase Details
+### Phase 100: Real work
+**Goal**: Do stuff.
+`;
+    const state = `---\nmilestone: v2.0\n---\n# State\n`;
+    await writeFile(join(tmpDir, '.planning', 'STATE.md'), state);
+    await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmapWithBacklog);
+
+    const result = await extractCurrentMilestone(roadmapWithBacklog, tmpDir);
+
+    // Must NOT include backlog phases
+    expect(result).not.toContain('Phase 999.1');
+    expect(result).not.toContain('Phase 999.2');
+    expect(result).not.toContain('Parking lot');
+    // Must include the actual v2.0 content
+    expect(result).toContain('Phase 100');
+  });
+
+  // ─── 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
+
+### 🚧 v2.0 My Milestone (In Progress)
+- [ ] **Phase 100: Real work**
+
+## v2.0 Phase Details
+### Phase 100: Real work
+**Goal**: Do stuff.
+`;
+    const state = `---\nmilestone: v2.0\n---\n# State\n`;
+    await writeFile(join(tmpDir, '.planning', 'STATE.md'), state);
+    await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmapWithDetails);
+
+    const result = await extractCurrentMilestone(roadmapWithDetails, tmpDir);
+
+    // The detail section must survive — not be cut off
+    expect(result).toContain('Phase 100');
+    expect(result).toContain('Phase Details');
+  });
 });
 
 // ─── roadmapGetPhase ──────────────────────────────────────────────────────

sdk/src/query/roadmap.ts

@@ -110,7 +110,7 @@ export async function extractCurrentMilestone(content: string, projectDir: strin
 
   // Fallback: derive from ROADMAP in-progress marker
   if (!version) {
-    const inProgressMatch = content.match(/🚧\s*\*\*v(\d+\.\d+)\s/);
+    const inProgressMatch = content.match(/🚧\s*\*\*v(\d+(?:\.\d+)+)\s/);
     if (inProgressMatch) {
       version = 'v' + inProgressMatch[1];
     }
@@ -130,30 +130,35 @@ export async function extractCurrentMilestone(content: string, projectDir: strin
 
   const sectionStart = sectionMatch.index;
 
-  // Find end: next milestone heading at same or higher level, or EOF
+  // Find end: next milestone heading at same or higher level, or EOF.
+  // Skip headings that belong to the SAME version (e.g. "## v2.0 Phase Details").
   const headingLevelMatch = sectionMatch[1].match(/^(#{1,3})\s/);
   const headingLevel = headingLevelMatch ? headingLevelMatch[1].length : 2;
   const restContent = content.slice(sectionStart + sectionMatch[0].length);
-  const nextMilestonePattern = new RegExp(
-    `^#{1,${headingLevel}}\\s+(?:.*v\\d+\\.\\d+|✅|📋|🚧)`,
-    'mi'
-  );
-  const nextMatch = restContent.match(nextMilestonePattern);
 
-  let sectionEnd: number;
-  if (nextMatch && nextMatch.index !== undefined) {
-    sectionEnd = sectionStart + sectionMatch[0].length + nextMatch.index;
-  } else {
-    sectionEnd = content.length;
+  // Extract current version so same-version sub-headings are not treated as boundaries.
+  // Capture full semver (major.minor.patch) so v2.0.1 is not collapsed to "2.0".
+  const currentVersionMatch = version ? version.match(/v(\d+(?:\.\d+)+)/i) : null;
+  const currentVersionStr = currentVersionMatch ? currentVersionMatch[1] : '';
+
+  const nextMilestoneRegex = new RegExp(
+    `^#{1,${headingLevel}}\\s+(?:.*v(\\d+(?:\\.\\d+)+)[^\\n]*|.*(?:✅|📋|🚧))`,
+    'gm'
+  );
+
+  let sectionEnd = content.length;
+  let m: RegExpExecArray | null;
+  while ((m = nextMilestoneRegex.exec(restContent)) !== null) {
+    const matchedVersion = m[1];
+    // Skip headings that reference the same version (e.g. "## v2.0 Phase Details").
+    if (matchedVersion && currentVersionStr && matchedVersion === currentVersionStr) continue;
+    sectionEnd = sectionStart + sectionMatch[0].length + m.index;
+    break;
   }
 
-  const beforeMilestones = content.slice(0, sectionStart);
-  const currentSection = content.slice(sectionStart, sectionEnd);
-
-  // Strip <details> from preamble
-  const preamble = beforeMilestones.replace(/<details>[\s\S]*?<\/details>/gi, '');
-
-  return preamble + currentSection;
+  // Return only the current milestone section — never include the preamble, which
+  // may contain ## Backlog and other non-current-milestone phases.
+  return content.slice(sectionStart, sectionEnd);
 }
 
 // ─── Internal helpers ─────────────────────────────────────────────────────

sdk/src/query/state-mutation.test.ts

@@ -304,6 +304,50 @@ describe('stateBeginPhase', () => {
     expect(content).toContain('Executing Phase 11');
     expect(content).toContain('State Mutations');
   });
+
+  // ─── Bug #2420: flag-form args not parsed ────────────────────────────
+  it('bug-2420: parses --phase/--name/--plans flag-form args correctly', async () => {
+    const { stateBeginPhase } = await import('./state-mutation.js');
+
+    // This is how execute-phase.md calls it: flag form
+    const result = await stateBeginPhase(
+      ['--phase', '99', '--name', 'probe-test', '--plans', '1'],
+      tmpDir
+    );
+    const data = result.data as Record<string, unknown>;
+
+    // Must return the actual values, not the flag names
+    expect(data.phase).toBe('99');
+    expect(data.name).toBe('probe-test');
+    expect(data.plan_count).toBe('1');
+
+    // STATE.md must contain clean output, not literal "--phase"
+    const content = await readFile(join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
+    expect(content).not.toContain('--phase');
+    expect(content).not.toContain('--name');
+    expect(content).not.toContain('--plans');
+    expect(content).toContain('Executing Phase 99');
+    expect(content).toContain('probe-test');
+  });
+
+  it('bug-2420: positional args still work after flag-parsing fix', async () => {
+    const { stateBeginPhase } = await import('./state-mutation.js');
+
+    const result = await stateBeginPhase(['42', 'Positional Test', '5'], tmpDir);
+    const data = result.data as Record<string, unknown>;
+    expect(data.phase).toBe('42');
+    expect(data.name).toBe('Positional Test');
+    expect(data.plan_count).toBe('5');
+  });
+
+  it('bug-2420: flag parser throws when a flag value is missing (next token is a flag)', async () => {
+    const { stateBeginPhase } = await import('./state-mutation.js');
+
+    // --phase has no value — next token is --name, which is itself a flag.
+    await expect(
+      stateBeginPhase(['--phase', '--name', 'Title', '--plans', '1'], tmpDir)
+    ).rejects.toThrow('missing value for --phase');
+  });
 });
 
 // ─── stateAdvancePlan ───────────────────────────────────────────────────────

sdk/src/query/state-mutation.ts

@@ -331,9 +331,32 @@ export const statePatch: QueryHandler = async (args, projectDir) => {
  * @returns QueryResult with { phase, name, plan_count }
  */
 export const stateBeginPhase: QueryHandler = async (args, projectDir) => {
-  const phaseNumber = args[0];
-  const phaseName = args[1] || '';
-  const planCount = args[2] || '?';
+  // Accept both flag form (--phase 04 --name "Title" --plans 3) and positional form (04 "Title" 3).
+  let phaseNumber: string;
+  let phaseName: string;
+  let planCount: string;
+
+  if (args.length > 0 && typeof args[0] === 'string' && args[0].startsWith('--')) {
+    const flags: Record<string, string> = {};
+    for (let i = 0; i < args.length; i++) {
+      const token = args[i];
+      if (typeof token !== 'string' || !token.startsWith('--')) continue;
+      const key = token.slice(2);
+      const value = args[i + 1];
+      if (value === undefined || (typeof value === 'string' && value.startsWith('--'))) {
+        throw new GSDError(`missing value for --${key}`, ErrorClassification.Validation);
+      }
+      flags[key] = value as string;
+      i += 1;
+    }
+    phaseNumber = flags['phase'] ?? '';
+    phaseName = flags['name'] ?? '';
+    planCount = flags['plans'] ?? flags['plan-count'] ?? '?';
+  } else {
+    phaseNumber = args[0] ?? '';
+    phaseName = args[1] || '';
+    planCount = args[2] || '?';
+  }
 
   if (!phaseNumber) {
     throw new GSDError('phase number required', ErrorClassification.Validation);