fix(#3163): scope phase.add insertion to the current milestone (#3400)

* test(#3163): phase add must insert in the active milestone, not the trailing archive

Regression for #3163: cmdPhaseAdd/cmdPhaseAddBatch pick the insertion point
via rawContent.lastIndexOf('\n---'), the file's last horizontal rule — which
on a roadmap with shipped/history material after the active phase list sits
deep in archive. Rows 1/2/4 fail RED on next (entry lands after the archive
heading); row 3 guards the no-milestone legacy fallback.

* fix(#3163): scope phase.add insertion to the current milestone window

cmdPhaseAdd and cmdPhaseAddBatch picked the insertion point via
rawContent.lastIndexOf('\n---') — the file's last horizontal rule, which on
a roadmap with shipped/history material after the active phase list sits deep
in archive. Extract phaseEntryInsertOffset(rawContent, cwd): scope the search
to currentMilestoneRawRanges' primary window so the entry lands at the end of
the active phase list. Fall back to the legacy whole-file heuristic when no
current milestone resolves, preserving simple no-milestone roadmaps. Applies
to both cmdPhaseAdd and cmdPhaseAddBatch (identical expression); the decimal
insert path was already header-anchored and is untouched.

* docs(#3163): add changeset

* docs(#3163): backfill changeset PR number (3400)

---------

Co-authored-by: sim <sim@local>

.changeset/daring-koalas-zip.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3400
+---
+**`phase add` no longer files new phases inside archived roadmap history** — the insertion point used the file's last horizontal rule, which on a long roadmap sits deep in shipped/archive content, so new phases landed under an unrelated archived phase's heading instead of at the end of the active phase list. Insertion is now scoped to the current milestone. (#3163)

src/phase.cts

@@ -993,6 +993,27 @@ function describeGoalShapedTitle(description: string): string | null {
   );
 }
 
+/**
+ * #3163: compute the byte offset in `rawContent` where a new `### Phase N:`
+ * entry should be inserted — at the end of the active phase list, scoped to the
+ * CURRENT MILESTONE so the entry can never land before a trailing `---` in
+ * shipped/history/backlog material (the file's last `---` on a long roadmap
+ * sits deep in archive). When no current milestone can be resolved (no
+ * STATE.md `milestone:` and no in-progress `🚧`/`🔄` marker), fall back to the
+ * legacy whole-file lastIndexOf('\n---') so simple no-milestone roadmaps keep
+ * their existing behavior.
+ */
+function phaseEntryInsertOffset(rawContent: string, cwd: string): number {
+  const ranges = currentMilestoneRawRanges(rawContent, cwd);
+  if (!ranges) {
+    const legacy = rawContent.lastIndexOf('\n---');
+    return legacy > 0 ? legacy : rawContent.length;
+  }
+  const window = rawContent.slice(ranges.primary.start, ranges.primary.end);
+  const lastSeparator = window.lastIndexOf('\n---');
+  return lastSeparator > 0 ? ranges.primary.start + lastSeparator : ranges.primary.end;
+}
+
 function cmdPhaseAdd(cwd: string, description: string, raw: boolean, customId?: string): void {
   if (!description) {
     error('description required for phase add');
@@ -1083,13 +1104,8 @@ function cmdPhaseAdd(cwd: string, description: string, raw: boolean, customId?:
     const phaseEntry =
       `\n### Phase ${_newPhaseId}: ${description}\n\n**Goal:** [To be planned]\n**Requirements**: TBD${dependsOn}\n**Plans:** 0 plans\n\nPlans:\n- [ ] TBD (run ${formatGsdSlash('plan-phase', resolveRuntime(cwd)) as string} ${_newPhaseId} to break down)\n`;
 
-    let updatedContent: string;
-    const lastSeparator = rawContent.lastIndexOf('\n---');
-    if (lastSeparator > 0) {
-      updatedContent = rawContent.slice(0, lastSeparator) + phaseEntry + rawContent.slice(lastSeparator);
-    } else {
-      updatedContent = rawContent + phaseEntry;
-    }
+    const insertAt = phaseEntryInsertOffset(rawContent, cwd);
+    const updatedContent = rawContent.slice(0, insertAt) + phaseEntry + rawContent.slice(insertAt);
 
     platformWriteSync(roadmapPath, updatedContent);
     return { newPhaseId: _newPhaseId, dirName: _dirName };
@@ -1174,11 +1190,8 @@ function cmdPhaseAddBatch(cwd: string, descriptions: string[], raw: boolean): vo
           : `\n**Depends on:** Phase ${typeof newPhaseId === 'number' ? newPhaseId - 1 : 'TBD'}`;
       const phaseEntry =
         `\n### Phase ${newPhaseId}: ${description}\n\n**Goal:** [To be planned]\n**Requirements**: TBD${dependsOn}\n**Plans:** 0 plans\n\nPlans:\n- [ ] TBD (run ${formatGsdSlash('plan-phase', resolveRuntime(cwd)) as string} ${newPhaseId} to break down)\n`;
-      const lastSeparator = rawContent.lastIndexOf('\n---');
-      rawContent =
-        lastSeparator > 0
-          ? rawContent.slice(0, lastSeparator) + phaseEntry + rawContent.slice(lastSeparator)
-          : rawContent + phaseEntry;
+      const insertAt = phaseEntryInsertOffset(rawContent, cwd);
+      rawContent = rawContent.slice(0, insertAt) + phaseEntry + rawContent.slice(insertAt);
       added.push({
         phase_number: typeof newPhaseId === 'number' ? newPhaseId : String(newPhaseId),
         padded:

tests/phase.test.cjs

@@ -1627,6 +1627,109 @@ describe('phase add command', () => {
 // phase add — orphan directory collision prevention (#2026)
 // ─────────────────────────────────────────────────────────────────────────────
 
+describe('#3163: phase add inserts in the active milestone phase list, not the trailing archive', () => {
+  let tmpDir;
+
+  beforeEach(() => {
+    tmpDir = createTempProject();
+  });
+
+  afterEach(() => {
+    cleanup(tmpDir);
+  });
+
+  // Fixture: a v1.0 ACTIVE milestone with a phase list, followed by a shipped
+  // v0.9 archive whose own `---` is the FILE's last `---`. The bug places the
+  // new phase before that archive `---`; the fix scopes to v1.0's window.
+  function writeArchiveRoadmap(dir, { singlePhase = false } = {}) {
+    fs.writeFileSync(
+      path.join(dir, '.planning', 'STATE.md'),
+      'milestone: v1.0\ncurrent_phase: 2\n'
+    );
+    const phases = singlePhase
+      ? '### Phase 1: Foundation\n**Goal:** setup\n'
+      : '### Phase 1: Foundation\n**Goal:** setup\n\n### Phase 2: API\n**Goal:** build\n';
+    fs.writeFileSync(
+      path.join(dir, '.planning', 'ROADMAP.md'),
+      '# Roadmap\n\n## v1.0: Active Milestone\n\n' +
+        phases +
+        '\n---\n\n## v0.9: Shipped Archive\n\n### Phase 0 (original scope): Bootstrap\n**Goal:** init\n\n#### Operator decisions\n- decided X\n\n---\n'
+    );
+  }
+
+  test('row 1 — phase add lands in the active milestone, before the archive', () => {
+    writeArchiveRoadmap(tmpDir);
+    const result = runGsdTools('phase add New Feature', tmpDir);
+    assert.ok(result.success, `Command failed: ${result.error}`);
+
+    const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
+    const phase3 = roadmap.indexOf('### Phase 3: New Feature');
+    const phase2 = roadmap.indexOf('### Phase 2: API');
+    const archive = roadmap.indexOf('## v0.9: Shipped Archive');
+    assert.notStrictEqual(phase3, -1, 'new phase entry should exist in the roadmap');
+    assert.ok(phase2 > -1 && phase2 < phase3, `Phase 3 must come after Phase 2; got p2@${phase2} p3@${phase3}`);
+    assert.ok(
+      phase3 < archive,
+      `#3163: Phase 3 must land INSIDE the active v1.0 milestone (before the v0.9 archive), not before the file's last \`---\`; got phase3@${phase3} archive@${archive}`
+    );
+  });
+
+  test('row 2 — phase add-batch is also scoped to the active milestone', () => {
+    writeArchiveRoadmap(tmpDir);
+    const result = runGsdTools(
+      ['phase', 'add-batch', '--descriptions', '["First Add","Second Add"]'],
+      tmpDir
+    );
+    assert.ok(result.success, `Command failed: ${result.error}`);
+
+    const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
+    const archive = roadmap.indexOf('## v0.9: Shipped Archive');
+    for (const [num, title] of [['3', 'First Add'], ['4', 'Second Add']]) {
+      const at = roadmap.indexOf(`### Phase ${num}: ${title}`);
+      assert.notStrictEqual(at, -1, `Phase ${num} (${title}) should exist`);
+      assert.ok(
+        at < archive,
+        `#3163: Phase ${num} must land before the archive; got @${at} archive@${archive}`
+      );
+    }
+  });
+
+  test('row 3 — no-milestone fallback keeps legacy insertion before the trailing ---', () => {
+    // No STATE.md milestone field and no WIP marker → currentMilestoneRawRanges
+    // returns null → the legacy whole-file lastIndexOf('\n---') path is used.
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'ROADMAP.md'),
+      '# Roadmap\n\n### Phase 1: Foundation\n**Goal:** setup\n\n---\n'
+    );
+    const result = runGsdTools('phase add Next Phase', tmpDir);
+    assert.ok(result.success, `Command failed: ${result.error}`);
+
+    const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
+    const phase2 = roadmap.indexOf('### Phase 2: Next Phase');
+    const sep = roadmap.indexOf('\n---');
+    assert.notStrictEqual(phase2, -1, 'Phase 2 should exist');
+    assert.ok(
+      phase2 < sep,
+      `no-milestone fallback must preserve legacy placement (before the trailing ---); got phase2@${phase2} sep@${sep}`
+    );
+  });
+
+  test('row 4 — single-phase milestone still scopes to the active window', () => {
+    writeArchiveRoadmap(tmpDir, { singlePhase: true });
+    const result = runGsdTools('phase add Second Phase', tmpDir);
+    assert.ok(result.success, `Command failed: ${result.error}`);
+
+    const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
+    const phase2 = roadmap.indexOf('### Phase 2: Second Phase');
+    const archive = roadmap.indexOf('## v0.9: Shipped Archive');
+    assert.notStrictEqual(phase2, -1, 'Phase 2 should exist');
+    assert.ok(
+      phase2 < archive,
+      `Phase 2 must land inside the single-phase v1.0 window, before the archive; got @${phase2} archive@${archive}`
+    );
+  });
+});
+
 describe('phase add — orphan directory collision prevention (#2026)', () => {
   let tmpDir;