fix: address review findings on milestone-summary PR

1. Add <success_criteria> section to command (pattern compliance)
2. Fix archived audit path: check .planning/milestones/ not .planning/
3. Add RESEARCH.md to command context block
4. Add empty phase handling (graceful skip to minimal summary)
5. Add overwrite guard for existing summary files
6. Add 7 new tests: overwrite guard, empty phases, audit paths,
   success_criteria, RESEARCH.md context, artifact path resolution

Tests: 18/18 milestone-summary, 1184/1184 full suite

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

commands/gsd/milestone-summary.md

@@ -29,7 +29,7 @@ Output: MILESTONE_SUMMARY written to `.planning/reports/`, presented inline, opt
 - `.planning/RETROSPECTIVE.md`
 - `.planning/milestones/v{version}-ROADMAP.md` (if archived)
 - `.planning/milestones/v{version}-REQUIREMENTS.md` (if archived)
-- `.planning/phases/*-*/` (SUMMARY.md, VERIFICATION.md, CONTEXT.md)
+- `.planning/phases/*-*/` (SUMMARY.md, VERIFICATION.md, CONTEXT.md, RESEARCH.md)
 
 **User input:**
 - Version: $ARGUMENTS (optional — defaults to current/latest milestone)
@@ -38,3 +38,13 @@ Output: MILESTONE_SUMMARY written to `.planning/reports/`, presented inline, opt
 <process>
 Read and execute the milestone-summary workflow from @~/.claude/get-shit-done/workflows/milestone-summary.md end-to-end.
 </process>
+
+<success_criteria>
+- Milestone version resolved (from args, STATE.md, or archive scan)
+- All available artifacts read (ROADMAP, REQUIREMENTS, CONTEXT, SUMMARY, VERIFICATION, RESEARCH, RETROSPECTIVE)
+- Summary document written to `.planning/reports/MILESTONE_SUMMARY-v{version}.md`
+- All 7 sections generated (Overview, Architecture, Phases, Decisions, Requirements, Tech Debt, Getting Started)
+- Summary presented inline to user
+- Interactive Q&A offered
+- STATE.md updated
+</success_criteria>

get-shit-done/workflows/milestone-summary.md

@@ -27,7 +27,7 @@ Determine whether the milestone is **archived** or **current**:
 ```
 ROADMAP_PATH=".planning/milestones/v${VERSION}-ROADMAP.md"
 REQUIREMENTS_PATH=".planning/milestones/v${VERSION}-REQUIREMENTS.md"
-AUDIT_PATH=".planning/v${VERSION}-MILESTONE-AUDIT.md"
+AUDIT_PATH=".planning/milestones/v${VERSION}-MILESTONE-AUDIT.md"
 ```
 
 **Current/in-progress milestone** (no archive yet):
@@ -37,6 +37,8 @@ REQUIREMENTS_PATH=".planning/REQUIREMENTS.md"
 AUDIT_PATH=".planning/v${VERSION}-MILESTONE-AUDIT.md"
 ```
 
+Note: The audit file moves to `.planning/milestones/` on archive (per `complete-milestone` workflow). Check both locations as a fallback.
+
 **Always available:**
 ```
 PROJECT_PATH=".planning/PROJECT.md"
@@ -63,6 +65,8 @@ This returns phase metadata. For each phase in the milestone scope:
 
 Track which phases have which artifacts.
 
+**If no phase directories exist** (empty milestone or pre-build state): skip to Step 5 and generate a minimal summary noting "No phases have been executed yet." Do not error — the summary should still capture PROJECT.md and ROADMAP.md content.
+
 ## Step 4: Gather Git Statistics
 
 ```bash
@@ -153,8 +157,18 @@ Present as a bulleted list of decisions with brief rationale:
 - **Contributors:** {list}
 ```
 
-## Step 6: Commit
+## Step 6: Write and Commit
 
+**Overwrite guard:** If `.planning/reports/MILESTONE_SUMMARY-v${VERSION}.md` already exists, ask the user:
+> "A milestone summary for v{VERSION} already exists. Overwrite it, or view the existing one?"
+If "view": display existing file and skip to Step 8 (interactive mode). If "overwrite": proceed.
+
+Create the reports directory if needed:
+```bash
+mkdir -p .planning/reports
+```
+
+Write the summary, then commit:
 ```bash
 gsd-tools.cjs commit "docs(v${VERSION}): generate milestone summary for onboarding" \
   --files ".planning/reports/MILESTONE_SUMMARY-v${VERSION}.md"

tests/milestone-summary.test.cjs

@@ -111,4 +111,82 @@ describe('milestone-summary workflow', () => {
       'should update STATE.md via gsd-tools'
     );
   });
+
+  test('workflow has overwrite guard for existing summaries', () => {
+    const content = fs.readFileSync(workflowPath, 'utf-8');
+    assert.ok(
+      content.includes('already exists'),
+      'should check for existing summary before overwriting'
+    );
+  });
+
+  test('workflow handles empty phase directories gracefully', () => {
+    const content = fs.readFileSync(workflowPath, 'utf-8');
+    assert.ok(
+      content.includes('no phase directories') || content.includes('No phases'),
+      'should handle case where no phases exist'
+    );
+  });
+
+  test('workflow checks both audit file locations for archived milestones', () => {
+    const content = fs.readFileSync(workflowPath, 'utf-8');
+    assert.ok(
+      content.includes('.planning/milestones/v${VERSION}-MILESTONE-AUDIT.md'),
+      'should check milestones/ directory for archived audit file'
+    );
+  });
+});
+
+describe('milestone-summary command structure', () => {
+  test('command has success_criteria section', () => {
+    const content = fs.readFileSync(commandPath, 'utf-8');
+    assert.ok(
+      content.includes('<success_criteria>'),
+      'should have success_criteria section (follows complete-milestone pattern)'
+    );
+  });
+
+  test('command context lists RESEARCH.md', () => {
+    const content = fs.readFileSync(commandPath, 'utf-8');
+    assert.ok(
+      content.includes('RESEARCH.md'),
+      'should list RESEARCH.md in context block'
+    );
+  });
+});
+
+describe('milestone-summary artifact path resolution', () => {
+  const { createTempProject, cleanup } = require('./helpers.cjs');
+  let tmpDir;
+
+  test('archived milestone paths point to milestones/ directory', () => {
+    const content = fs.readFileSync(workflowPath, 'utf-8');
+    // Archived roadmap path should be under milestones/
+    assert.ok(
+      content.includes('.planning/milestones/v${VERSION}-ROADMAP.md'),
+      'archived ROADMAP path should be under .planning/milestones/'
+    );
+    assert.ok(
+      content.includes('.planning/milestones/v${VERSION}-REQUIREMENTS.md'),
+      'archived REQUIREMENTS path should be under .planning/milestones/'
+    );
+    assert.ok(
+      content.includes('.planning/milestones/v${VERSION}-MILESTONE-AUDIT.md'),
+      'archived AUDIT path should be under .planning/milestones/'
+    );
+  });
+
+  test('current milestone paths point to .planning/ root', () => {
+    const content = fs.readFileSync(workflowPath, 'utf-8');
+    // Current milestone should read from .planning/ root
+    const lines = content.split('\n');
+    const currentSection = lines.slice(
+      lines.findIndex(l => l.includes('Current/in-progress')),
+      lines.findIndex(l => l.includes('Current/in-progress')) + 10
+    ).join('\n');
+    assert.ok(
+      currentSection.includes('ROADMAP_PATH=".planning/ROADMAP.md"'),
+      'current ROADMAP path should be at .planning/ root'
+    );
+  });
 });