From 2ded61bf452e8cd49d5b1d392850cbfb3c81edc4 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 5 Apr 2026 18:05:32 -0400 Subject: [PATCH] fix(cli): require --confirm flag before phases clear deletes directories (#1832) phases clear now checks for phase dirs before deleting. If any exist and --confirm is absent, the command exits non-zero with a message showing the count and how to proceed. Empty phases dir (nothing to delete) succeeds without --confirm unchanged. Updates new-milestone.md workflow to pass --confirm (intentional programmatic caller). Updates existing new-milestone-clear-phases tests to match new API. Closes #1826 Co-authored-by: Claude Sonnet 4.6 --- get-shit-done/bin/gsd-tools.cjs | 2 +- get-shit-done/bin/lib/milestone.cjs | 17 +++-- get-shit-done/workflows/new-milestone.md | 2 +- tests/bug-1826-phases-clear-confirm.test.cjs | 72 ++++++++++++++++++++ tests/new-milestone-clear-phases.test.cjs | 10 +-- 5 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 tests/bug-1826-phases-clear-confirm.test.cjs diff --git a/get-shit-done/bin/gsd-tools.cjs b/get-shit-done/bin/gsd-tools.cjs index f2768a8d5..399fc560c 100755 --- a/get-shit-done/bin/gsd-tools.cjs +++ b/get-shit-done/bin/gsd-tools.cjs @@ -602,7 +602,7 @@ async function runCommand(command, args, cwd, raw) { }; phase.cmdPhasesList(cwd, options, raw); } else if (subcommand === 'clear') { - milestone.cmdPhasesClear(cwd, raw); + milestone.cmdPhasesClear(cwd, raw, args.slice(2)); } else { error('Unknown phases subcommand. Available: list, clear'); } diff --git a/get-shit-done/bin/lib/milestone.cjs b/get-shit-done/bin/lib/milestone.cjs index 3721c0860..adfb149a7 100644 --- a/get-shit-done/bin/lib/milestone.cjs +++ b/get-shit-done/bin/lib/milestone.cjs @@ -246,15 +246,24 @@ function cmdMilestoneComplete(cwd, version, options, raw) { output(result, raw); } -function cmdPhasesClear(cwd, raw) { +function cmdPhasesClear(cwd, raw, args) { const phasesDir = planningPaths(cwd).phases; + const confirm = Array.isArray(args) && args.includes('--confirm'); let cleared = 0; if (fs.existsSync(phasesDir)) { + const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); + const dirs = entries.filter(e => e.isDirectory()); + + if (dirs.length > 0 && !confirm) { + error( + `phases clear would delete ${dirs.length} phase director${dirs.length === 1 ? 'y' : 'ies'}. ` + + `Pass --confirm to proceed.` + ); + } + try { - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - for (const entry of entries) { - if (!entry.isDirectory()) continue; + for (const entry of dirs) { fs.rmSync(path.join(phasesDir, entry.name), { recursive: true, force: true }); cleared++; } diff --git a/get-shit-done/workflows/new-milestone.md b/get-shit-done/workflows/new-milestone.md index 9a88dff59..b3178a570 100644 --- a/get-shit-done/workflows/new-milestone.md +++ b/get-shit-done/workflows/new-milestone.md @@ -140,7 +140,7 @@ Delete MILESTONE-CONTEXT.md if exists (consumed). Clear leftover phase directories from the previous milestone: ```bash -node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" phases clear +node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" phases clear --confirm ``` ```bash diff --git a/tests/bug-1826-phases-clear-confirm.test.cjs b/tests/bug-1826-phases-clear-confirm.test.cjs new file mode 100644 index 000000000..b45c924ee --- /dev/null +++ b/tests/bug-1826-phases-clear-confirm.test.cjs @@ -0,0 +1,72 @@ +/** + * Regression tests for bug #1826 + * + * `phases clear` must require an explicit --confirm flag before deleting any + * phase directories. Without it, any accidental or hallucinated invocation + * wipes the entire .planning/phases/ tree with no warning. + * + * Rules: + * - Phase dirs present + no --confirm → non-zero exit, clear error message + * - Phase dirs present + --confirm → deletes, exits 0, reports count + * - No phase dirs + no --confirm → exits 0, cleared=0 (nothing to guard) + */ + +'use strict'; + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +describe('bug #1826: phases clear --confirm guard', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('phases clear without --confirm is rejected when phase dirs exist', () => { + const phasesDir = path.join(tmpDir, '.planning', 'phases'); + fs.mkdirSync(path.join(phasesDir, '01-foundation'), { recursive: true }); + fs.mkdirSync(path.join(phasesDir, '02-api'), { recursive: true }); + + const result = runGsdTools(['phases', 'clear'], tmpDir); + + assert.ok(!result.success, 'should exit non-zero when dirs exist and --confirm absent'); + assert.ok( + result.error.includes('--confirm'), + `error message must mention --confirm; got: ${result.error}` + ); + + // Dirs must be untouched + assert.ok(fs.existsSync(path.join(phasesDir, '01-foundation')), 'dirs must not be deleted'); + assert.ok(fs.existsSync(path.join(phasesDir, '02-api')), 'dirs must not be deleted'); + }); + + test('phases clear --confirm deletes dirs and reports count', () => { + const phasesDir = path.join(tmpDir, '.planning', 'phases'); + fs.mkdirSync(path.join(phasesDir, '01-foundation'), { recursive: true }); + fs.mkdirSync(path.join(phasesDir, '02-api'), { recursive: true }); + + const result = runGsdTools(['phases', 'clear', '--confirm'], tmpDir); + + assert.ok(result.success, `should succeed with --confirm: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.cleared, 2); + assert.ok(!fs.existsSync(path.join(phasesDir, '01-foundation')), 'dirs should be removed'); + }); + + test('phases clear without --confirm succeeds when no phase dirs exist', () => { + // .planning/phases/ exists but is empty — nothing to guard + const result = runGsdTools(['phases', 'clear'], tmpDir); + + assert.ok(result.success, `should succeed with empty phases dir: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.cleared, 0); + }); +}); diff --git a/tests/new-milestone-clear-phases.test.cjs b/tests/new-milestone-clear-phases.test.cjs index be6e016c9..82478a2a0 100644 --- a/tests/new-milestone-clear-phases.test.cjs +++ b/tests/new-milestone-clear-phases.test.cjs @@ -35,7 +35,7 @@ describe('phases clear command', () => { fs.writeFileSync(path.join(phase1, '01-01-PLAN.md'), '# Plan'); fs.writeFileSync(path.join(phase2, '02-01-SUMMARY.md'), '# Summary'); - const result = runGsdTools('phases clear', tmpDir); + const result = runGsdTools('phases clear --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -54,7 +54,7 @@ describe('phases clear command', () => { const phasesDir = path.join(tmpDir, '.planning', 'phases'); // createTempProject creates the directory but leaves it empty - const result = runGsdTools('phases clear', tmpDir); + const result = runGsdTools('phases clear --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -66,7 +66,7 @@ describe('phases clear command', () => { // Remove the phases directory entirely fs.rmSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true, force: true }); - const result = runGsdTools('phases clear', tmpDir); + const result = runGsdTools('phases clear --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -83,7 +83,7 @@ describe('phases clear command', () => { fs.mkdirSync(phase1, { recursive: true }); fs.writeFileSync(path.join(phase1, '01-01-PLAN.md'), '# Plan'); - const result = runGsdTools('phases clear', tmpDir); + const result = runGsdTools('phases clear --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -103,7 +103,7 @@ describe('phases clear command', () => { fs.mkdirSync(nested, { recursive: true }); fs.writeFileSync(path.join(nested, 'deep-file.md'), '# Deep'); - const result = runGsdTools('phases clear', tmpDir); + const result = runGsdTools('phases clear --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); assert.ok(!fs.existsSync(phase1), 'phase directory including nested content should be removed');