From 5195a36cc539c56b0be1461ae97703cb88bf5ea2 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 2 Jul 2026 10:25:31 -0400 Subject: [PATCH] fix(#1871): phases clear archives dirs instead of destroying them (#1919) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cmdPhasesClear hard-deleted committed phase directories (rmSync) with no archive, so browsable phase history was silently lost at a milestone switch. The #1447 dirty-tree guard was a no-op for the common committed case (a clean tree passes the guard, then rmSync destroyed the dirs, leaving orphaned uncommitted deletions and no archive). Archive-then-remove: move each non-999 phase dir to milestones/-phases/ (version from getMilestoneInfo; timestamp fallback; collision-safe) using retryRenameSync — mirroring the existing archivePhases path in cmdMilestoneComplete. The #1447 uncommitted-changes guard is retained as a secondary backstop. Tests in tests/new-milestone-clear-phases.test.cjs updated: phase content is asserted to SURVIVE in milestones/*-phases/ (archived), not be destroyed — including the committed-dirs case that previously codified the no-op. Closes #1871 --- .changeset/humble-zebras-zip.md | 5 ++++ src/milestone.cts | 26 ++++++++++++++-- tests/new-milestone-clear-phases.test.cjs | 36 +++++++++++++++++++++-- 3 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 .changeset/humble-zebras-zip.md diff --git a/.changeset/humble-zebras-zip.md b/.changeset/humble-zebras-zip.md new file mode 100644 index 000000000..593da8622 --- /dev/null +++ b/.changeset/humble-zebras-zip.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1919 +--- +**`phases clear` archives phase directories instead of destroying them** — at a milestone switch, committed phase directories were hard-deleted (`rmSync`) with no archive, silently losing browsable phase history (the #1447 dirty-tree guard was a no-op for the common committed case). Phase directories are now moved to `milestones/-phases/` (collision-safe; timestamp fallback when no version resolves), so history survives the switch. The #1447 uncommitted-changes guard is retained as a secondary backstop. (#1871) diff --git a/src/milestone.cts b/src/milestone.cts index 58a0ad9e5..2c91429e9 100644 --- a/src/milestone.cts +++ b/src/milestone.cts @@ -26,7 +26,7 @@ import phaseIdMod = require('./phase-id.cjs'); const { escapeRegex, normalizePhaseName, phaseTokenMatches } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserMod = require('./roadmap-parser.cjs'); -const { getMilestonePhaseFilter, extractCurrentMilestone } = roadmapParserMod; +const { getMilestonePhaseFilter, extractCurrentMilestone, getMilestoneInfo } = roadmapParserMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import coreUtilsMod = require('./core-utils.cjs'); const { extractOneLinerFromBody } = coreUtilsMod; @@ -440,8 +440,30 @@ function cmdPhasesClear(cwd: string, raw: boolean, args: string[]): void { } try { + // #1871: archive phase directories instead of destroying them. Move each + // non-999 dir to milestones/-phases/ so browsable phase history + // survives the milestone switch (previously rmSync hard-deleted committed + // dirs, leaving orphaned uncommitted deletions and no archive). + let archiveVersion: string | null = null; + try { + archiveVersion = getMilestoneInfo(cwd).version ?? null; + } catch { + /* ROADMAP/STATE unreadable — fall back to a dated label */ + } + if (!archiveVersion) { + archiveVersion = `archived-${new Date().toISOString().replace(/[-:T]/g, '').slice(0, 8)}`; + } + const archivePhasesDir = path.join(planningPaths(cwd).planning, 'milestones', `${archiveVersion}-phases`); + platformEnsureDir(archivePhasesDir); for (const entry of dirs) { - fs.rmSync(path.join(phasesDir, entry.name), { recursive: true, force: true }); + const src = path.join(phasesDir, entry.name); + // Collision-safe: if a same-named archive entry exists (re-run), suffix it. + let dest = path.join(archivePhasesDir, entry.name); + let n = 1; + while (fs.existsSync(dest)) { + dest = path.join(archivePhasesDir, `${entry.name}.${n++}`); + } + retryRenameSync(src, dest); cleared++; } } catch (e) { diff --git a/tests/new-milestone-clear-phases.test.cjs b/tests/new-milestone-clear-phases.test.cjs index 8aa0ad752..7da7c14da 100644 --- a/tests/new-milestone-clear-phases.test.cjs +++ b/tests/new-milestone-clear-phases.test.cjs @@ -101,7 +101,7 @@ describe('phases clear command', () => { ); }); - test('clears nested phase content (recursive delete)', () => { + test('archives nested phase content (moved, not deleted) (#1871)', () => { const phasesDir = path.join(tmpDir, '.planning', 'phases'); const phase1 = path.join(phasesDir, '01-foundation'); const nested = path.join(phase1, 'subdir'); @@ -111,10 +111,33 @@ describe('phases clear command', () => { 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'); + // Source is cleared (moved away)... + assert.ok(!fs.existsSync(phase1), 'phase directory should be moved out of .planning/phases/'); + // ...but the nested content SURVIVES in the archive (not destroyed). + const archive = findPhasesArchive(tmpDir); + assert.ok(archive, 'an archive dir milestones/*-phases/ should exist'); + assert.ok( + fs.existsSync(path.join(archive, '01-foundation', 'subdir', 'deep-file.md')), + 'nested phase content must be preserved in the archive, not deleted', + ); }); }); +// Locate the `milestones/-phases/` archive directory created by phases clear. +function findPhasesArchive(tmpDir) { + const milestonesDir = path.join(tmpDir, '.planning', 'milestones'); + try { + for (const entry of fs.readdirSync(milestonesDir, { withFileTypes: true })) { + if (entry.isDirectory() && /-phases$/.test(entry.name)) { + return path.join(milestonesDir, entry.name); + } + } + } catch { + /* no milestones dir */ + } + return null; +} + // ─── #1447: uncommitted-changes guard ─────────────────────────────────────── describe('phases clear: uncommitted-changes guard (#1447)', () => { @@ -190,7 +213,14 @@ describe('phases clear: uncommitted-changes guard (#1447)', () => { assert.ok(result.success, `should succeed when phase files are committed: ${result.error}`); const output = JSON.parse(result.output); assert.strictEqual(output.cleared, 1, 'should clear 1 phase directory'); - assert.ok(!fs.existsSync(phase1), 'committed phase directory should be removed'); + // #1871: a committed phase dir is ARCHIVED (moved to milestones/*-phases/), not destroyed. + assert.ok(!fs.existsSync(phase1), 'committed phase directory should be moved out of .planning/phases/'); + const archive = findPhasesArchive(tmpDir); + assert.ok(archive, 'a milestones/*-phases/ archive should be created for committed phase dirs'); + assert.ok( + fs.existsSync(path.join(archive, '01-foundation', 'PLAN.md')), + 'committed phase content must be preserved in the archive, not hard-deleted', + ); }); test('guard skips gracefully when not in a git repo (no guard, proceeds normally)', () => {