fix(#1871): phases clear archives dirs instead of destroying them (#1919)

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/<version>-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
This commit is contained in:
Tom Boucher
2026-07-02 10:25:31 -04:00
committed by GitHub
parent 88da609b3e
commit 5195a36cc5
3 changed files with 62 additions and 5 deletions

View File

@@ -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/<version>-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)

View File

@@ -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/<version>-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) {

View File

@@ -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/<version>-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)', () => {