fix: stale phase dirs corrupt phase counts; stopped_at overwritten by historical prose (#2459)

* fix(sdk): extractCurrentMilestone Backlog leak + state.begin-phase flag parsing

Closes #2422
Closes #2420

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(#2444,#2445): scope stopped_at extraction to Session section; filter stale phase dirs

- buildStateFrontmatter now extracts stopped_at only from the ## Session
  section when one exists, preventing historical prose elsewhere in the
  body (e.g. "Stopped at: Phase 5 complete" in old notes) from overwriting
  the current value in frontmatter (bug #2444)
- buildStateFrontmatter de-duplicates phase dirs by normalized phase number
  before computing plan/phase counts, so stale phase dirs from a prior
  milestone with the same phase numbers as the new milestone don't inflate
  totals (bug #2445)
- cmdInitNewMilestone now filters phase dirs through getMilestonePhaseFilter
  so phase_dir_count excludes stale prior-milestone dirs (bug #2445)
- Tests: 4 tests in state.test.cjs covering both bugs

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-04-20 10:08:43 -04:00
committed by GitHub
parent 5afcd5577e
commit 53b9fba324
3 changed files with 258 additions and 3 deletions

View File

@@ -458,8 +458,11 @@ function cmdInitNewMilestone(cwd, raw) {
try {
if (fs.existsSync(phasesDir)) {
// Bug #2445: filter phase dirs to current milestone only so stale dirs
// from a prior milestone that were not archived don't inflate the count.
const isDirInMilestone = getMilestonePhaseFilter(cwd);
phaseDirCount = fs.readdirSync(phasesDir, { withFileTypes: true })
.filter(entry => entry.isDirectory())
.filter(entry => entry.isDirectory() && isDirInMilestone(entry.name))
.length;
}
} catch {}

View File

@@ -720,7 +720,13 @@ function buildStateFrontmatter(bodyContent, cwd) {
const status = stateExtractField(bodyContent, 'Status');
const progressRaw = stateExtractField(bodyContent, 'Progress');
const lastActivity = stateExtractField(bodyContent, 'Last Activity');
const stoppedAt = stateExtractField(bodyContent, 'Stopped At') || stateExtractField(bodyContent, 'Stopped at');
// Bug #2444: scope Stopped At extraction to the ## Session section so that
// historical "Stopped at:" prose elsewhere in the body (e.g. in a
// Session Continuity Archive section) never overwrites the current value.
// Fall back to full-body search only when no ## Session section exists.
const sessionSectionMatch = bodyContent.match(/##\s*Session\s*\n([\s\S]*?)(?=\n##|$)/i);
const sessionBodyScope = sessionSectionMatch ? sessionSectionMatch[1] : bodyContent;
const stoppedAt = stateExtractField(sessionBodyScope, 'Stopped At') || stateExtractField(sessionBodyScope, 'Stopped at');
const pausedAt = stateExtractField(bodyContent, 'Paused At');
let milestone = null;
@@ -747,9 +753,33 @@ function buildStateFrontmatter(bodyContent, cwd) {
let cached = _diskScanCache.get(cwd);
if (!cached) {
const isDirInMilestone = getMilestonePhaseFilter(cwd);
const phaseDirs = fs.readdirSync(phasesDir, { withFileTypes: true })
const allMatchingDirs = fs.readdirSync(phasesDir, { withFileTypes: true })
.filter(e => e.isDirectory()).map(e => e.name)
.filter(isDirInMilestone);
// Bug #2445: when stale phase dirs from a prior milestone remain in
// .planning/phases/ alongside new dirs with the same phase number,
// de-duplicate by normalized phase number keeping the most recently
// modified dir. This prevents double-counting (e.g. two "Phase 1" dirs).
const seenPhaseNums = new Map(); // normalizedNum -> dirName
for (const dir of allMatchingDirs) {
const m = dir.match(/^0*(\d+[A-Za-z]?(?:\.\d+)*)/);
const key = m ? m[1].toLowerCase() : dir;
if (!seenPhaseNums.has(key)) {
seenPhaseNums.set(key, dir);
} else {
// Keep the dir that is newer on disk (more likely current milestone)
try {
const existing = path.join(phasesDir, seenPhaseNums.get(key));
const candidate = path.join(phasesDir, dir);
if (fs.statSync(candidate).mtimeMs > fs.statSync(existing).mtimeMs) {
seenPhaseNums.set(key, dir);
}
} catch { /* keep existing on stat error */ }
}
}
const phaseDirs = [...seenPhaseNums.values()];
let diskTotalPlans = 0;
let diskTotalSummaries = 0;
let diskCompletedPhases = 0;

View File

@@ -2148,6 +2148,228 @@ describe('state sync command', () => {
});
});
// ─────────────────────────────────────────────────────────────────────────────
// Bug #2444: stopped_at frontmatter must not be overwritten by historical body prose
// ─────────────────────────────────────────────────────────────────────────────
describe('stopped_at frontmatter not overwritten by historical prose (bug #2444)', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject();
});
afterEach(() => {
cleanup(tmpDir);
});
test('state sync preserves correct stopped_at frontmatter when historical plain-text match appears before Session section', () => {
// The bug: body has plain "Stopped at:" in old notes (no bold) — stateExtractField
// uses a plain ^Stopped at:\s*(.+) pattern with /im which matches the first line,
// returning the stale historical value. syncStateFrontmatter has no preservation
// step for stopped_at like cmdStateJson does, so it overwrites the correct value.
fs.writeFileSync(
path.join(tmpDir, '.planning', 'STATE.md'),
`---
gsd_state_version: '1.0'
status: executing
stopped_at: Phase 3, Plan 2 — current correct value
---
# Project State
**Current Phase:** 03
**Status:** In progress
## Previous Session Notes
Stopped at: Phase 5 complete — v1.0 shipped (OLD stale historical note)
## Session
Last Date: 2026-04-19
Stopped At: Phase 3, Plan 2 — current correct value
`
);
const result = runGsdTools('state sync', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const stateContent = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
// The correct frontmatter value must survive the sync
assert.ok(
stateContent.includes('Phase 3, Plan 2 — current correct value'),
'stopped_at must retain the correct value from the ## Session section'
);
assert.ok(
!stateContent.includes('stopped_at: Phase 5 complete'),
'stopped_at must NOT be overwritten with the old historical note'
);
});
test('state sync does not promote stale body prose to stopped_at frontmatter when frontmatter has no stopped_at', () => {
// No existing stopped_at in frontmatter, body has plain Stopped at: in
// a historical notes section appearing BEFORE the real ## Session entry.
// buildStateFrontmatter should scope extraction to ## Session section, not
// match the first occurrence anywhere in the body.
fs.writeFileSync(
path.join(tmpDir, '.planning', 'STATE.md'),
`---
gsd_state_version: '1.0'
status: executing
---
# Project State
**Current Phase:** 03
**Status:** In progress
## Old Notes
Stopped at: Phase 5 complete — v1.0 STALE (should never land in frontmatter)
## Session
Last Date: 2026-04-19
Stopped At: Phase 3, Plan 1 — real current value
`
);
const syncResult = runGsdTools('state sync', tmpDir);
assert.ok(syncResult.success, `state sync failed: ${syncResult.error}`);
const jsonResult = runGsdTools('state json', tmpDir);
assert.ok(jsonResult.success, `state json failed: ${jsonResult.error}`);
const output = JSON.parse(jsonResult.output);
assert.strictEqual(output.stopped_at, 'Phase 3, Plan 1 — real current value',
'stopped_at must be extracted from ## Session section, not the first plain-text match in the body');
});
});
// ─────────────────────────────────────────────────────────────────────────────
// Bug #2445: stale phase dirs from closed milestone inflate phase counts
// ─────────────────────────────────────────────────────────────────────────────
describe('stale phase dirs do not corrupt phase counts (bug #2445)', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject();
});
afterEach(() => {
cleanup(tmpDir);
});
test('state json excludes stale prior-milestone phase dirs from phase count when ROADMAP scopes current milestone', () => {
// Old milestone had phases 1-5; new milestone starts fresh with phases 1-2.
// Stale dirs for old phases 3, 4, 5 remain in .planning/phases/ and must be
// excluded by getMilestonePhaseFilter (new ROADMAP only lists phases 1 and 2).
// Old phases 1 and 2 dirs are ambiguous (same number reused) but phase 3-5 dirs
// must not inflate total_phases beyond the ROADMAP's phaseCount of 2.
fs.writeFileSync(
path.join(tmpDir, '.planning', 'ROADMAP.md'),
[
'# Roadmap',
'',
'<details>',
'<summary>v1.0 — Old Milestone (Shipped)</summary>',
'',
'## Roadmap v1.0: Old Milestone',
'### Phase 1: Old Foundation',
'### Phase 2: Old API',
'### Phase 3: Old Deploy',
'### Phase 4: Old Polish',
'### Phase 5: Old Wrap',
'',
'</details>',
'',
'## Roadmap v2.0: New Milestone',
'### Phase 1: New Foundation',
'### Phase 2: New API',
].join('\n')
);
fs.writeFileSync(
path.join(tmpDir, '.planning', 'STATE.md'),
'---\nmilestone: v2.0\n---\n\n# State\n\n**Current Phase:** 01\n**Status:** Planning\n'
);
// Create stale v1.0 phase dirs 3, 4, 5 — these are NOT in the new ROADMAP
const phasesDir = path.join(tmpDir, '.planning', 'phases');
for (const dir of ['03-old-deploy', '04-old-polish', '05-old-wrap']) {
const d = path.join(phasesDir, dir);
fs.mkdirSync(d, { recursive: true });
fs.writeFileSync(path.join(d, `${dir.slice(0, 2)}-01-PLAN.md`), '# stale plan\n');
}
// New milestone has only Phase 1 started so far
const newPhaseDir = path.join(phasesDir, '01-new-foundation');
fs.mkdirSync(newPhaseDir, { recursive: true });
fs.writeFileSync(path.join(newPhaseDir, '01-01-PLAN.md'), '# new plan\n');
const result = runGsdTools('state json', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
// total_phases must be bounded by the ROADMAP's 2 phases, not 4 total dirs
// (the 3 stale dirs for phases 3-5 must be excluded by the milestone filter)
assert.ok(
output.progress && output.progress.total_phases <= 2,
`total_phases should be ≤ 2 (new milestone phases 1-2 only), got ${output.progress?.total_phases}`
);
// total_plans must only count plans from current-milestone phase dirs
assert.ok(
output.progress && output.progress.total_plans <= 1,
`total_plans should be 1 (only new phase 1 dir), got ${output.progress?.total_plans}`
);
});
test('init new-milestone phase_dir_count excludes stale prior-milestone dirs', () => {
// ROADMAP scoped to v2.0 with 2 phases
fs.writeFileSync(
path.join(tmpDir, '.planning', 'ROADMAP.md'),
[
'# Roadmap',
'',
'<details>',
'<summary>v1.0 — Shipped</summary>',
'',
'## Roadmap v1.0: Old',
'### Phase 1: Old One',
'### Phase 2: Old Two',
'### Phase 3: Old Three',
'',
'</details>',
'',
'## Roadmap v2.0: New',
'### Phase 1: New One',
'### Phase 2: New Two',
].join('\n')
);
fs.writeFileSync(
path.join(tmpDir, '.planning', 'STATE.md'),
'---\nmilestone: v2.0\n---\n\n# State\n\n**Status:** Planning\n'
);
// Three stale phase dirs from the old milestone
const phasesDir = path.join(tmpDir, '.planning', 'phases');
for (const dir of ['01-old-one', '02-old-two', '03-old-three']) {
fs.mkdirSync(path.join(phasesDir, dir), { recursive: true });
}
const result = runGsdTools('init new-milestone', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
// phase_dir_count should not include stale dirs from the old milestone
assert.ok(
output.phase_dir_count <= 2,
`phase_dir_count should be ≤ 2 (only new-milestone dirs), got ${output.phase_dir_count}`
);
});
});
// ─────────────────────────────────────────────────────────────────────────────
// summary-extract command
// ─────────────────────────────────────────────────────────────────────────────