From 59fd17251ae94abfa426e194cef3b1b916a0f7c1 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 4 May 2026 23:22:20 -0400 Subject: [PATCH] fix(phase): clarify insert preconditions and reject unsupported dry-run flag --- .../bin/lib/phase-command-router.cjs | 3 +++ get-shit-done/bin/lib/phase.cjs | 4 ++++ tests/phase.test.cjs | 22 +++++++++++++++++++ 3 files changed, 29 insertions(+) diff --git a/get-shit-done/bin/lib/phase-command-router.cjs b/get-shit-done/bin/lib/phase-command-router.cjs index e4f4a6653..979088ef4 100644 --- a/get-shit-done/bin/lib/phase-command-router.cjs +++ b/get-shit-done/bin/lib/phase-command-router.cjs @@ -33,6 +33,9 @@ function routePhaseCommand({ phase, args, cwd, raw, error }) { } phase.cmdPhaseAddBatch(cwd, descriptions, raw); } else if (subcommand === 'insert') { + if (args.includes('--dry-run')) { + error('phase insert does not support --dry-run'); + } phase.cmdPhaseInsert(cwd, args[2], args.slice(3).join(' '), raw); } else if (subcommand === 'remove') { const forceFlag = args.includes('--force'); diff --git a/get-shit-done/bin/lib/phase.cjs b/get-shit-done/bin/lib/phase.cjs index 9c078e0aa..b1acadfcc 100644 --- a/get-shit-done/bin/lib/phase.cjs +++ b/get-shit-done/bin/lib/phase.cjs @@ -556,6 +556,10 @@ function cmdPhaseInsert(cwd, afterPhase, description, raw) { const afterPhaseEscaped = unpadded.replace(/\./g, '\\.'); const targetPattern = new RegExp(`#{2,4}\\s*Phase\\s+0*${afterPhaseEscaped}:`, 'i'); if (!targetPattern.test(content)) { + const checklistPattern = new RegExp(`-\\s*\\[[ x]\\]\\s*\\*\\*Phase\\s+0*${afterPhaseEscaped}:`, 'i'); + if (checklistPattern.test(content)) { + error(`Phase ${afterPhase} exists in roadmap summary but is missing a detail section (### Phase ${afterPhase}: ...).`); + } error(`Phase ${afterPhase} not found in ROADMAP.md`); } diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 68779c1af..0af6ab12c 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -1237,6 +1237,28 @@ describe('phase insert command', () => { assert.ok(roadmap.includes('**Requirements**: TBD'), 'inserted phase entry should include Requirements TBD'); }); + test('reports actionable error for summary-only placeholder phase without detail section (#3098)', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap\n\n- [ ] **Phase 5: Placeholder**\n` + ); + + const result = runGsdTools('phase insert 5 Hotfix', tmpDir); + assert.ok(!result.success, 'should fail when phase is summary-only placeholder'); + assert.ok(result.error.includes('missing a detail section')); + }); + + test('phase insert rejects unsupported --dry-run flag explicitly (#3098)', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap\n\n### Phase 1: Foundation\n**Goal:** Setup\n` + ); + + const result = runGsdTools('phase insert 1 Hotfix --dry-run', tmpDir); + assert.ok(!result.success, 'phase insert should reject unsupported --dry-run'); + assert.ok(result.error.includes('does not support --dry-run')); + }); + test('handles #### heading depth from multi-milestone roadmaps', () => { fs.writeFileSync( path.join(tmpDir, '.planning', 'ROADMAP.md'),