From e4aca8dab0ab6b4c8e4138e659d6e44542dfeb9a Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 27 May 2026 22:57:42 -0400 Subject: [PATCH] fix(#416): return null when active milestone has no archive; tighten **Milestone:** regex (#419) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#416): return null when active milestone has no archive (no fall-through to prior milestone's archive) Co-Authored-By: Claude Sonnet 4.6 * fix(#416): handle bold-formatted Milestone: field in archive dir resolver The STATE.md regex in getActiveMilestoneArchiveDir failed to extract the version from **Milestone:** vX.Y format (bold wraps the label+colon). The old pattern captured '**' instead of the version, causing the milestone→archive lookup to produce a false candidate path, then return null (post-fix behavior) instead of falling through to the version-sort fallback — breaking the #3164 consistency scanner tests. Fix: extend the regex to skip optional trailing '**' after the colon so both 'milestone: vX.Y' and '**Milestone:** vX.Y' parse correctly. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .../416-active-milestone-archive-null.md | 5 + get-shit-done/bin/lib/verify.cjs | 24 +- tests/bug-416-archive-dir-null.test.cjs | 212 ++++++++++++++++++ 3 files changed, 236 insertions(+), 5 deletions(-) create mode 100644 .changeset/416-active-milestone-archive-null.md create mode 100644 tests/bug-416-archive-dir-null.test.cjs diff --git a/.changeset/416-active-milestone-archive-null.md b/.changeset/416-active-milestone-archive-null.md new file mode 100644 index 000000000..bb7940716 --- /dev/null +++ b/.changeset/416-active-milestone-archive-null.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 416 +--- +`getActiveMilestoneArchiveDir` no longer falls back to the newest archive directory when the active milestone has no archive yet — returns `null` so the verifier no longer reports prior-milestone phases as "active" (eliminates W007 false positives during the flat→archive transition). diff --git a/get-shit-done/bin/lib/verify.cjs b/get-shit-done/bin/lib/verify.cjs index 4383e7b57..59f332a6c 100644 --- a/get-shit-done/bin/lib/verify.cjs +++ b/get-shit-done/bin/lib/verify.cjs @@ -441,24 +441,38 @@ function forEachArchivedPhaseToken(planBase, onPhase) { } function getActiveMilestoneArchiveDir(planBase) { + // Knuth invariant: the resolver answers exactly one question — + // "what archive directory holds the active milestone's phases?" + // Answer space: | null. + // + // When STATE.md is present and names a milestone: + // - If a matching milestones/-phases/ directory exists → return it. + // - If no matching directory exists → return null. The active milestone + // has no archive yet (phases live in flat phases/). Falling through to + // an older milestone's archive is wrong and produces W007 false positives. + // + // The version-sort fallback to the newest archive fires ONLY when STATE.md is + // absent or unparseable — not when it cleanly names an unarchived milestone. + const archiveDirs = listMilestoneArchiveDirs(planBase); if (archiveDirs.length === 0) return null; - // Prefer STATE.md milestone when it maps to an on-disk archive dir. + // STATE.md present and parseable: match wins, no-match returns null. try { const statePath = path.join(planBase, 'STATE.md'); if (fs.existsSync(statePath)) { const state = fs.readFileSync(statePath, 'utf-8'); - const m = state.match(/^\s*(?:\*\*)?milestone(?:\*\*)?:\s*([^\s\r\n#]+).*$/mi); + const m = state.match(/^\s*(?:\*\*)?milestone(?:\*\*)?:\s*\*{0,2}\s*([^\s*\r\n#][^\s\r\n#]*)/mi); if (m && m[1]) { const milestone = m[1].trim(); const candidate = path.join(planBase, 'milestones', `${milestone}-phases`); - if (archiveDirs.includes(candidate)) return candidate; + // Return the matching archive, or null if the active milestone has no archive yet. + return archiveDirs.includes(candidate) ? candidate : null; } } - } catch { /* intentionally empty */ } + } catch { /* intentionally empty — fall through to version-sort below */ } - // Fallback when STATE.md is absent/stale: highest (most recent) archive by version-ish name. + // Fallback: STATE.md is absent or unparseable — highest (most recent) archive by version-ish name. return archiveDirs[archiveDirs.length - 1]; } diff --git a/tests/bug-416-archive-dir-null.test.cjs b/tests/bug-416-archive-dir-null.test.cjs new file mode 100644 index 000000000..6c67caf32 --- /dev/null +++ b/tests/bug-416-archive-dir-null.test.cjs @@ -0,0 +1,212 @@ +'use strict'; + +/** + * Regression tests for issue #416 (open-gsd/get-shit-done-redux). + * + * Bug: getActiveMilestoneArchiveDir falls back to the newest archive directory + * when STATE.md names a milestone that has no matching archive yet, producing + * W007 false positives for phases from a prior (completed) milestone. + * + * Fix: when STATE.md is present and parseable and names a milestone, but no + * milestones/-phases/ directory matches, return null. The version-sort + * fallback to the newest archive fires only when STATE.md is absent or + * unparseable. + * + * Knuth invariant: the resolver answers one question — + * "what archive directory holds the active milestone's phases?" + * Answer space: | null. + */ + +process.env.GSD_TEST_MODE = '1'; + +const { describe, test, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); + +const { runGsdTools } = require('./helpers.cjs'); + +// ── helpers ────────────────────────────────────────────────────────────────── + +function mkplanning(base) { + const planningDir = path.join(base, '.planning'); + const phasesDir = path.join(planningDir, 'phases'); + fs.mkdirSync(phasesDir, { recursive: true }); + return planningDir; +} + +function writeMinimalRoadmap(planningDir, phases) { + // phases: array of { num, name, checked } + const checkboxes = phases.map(({ num, name, checked }) => + `- [${checked ? 'x' : ' '}] **Phase ${num}:** ${name}`, + ).join('\n'); + const headings = phases.map(({ num, name }) => + `### Phase ${num}: ${name}\n**Goal:** Completed.\n`, + ).join('\n'); + fs.writeFileSync( + path.join(planningDir, 'ROADMAP.md'), + `# Roadmap\n\n${checkboxes}\n\n${headings}`, + ); +} + +function writeStateMdMilestone(planningDir, milestone) { + fs.writeFileSync( + path.join(planningDir, 'STATE.md'), + `# State\n\n**milestone:** ${milestone}\n**Current Phase:** 23\n**Status:** In progress\n`, + ); +} + +function writeProjectMd(planningDir) { + fs.writeFileSync( + path.join(planningDir, 'PROJECT.md'), + '# Project\n\n## What This Is\nTest.\n\n## Core Value\nTest.\n\n## Requirements\nTest.\n', + ); +} + +function writeConfigJson(planningDir) { + fs.writeFileSync( + path.join(planningDir, 'config.json'), + JSON.stringify({ model_profile: 'balanced' }), + ); +} + +function mkArchivePhases(planningDir, version, phaseNums) { + // Creates .planning/milestones/-phases/-phase-name/ dirs + const archiveDir = path.join(planningDir, 'milestones', `${version}-phases`); + for (const num of phaseNums) { + const padded = String(num).padStart(2, '0'); + fs.mkdirSync(path.join(archiveDir, `${padded}-phase-${num}`), { recursive: true }); + } + return archiveDir; +} + +// ───────────────────────────────────────────────────────────────────────────── +// Case 1: STATE.md milestone: v6.0, only v5.0-phases/ on disk +// → resolver returns null, verifier emits zero W007 for phases 17–22 +// ───────────────────────────────────────────────────────────────────────────── + +describe('bug #416 case 1: STATE.md v6.0 with only v5.0-phases/ on disk → null, no W007', () => { + let tmpDir; + + before(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-416-c1-')); + const planningDir = mkplanning(tmpDir); + writeProjectMd(planningDir); + writeConfigJson(planningDir); + + // Active milestone is v6.0 — no archive for it yet (phases live in flat phases/) + writeStateMdMilestone(planningDir, 'v6.0'); + + // v5.0 was the prior completed milestone; its archive exists on disk + mkArchivePhases(planningDir, 'v5.0', [17, 18, 19, 20, 21, 22]); + + // ROADMAP reflects only v6.0 phases (v5.0 phases are in a prior milestone, + // not in the current roadmap section) + writeMinimalRoadmap(planningDir, [ + { num: 23, name: 'New Foundation', checked: false }, + ]); + }); + + after(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); + + test('validate health emits zero W007 warnings (no prior-milestone phases surfaced)', () => { + const result = runGsdTools(['validate', 'health', '--json'], tmpDir); + assert.strictEqual(result.success, true, `validate health failed: ${result.error}`); + const data = JSON.parse(result.output); + const w007 = (data.warnings ?? []).filter((w) => w.code === 'W007'); + assert.strictEqual( + w007.length, + 0, + `Expected zero W007 — phases 17–22 from v5.0-phases/ must not appear as "active".\nGot: ${JSON.stringify(w007, null, 2)}`, + ); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Case 2: No STATE.md, multiple archives on disk → version-sort fallback +// returns the highest-versioned archive (existing behavior preserved) +// ───────────────────────────────────────────────────────────────────────────── + +describe('bug #416 case 2: no STATE.md + multiple archives → version-sort fallback to newest', () => { + let tmpDir; + + before(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-416-c2-')); + const planningDir = mkplanning(tmpDir); + writeProjectMd(planningDir); + writeConfigJson(planningDir); + + // No STATE.md — resolver must use the version-sort fallback + // Two archives: v4.0 and v5.0; v5.0 is newer + mkArchivePhases(planningDir, 'v4.0', [10, 11, 12]); + mkArchivePhases(planningDir, 'v5.0', [17, 18, 19]); + + // ROADMAP lists v5.0 phases so W007 does not fire + writeMinimalRoadmap(planningDir, [ + { num: 17, name: 'Alpha', checked: true }, + { num: 18, name: 'Beta', checked: true }, + { num: 19, name: 'Gamma', checked: true }, + ]); + }); + + after(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); + + test('validate health succeeds and does not emit W007 for v5.0 archive phases', () => { + const result = runGsdTools(['validate', 'health', '--json'], tmpDir); + assert.strictEqual(result.success, true, `validate health failed: ${result.error}`); + const data = JSON.parse(result.output); + const w007 = (data.warnings ?? []).filter((w) => w.code === 'W007'); + // v5.0 phases (17–19) are in the archive returned by the fallback and + // in the ROADMAP, so no W007 should fire. + assert.strictEqual( + w007.length, + 0, + `Expected zero W007 for v5.0 archive phases present in ROADMAP.\nGot: ${JSON.stringify(w007, null, 2)}`, + ); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Case 3: STATE.md milestone: v5.0, matching v5.0-phases/ exists → returns it +// (regression guard — happy path must not break) +// ───────────────────────────────────────────────────────────────────────────── + +describe('bug #416 case 3: STATE.md v5.0 with matching v5.0-phases/ → returns archive dir', () => { + let tmpDir; + + before(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-416-c3-')); + const planningDir = mkplanning(tmpDir); + writeProjectMd(planningDir); + writeConfigJson(planningDir); + + // STATE.md names v5.0 and a matching archive exists + writeStateMdMilestone(planningDir, 'v5.0'); + mkArchivePhases(planningDir, 'v5.0', [17, 18, 19, 20, 21, 22]); + + // ROADMAP lists v5.0 phases so W007 does not fire + writeMinimalRoadmap(planningDir, [ + { num: 17, name: 'Alpha', checked: true }, + { num: 18, name: 'Beta', checked: true }, + { num: 19, name: 'Gamma', checked: true }, + { num: 20, name: 'Delta', checked: true }, + { num: 21, name: 'Epsilon', checked: true }, + { num: 22, name: 'Zeta', checked: true }, + ]); + }); + + after(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); + + test('validate health emits zero W007 — archive phases are in ROADMAP and active', () => { + const result = runGsdTools(['validate', 'health', '--json'], tmpDir); + assert.strictEqual(result.success, true, `validate health failed: ${result.error}`); + const data = JSON.parse(result.output); + const w007 = (data.warnings ?? []).filter((w) => w.code === 'W007'); + assert.strictEqual( + w007.length, + 0, + `Expected zero W007 for matching v5.0 archive with v5.0 in STATE.md.\nGot: ${JSON.stringify(w007, null, 2)}`, + ); + }); +});