Skip to content

fix(milestone): correct version detection, stats scoping, and entry ordering#2

Closed
Tibsfox wants to merge 3 commits intomainfrom
fix/milestone-completion-bugs
Closed

fix(milestone): correct version detection, stats scoping, and entry ordering#2
Tibsfox wants to merge 3 commits intomainfrom
fix/milestone-completion-bugs

Conversation

@Tibsfox
Copy link
Owner

@Tibsfox Tibsfox commented Feb 28, 2026

Summary

This PR fixes three interrelated bugs in the milestone complete workflow that caused incorrect behavior when completing milestones in projects with a multi-milestone history. These bugs were discovered during production use of GSD on a project that had shipped 50+ milestones.

Bug 1 — Wrong version detection (gsd-build#768): After completing a milestone, getMilestoneInfo() would return the version of a previously shipped milestone instead of the current active one. This happened because shipped milestones were collapsed into <details> blocks in ROADMAP.md but their ## Roadmap vX.Y: headings still matched the regex first.

Bug 2 — Inflated stats and stale accomplishments: The milestone complete command counted all phase directories on disk — including phases from prior milestones that had not been archived. This inflated phase counts, plan counts, and polluted the accomplishments list with work from previous milestones.

Bug 3 — Wrong chronological order in MILESTONES.md: New milestone entries were appended to the bottom of MILESTONES.md, producing oldest-first ordering. Users expect reverse chronological order (newest first), matching the convention used by changelogs and release history files.

Root Cause

Bug 1 (core.cjsgetMilestoneInfo): The version and name were extracted with two independent regexes that both scanned the entire ROADMAP.md file, including <details> blocks containing shipped milestones:

// BEFORE (buggy) — matches first v\d+\.\d+ in the file, which is the shipped milestone
const versionMatch = roadmap.match(/v(\d+\.\d+)/);
const nameMatch = roadmap.match(/## .*v\d+\.\d+[:\s]+([^\n(]+)/);

When a ROADMAP.md looked like this, the regex matched v0.1 from the collapsed <details> block instead of the active v0.2:

<details>
<summary>v0.1 — Legacy Feature Parity (Shipped)</summary>
## Roadmap v0.1: Legacy Feature Parity    <-- matched first!
</details>

## Roadmap v0.2: Dashboard Overhaul       <-- should have matched

Bug 2 (milestone.cjscmdMilestoneComplete): The stats-gathering loop iterated over every directory in .planning/phases/ with no filtering:

// BEFORE (buggy) — counts ALL phase directories, not just current milestone
for (const dir of dirs) {
  phaseCount++;    // ← counts phases from prior milestones too
  // ...
}

The same unscoped iteration applied to --archive-phases, which would move all phase directories (including ones belonging to other milestones) into the archive.

Bug 3 (milestone.cjs — MILESTONES.md write): New entries were simply appended:

// BEFORE (buggy) — appends to end, producing oldest-first ordering
fs.writeFileSync(milestonesPath, existing + '\n' + milestoneEntry, 'utf-8');

Fix

Commit 1: getMilestoneInfo() version detection

  • Strip all <details>...</details> blocks from the ROADMAP before regex matching, so shipped milestones collapsed in details blocks cannot interfere
  • Extract version and name from the same ## Roadmap vX.Y: Name heading match (single regex) instead of two independent matches, ensuring consistency
  • Fall back to a bare v\d+\.\d+ match (on the cleaned content) if no heading is found

Commit 2: Scope stats, accomplishments, and archive to current milestone phases

  • Parse ROADMAP.md for ### Phase N: headings to build a set of phase numbers belonging to the current milestone
  • Normalize phase numbers (strip leading zeros, lowercase letter suffixes) for O(1) lookup
  • Add isDirInMilestone(dirName) helper that maps directory names like 01-setup to phase number 1, 456-dacp to 456, 3A-bar to 3a
  • Gate the stats loop and the archive loop with isDirInMilestone() so only current-milestone phases are counted/moved
  • Graceful fallback: if ROADMAP.md has no phase headings, all directories are included (backward compatible)
  • Edge case: non-numeric directories (e.g., notes/) are excluded when scoping is active
  • Edge case: phase 1 does not prefix-match phase 10 (exact number match, not string prefix)

Commit 3: Reverse chronological MILESTONES.md insertion

  • Detect the header line(s) at the top of MILESTONES.md using /^(#{1,3}\s+[^\n]*\n\n?)/
  • Insert the new entry immediately after the header, before all existing entries
  • Fall back to prepending if no recognizable header is found

Relationship to Other PRs

This is PR 1 of 6 from the dev-bugfix branch, which collects bug fixes and enhancements discovered during production use:

PR Title Scope
#2 (this) Milestone completion bugs core.cjs, milestone.cjs
#3 Cross-platform Windows CI fixes CI, path handling
#4 Feature enhancements (Codex, discuss, agents) New features
#5 Agent frontmatter + heredoc fix Agent output parsing
#6 CLI/config bug fixes (9 issues) Various CLI modules
#1 MCP migration helper Targets dev-bugfix, not main

These PRs have no code overlap and can be merged independently in any order. This PR touches only get-shit-done/bin/lib/core.cjs and get-shit-done/bin/lib/milestone.cjs.

Testing

New tests added (10 test cases)

Test What it verifies
returns active milestone when shipped milestone is collapsed in details block Single <details> block with shipped milestone is stripped; active milestone is detected
returns active milestone when multiple shipped milestones exist in details blocks Multiple <details> blocks are all stripped; only the active heading is matched
returns defaults when roadmap has no heading matches Graceful fallback to v1.0 / milestone when no ## Roadmap vX.Y: heading exists
prepends to existing MILESTONES.md (reverse chronological) New entry appears before existing entries (updated from prior append test)
three sequential completions maintain reverse-chronological order After completing v1.0 → v1.1 → v1.2, entries appear in v1.2, v1.1, v1.0 order
scopes stats to current milestone phases only Phases 1-2 from prior milestone excluded; only phases 3-4 counted in stats and accomplishments
archive-phases only archives current milestone phases --archive-phases moves only current-milestone phase dirs; prior-milestone dirs remain in place
phase 1 in roadmap does NOT match directory 10-something Phase number 1 does not prefix-collide with phase directory 10-scaling
non-numeric directory is excluded when milestone scoping is active A notes/ directory inside phases/ is not counted as a phase
large phase numbers (456, 457) scope correctly Three-digit phase numbers match correctly; nearby phase 45 is excluded

Full suite results

# tests 428
# suites 79
# pass 428
# fail 0
# cancelled 0
# skipped 0
# duration_ms 4839ms

Impact

  • Multi-milestone projectsgetMilestoneInfo() now correctly returns the active milestone version even when ROADMAP.md contains shipped milestones in <details> blocks. This fixes downstream consumers that rely on version detection (e.g., discuss-phase, plan-phase, execute-phase).
  • Milestone completion stats — Phase counts, plan counts, and accomplishment lists now reflect only the work done in the current milestone, not accumulated totals from the entire project history. A project with 400+ historical phases and 2 current-milestone phases will correctly report "2 phases" instead of "402 phases".
  • MILESTONES.md ordering — New entries are inserted at the top (after the header), matching the reverse-chronological convention used by changelogs. Existing entries are preserved in their original positions.
  • Phase archival--archive-phases now moves only the current milestone's phase directories, leaving prior-milestone phases untouched for projects that retain them on disk.
  • Backward compatibility — All changes are backward compatible. If ROADMAP.md contains no <details> blocks, getMilestoneInfo() behaves identically. If ROADMAP.md contains no phase headings, the stats loop includes all directories (same as before). The only visible change for existing users is that MILESTONES.md entries are now prepended instead of appended.

Hnturk and others added 3 commits February 28, 2026 02:22
…ion (gsd-build#768)

Strip <details> blocks (shipped milestones) before matching, and extract
both version and name from the same ## heading for consistency.
…milestone phases

cmdMilestoneComplete previously iterated ALL phase directories in
.planning/phases/, including phases from prior milestones that remain on
disk. This produced inflated stats, stale accomplishments from old
summaries, and --archive-phases moving directories that belong to
earlier milestones.

This fix parses ROADMAP.md to extract phase numbers for the current
milestone, builds a normalized Set for O(1) lookup, and filters all
phase directory operations through an isDirInMilestone() helper.

The helper handles edge cases: leading zeros (01 -> 1), letter suffixes
(3A), decimal phases (3.1), large numbers (456), and excludes
non-numeric directories (notes/, misc/).

Adds 5 tests covering scoped stats, scoped archive, prefix collision
guard, non-numeric directory exclusion, and large phase numbers.

Complements upstream PRs gsd-build#756 and gsd-build#783 which fix getMilestoneInfo()
milestone detection — this fix addresses milestone completion scoping.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
… order

cmdMilestoneComplete previously appended new milestone entries at the
end of MILESTONES.md. Over successive milestones this produced
chronological order (oldest first), which is counterintuitive — users
expect the most recent milestone at the top.

This fix detects the file header (h1-h3) and inserts the new entry
immediately after it, pushing older entries down. Files without a
recognizable header get the entry prepended. New files still get a
default '# Milestones' header.

Adds 2 tests: single insertion ordering assertion and three-sequential-
completions verification (v1.2 < v1.1 < v1.0 in file order).

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@Tibsfox
Copy link
Owner Author

Tibsfox commented Mar 3, 2026

Code Logic Verification of PR #2

1. The Before State (Bug Paths)

Bug 1 (gsd-build#768) — Wrong version detection: getMilestoneInfo() used two independent regexes that scanned the entire ROADMAP.md including <details> blocks. Shipped milestones inside <details> matched first, returning the wrong version.

Bug 2 — Inflated stats: cmdMilestoneComplete() iterated ALL phase directories on disk with no filtering. Projects with prior-milestone phases on disk got inflated counts and polluted accomplishment lists.

Bug 3 — Wrong MILESTONES.md order: New entries appended to bottom (oldest-first). Should be reverse chronological (newest-first).

2. The After State (Fix Paths)

Fix 1 (core.cjs:392-425):

  1. First checks list-format 🚧 in-progress marker (line 398) — returns immediately if found
  2. Strips <details>...</details> blocks via roadmap.replace(/<details>[\s\S]*?<\/details>/gi, '') (line 407)
  3. Single regex captures version AND name from same ## heading (line 409) — no divergence possible
  4. Fallback to bare v\d+\.\d+ on cleaned content, then v1.0/milestone defaults

Fix 2 (milestone.cjs:104-143,235 + core.cjs:432-460):

  • Parses ### Phase N: headings from ROADMAP to build milestone phase set (line 108)
  • isDirInMilestone() helper strips leading zeros, lowercases, uses exact set membership — phase 1 cannot match dir 10-something
  • Stats loop (line 143) and archive loop (line 235) both gated by filter
  • Pass-all fallback when no phase headings found (backward compatible)
  • getMilestonePhaseFilter() exported from core.cjs for reuse

Fix 3 (milestone.cjs:190-204):

  • Detects header line(s) via /^(#{1,3}\s+[^\n]*\n\n?)/
  • Inserts new entry after header, before existing entries
  • Prepends if no header recognized; creates fresh file if nonexistent

3. Key Correctness Checks

Check Result
<details> blocks stripped before version match core.cjs:407 — case-insensitive, non-greedy
Version + name from same heading (no divergence) ✅ Single regex at core.cjs:409
🚧 in-progress fast-path for list-format core.cjs:398 — checked first
Phase scoping: exact set membership, not prefix ✅ Normalized set + has() at milestone.cjs:130
Phase 1 vs dir 10: no collision ✅ Test at milestone.test.cjs:328-368
Decimal phases (6.1) handled ✅ Test at core.test.cjs:754-765
Letter-suffix phases (3A) handled ✅ Test at core.test.cjs:741-752
Non-numeric dirs excluded ✅ Test at milestone.test.cjs:370-395
Large phase numbers (456, 457) ✅ Test at milestone.test.cjs:397-425
Pass-all fallback when no phase headings core.test.cjs:722-739
Reverse chronological ordering ✅ 3-completion sequence test at milestone.test.cjs:98-136
10 new tests + full suite 428 tests pass

Pre-existing edge cases identified (not introduced by this PR):

  • Nested <details> blocks: non-greedy regex matches to first </details>, potentially leaving inner content visible. Uncommon in practice.
  • Empty MILESTONES.md: treated as existing file without header, entry prepended without # Milestones title. Second completion would insert in wrong position.
  • Duplicated phase filter logic between milestone.cjs (inline) and core.cjs (getMilestonePhaseFilter) — maintenance risk, should refactor to single implementation.
  • .claude/ copy of core.cjs may lag behind get-shit-done/ copy — re-run install to sync.

4. Verdict

PASS — All three bugs correctly fixed. Phase scoping is robust with exact-match normalization. Version detection correctly handles <details> blocks and list-format roadmaps. Reverse chronological insertion works for standard MILESTONES.md format. Edge cases in nested <details> and empty files are low-risk.

Status: All 3 commits already present on main. Upstream PRs gsd-build#784, gsd-build#785 merged; gsd-build#809 closed. Closing as completed.

  • Tibsfox ^.^

@Tibsfox
Copy link
Owner Author

Tibsfox commented Mar 3, 2026

Closing — all 3 commits already merged to main. Upstream PRs gsd-build#784, gsd-build#785 merged; gsd-build#809 closed. Core Logic Verification posted above confirms PASS.

@Tibsfox Tibsfox closed this Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants