From b1278f6fc349a7b9250dba6ae9e8963c6c6bf792 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 24 Apr 2026 20:20:11 -0400 Subject: [PATCH] fix(#2674): align initProgress with initManager ROADMAP [x] precedence (#2681) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit initProgress computed phase status purely from disk (PLAN/SUMMARY counts), consulting the ROADMAP `- [x] Phase N` checkbox only for phases with no directory. initManager, by contrast, applied an explicit override: a ROADMAP `[x]` forces status to `complete` regardless of disk state. Result: a phase with a stub directory (no SUMMARY.md) and a ticked ROADMAP checkbox reported `complete` from /gsd-manager and `pending` from /gsd-progress — same data, different answer. Apply ROADMAP-[x]-wins as the unified policy inside initProgress, mirroring initManager's override. A user who typed `- [x] Phase 3` has made an explicit assertion; a leftover stub dir is the weaker signal. Adds sdk/src/query/init-progress-precedence.test.ts covering six cases (stub dir + [x], full dir + [x], full dir + [ ], stub dir + [ ], ROADMAP-only + [x], and completed_count parity). Pre-fix: cases 1 and 6 failed. Post-fix: all six pass. No existing tests were modified. Closes #2674 --- sdk/src/query/init-complex.ts | 13 +- .../query/init-progress-precedence.test.ts | 177 ++++++++++++++++++ 2 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 sdk/src/query/init-progress-precedence.test.ts diff --git a/sdk/src/query/init-complex.ts b/sdk/src/query/init-complex.ts index 73feffd7..f3c2a869 100644 --- a/sdk/src/query/init-complex.ts +++ b/sdk/src/query/init-complex.ts @@ -262,11 +262,22 @@ export const initProgress: QueryHandler = async (_args, projectDir, _workstream) const summaries = phaseFiles.filter(f => f.endsWith('-SUMMARY.md') || f === 'SUMMARY.md'); const hasResearch = phaseFiles.some(f => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md'); - const status = + let status = summaries.length >= plans.length && plans.length > 0 ? 'complete' : plans.length > 0 ? 'in_progress' : hasResearch ? 'researched' : 'pending'; + // #2674: align with initManager — a ROADMAP `- [x] Phase N` checkbox + // wins over disk state. A stub phase dir with no SUMMARY is leftover + // scaffolding; the user's explicit [x] is the authoritative signal. + const strippedNum = phaseNumber.replace(/^0+/, '') || '0'; + const roadmapComplete = + checkboxStates.get(phaseNumber) === true || + checkboxStates.get(strippedNum) === true; + if (roadmapComplete && status !== 'complete') { + status = 'complete'; + } + const phaseInfo: Record = { number: phaseNumber, name: phaseName, diff --git a/sdk/src/query/init-progress-precedence.test.ts b/sdk/src/query/init-progress-precedence.test.ts new file mode 100644 index 00000000..52b119e2 --- /dev/null +++ b/sdk/src/query/init-progress-precedence.test.ts @@ -0,0 +1,177 @@ +/** + * Regression guard for #2674. + * + * initProgress and initManager must agree on phase status given the same + * inputs. Specifically, a ROADMAP `- [x] Phase N` checkbox wins over disk + * state: a stub phase directory with no SUMMARY.md that is checked in + * ROADMAP reports as `complete` from both handlers. + * + * Pre-fix: initManager reported `complete` (explicit override at line ~451), + * initProgress reported `pending` (disk-only policy). This mismatch meant + * /gsd-manager and /gsd-progress disagreed on the same data. Post-fix: + * both apply the ROADMAP-[x]-wins policy. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtemp, writeFile, mkdir, rm } from 'node:fs/promises'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { initProgress, initManager } from './init-complex.js'; + +/** Find a phase by numeric value regardless of zero-padding ('3' vs '03'). */ +function findPhase( + phases: Record[], + num: number, +): Record | undefined { + return phases.find(p => parseInt(p.number as string, 10) === num); +} + +let tmpDir: string; + +const CONFIG = JSON.stringify({ + model_profile: 'balanced', + commit_docs: false, + git: { + branching_strategy: 'none', + phase_branch_template: 'gsd/phase-{phase}-{slug}', + milestone_branch_template: 'gsd/{milestone}-{slug}', + quick_branch_template: null, + }, + workflow: { research: true, plan_check: true, verifier: true, nyquist_validation: true }, +}); + +const STATE = [ + '---', + 'milestone: v1.0', + '---', +].join('\n'); + +/** + * Write a ROADMAP.md with the given phase list. Each entry is + * `{num, name, checked}`. Emits both the checkbox summary lines AND the + * `### Phase N:` heading sections (so initManager picks them up). + */ +async function writeRoadmap( + dir: string, + phases: Array<{ num: string; name: string; checked: boolean }>, +): Promise { + const checkboxes = phases + .map(p => `- [${p.checked ? 'x' : ' '}] Phase ${p.num}: ${p.name}`) + .join('\n'); + const sections = phases + .map(p => `### Phase ${p.num}: ${p.name}\n\n**Goal:** ${p.name} goal\n\n**Depends on:** None\n`) + .join('\n'); + await writeFile(join(dir, '.planning', 'ROADMAP.md'), [ + '# Roadmap', + '', + '## v1.0: Test', + '', + checkboxes, + '', + sections, + ].join('\n')); +} + +beforeEach(async () => { + tmpDir = await mkdtemp(join(tmpdir(), 'gsd-2674-')); + await mkdir(join(tmpDir, '.planning', 'phases'), { recursive: true }); + await writeFile(join(tmpDir, '.planning', 'config.json'), CONFIG); + await writeFile(join(tmpDir, '.planning', 'STATE.md'), STATE); +}); + +afterEach(async () => { + await rm(tmpDir, { recursive: true, force: true }); +}); + +describe('initProgress + initManager precedence (#2674)', () => { + it('case 1: ROADMAP [x] + stub phase dir + no SUMMARY → both report complete', async () => { + await writeRoadmap(tmpDir, [{ num: '3', name: 'Stubbed', checked: true }]); + await mkdir(join(tmpDir, '.planning', 'phases', '03-stubbed'), { recursive: true }); + // stub dir, no PLAN/SUMMARY/RESEARCH/CONTEXT files + + const progress = (await initProgress([], tmpDir)).data as Record; + const manager = (await initManager([], tmpDir)).data as Record; + + const pPhase = findPhase(progress.phases as Record[], 3); + const mPhase = findPhase(manager.phases as Record[], 3); + + expect(pPhase?.status).toBe('complete'); + expect(mPhase?.disk_status).toBe('complete'); + }); + + it('case 2: ROADMAP [x] + phase dir + SUMMARY present → both complete (sanity)', async () => { + await writeRoadmap(tmpDir, [{ num: '3', name: 'Done', checked: true }]); + await mkdir(join(tmpDir, '.planning', 'phases', '03-done'), { recursive: true }); + await writeFile(join(tmpDir, '.planning', 'phases', '03-done', '03-01-PLAN.md'), '# plan'); + await writeFile(join(tmpDir, '.planning', 'phases', '03-done', '03-01-SUMMARY.md'), '# done'); + + const progress = (await initProgress([], tmpDir)).data as Record; + const manager = (await initManager([], tmpDir)).data as Record; + + const pPhase = findPhase(progress.phases as Record[], 3); + const mPhase = findPhase(manager.phases as Record[], 3); + + expect(pPhase?.status).toBe('complete'); + expect(mPhase?.disk_status).toBe('complete'); + }); + + it('case 3: ROADMAP [ ] + phase dir + SUMMARY present → disk authoritative (complete)', async () => { + await writeRoadmap(tmpDir, [{ num: '3', name: 'Disk', checked: false }]); + await mkdir(join(tmpDir, '.planning', 'phases', '03-disk'), { recursive: true }); + await writeFile(join(tmpDir, '.planning', 'phases', '03-disk', '03-01-PLAN.md'), '# plan'); + await writeFile(join(tmpDir, '.planning', 'phases', '03-disk', '03-01-SUMMARY.md'), '# done'); + + const progress = (await initProgress([], tmpDir)).data as Record; + const manager = (await initManager([], tmpDir)).data as Record; + + const pPhase = findPhase(progress.phases as Record[], 3); + const mPhase = findPhase(manager.phases as Record[], 3); + + expect(pPhase?.status).toBe('complete'); + expect(mPhase?.disk_status).toBe('complete'); + }); + + it('case 4: ROADMAP [ ] + stub phase dir + no SUMMARY → not complete', async () => { + await writeRoadmap(tmpDir, [{ num: '3', name: 'Empty', checked: false }]); + await mkdir(join(tmpDir, '.planning', 'phases', '03-empty'), { recursive: true }); + + const progress = (await initProgress([], tmpDir)).data as Record; + const manager = (await initManager([], tmpDir)).data as Record; + + const pPhase = findPhase(progress.phases as Record[], 3); + const mPhase = findPhase(manager.phases as Record[], 3); + + // Neither should be 'complete' — preserves pre-existing classification. + expect(pPhase?.status).not.toBe('complete'); + expect(mPhase?.disk_status).not.toBe('complete'); + }); + + it('case 5: ROADMAP [x] + no phase dir → both complete (ROADMAP-only branch preserved)', async () => { + await writeRoadmap(tmpDir, [{ num: '3', name: 'Paper', checked: true }]); + // no directory for phase 3 + + const progress = (await initProgress([], tmpDir)).data as Record; + const manager = (await initManager([], tmpDir)).data as Record; + + const pPhase = findPhase(progress.phases as Record[], 3); + const mPhase = findPhase(manager.phases as Record[], 3); + + expect(pPhase?.status).toBe('complete'); + expect(mPhase?.disk_status).toBe('complete'); + }); + + it('case 6: completed_count agrees across handlers for the stub-dir [x] case', async () => { + await writeRoadmap(tmpDir, [ + { num: '3', name: 'Stub', checked: true }, + { num: '4', name: 'Todo', checked: false }, + ]); + await mkdir(join(tmpDir, '.planning', 'phases', '03-stub'), { recursive: true }); + await mkdir(join(tmpDir, '.planning', 'phases', '04-todo'), { recursive: true }); + + const progress = (await initProgress([], tmpDir)).data as Record; + const manager = (await initManager([], tmpDir)).data as Record; + + expect(progress.completed_count).toBe(1); + expect(manager.completed_count).toBe(1); + }); +});