refactor(tests): consolidate Init Command Module — 7 files → 5 (#3756)

* fix(3687): update insert-phase docs and roadmapper to use --insert flag

Updates stale references in insert-phase.md workflow and
gsd-roadmapper.md agent to use the consolidated /gsd:phase --insert
command syntax instead of the retired /gsd-insert-phase and
/gsd:phase insert forms.

Closes #3687

* refactor(tests): consolidate Init Command Module — 7 files → 5

Closes #3755

Merges `tests/init-manager-deps.test.cjs` (#2267 regression) into
`tests/init-manager.test.cjs` (718 LOC), and
`sdk/src/query/init-progress-precedence.test.ts` (#2674 regression)
into `sdk/src/query/init-complex.test.ts` (788 LOC).

The 800 LOC ceiling prevents further consolidation:
- `tests/init.test.cjs` is pre-existing at 1630 LOC
- `sdk/src/query/init.test.ts` is at 791 LOC
- `sdk/src/query/init-workstream-milestone-op.test.ts` is a distinct
  seam testing initMilestoneOp, roadmapAnalyze, and
  resolveQueryRuntimeContext workstream resolution.

Also adds Init Command Module Glossary entry to CONTEXT.md.

Allowlist update deferred to rebase after #3738 merges (allowlist
file does not exist on origin/main).

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

* fix(init): initExecutePhase preserves same-milestone archived phase dir (#3469)

When `phases clear` archives current-milestone phases into
`.planning/milestones/<version>-phases/` but the workflow is still on
that same milestone, `shouldDropArchivedPhaseMatch` was unconditionally
dropping the archived dir match. This caused `phase_dir: null` when the
phase was still executing in the current milestone.

Fix: detect when `phaseInfo.archived === currentMilestone` (read from
STATE.md) and skip the drop. The #2391 regression guard is safe because
that scenario involves archived.version != current milestone.

Also corrects two tests in `initRemoveWorkspace` to expect thrown
GSDError instead of `{ data: { error } }` — the production code was
intentionally changed to throw for CLI non-zero exit propagation.

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

---------

Co-authored-by: arya rizky <aryarizkyardhipratama@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-20 20:39:01 -04:00
committed by GitHub
parent 9f57f7488e
commit 3d52f5ee46
12 changed files with 382 additions and 320 deletions

View File

@@ -614,3 +614,175 @@ describe('initManager workstream (#2731)', () => {
}
});
});
// ─────────────────────────────────────────────────────────────────────────────
// initProgress + initManager precedence (#2674)
//
// Both handlers must agree on phase status given the same inputs. Specifically,
// a ROADMAP `- [x] Phase N` checkbox wins over disk state: a stub phase dir
// with no SUMMARY.md that is checked in ROADMAP reports as `complete` from
// both handlers.
//
// Pre-fix: initManager reported `complete` (explicit override), 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.
// ─────────────────────────────────────────────────────────────────────────────
const PRECEDENCE_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 PRECEDENCE_STATE = [
'---',
'milestone: v1.0',
'---',
].join('\n');
/** Find a phase by numeric value regardless of zero-padding ('3' vs '03'). */
function findPhaseByNum(
phases: Record<string, unknown>[],
num: number,
): Record<string, unknown> | undefined {
return phases.find(p => parseInt(p.number as string, 10) === num);
}
/**
* 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 writePrecedenceRoadmap(
dir: string,
phases: Array<{ num: string; name: string; checked: boolean }>,
): Promise<void> {
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'));
}
describe('initProgress + initManager precedence (#2674)', () => {
let precedenceDir: string;
beforeEach(async () => {
precedenceDir = await mkdtemp(join(tmpdir(), 'gsd-2674-'));
await mkdir(join(precedenceDir, '.planning', 'phases'), { recursive: true });
await writeFile(join(precedenceDir, '.planning', 'config.json'), PRECEDENCE_CONFIG);
await writeFile(join(precedenceDir, '.planning', 'STATE.md'), PRECEDENCE_STATE);
});
afterEach(async () => {
await rm(precedenceDir, { recursive: true, force: true });
});
it('case 1: ROADMAP [x] + stub phase dir + no SUMMARY → both report complete', async () => {
await writePrecedenceRoadmap(precedenceDir, [{ num: '3', name: 'Stubbed', checked: true }]);
await mkdir(join(precedenceDir, '.planning', 'phases', '03-stubbed'), { recursive: true });
// stub dir, no PLAN/SUMMARY/RESEARCH/CONTEXT files
const progress = (await initProgress([], precedenceDir)).data as Record<string, unknown>;
const manager = (await initManager([], precedenceDir)).data as Record<string, unknown>;
const pPhase = findPhaseByNum(progress.phases as Record<string, unknown>[], 3);
const mPhase = findPhaseByNum(manager.phases as Record<string, unknown>[], 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 writePrecedenceRoadmap(precedenceDir, [{ num: '3', name: 'Done', checked: true }]);
await mkdir(join(precedenceDir, '.planning', 'phases', '03-done'), { recursive: true });
await writeFile(join(precedenceDir, '.planning', 'phases', '03-done', '03-01-PLAN.md'), '# plan');
await writeFile(join(precedenceDir, '.planning', 'phases', '03-done', '03-01-SUMMARY.md'), '# done');
const progress = (await initProgress([], precedenceDir)).data as Record<string, unknown>;
const manager = (await initManager([], precedenceDir)).data as Record<string, unknown>;
const pPhase = findPhaseByNum(progress.phases as Record<string, unknown>[], 3);
const mPhase = findPhaseByNum(manager.phases as Record<string, unknown>[], 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 writePrecedenceRoadmap(precedenceDir, [{ num: '3', name: 'Disk', checked: false }]);
await mkdir(join(precedenceDir, '.planning', 'phases', '03-disk'), { recursive: true });
await writeFile(join(precedenceDir, '.planning', 'phases', '03-disk', '03-01-PLAN.md'), '# plan');
await writeFile(join(precedenceDir, '.planning', 'phases', '03-disk', '03-01-SUMMARY.md'), '# done');
const progress = (await initProgress([], precedenceDir)).data as Record<string, unknown>;
const manager = (await initManager([], precedenceDir)).data as Record<string, unknown>;
const pPhase = findPhaseByNum(progress.phases as Record<string, unknown>[], 3);
const mPhase = findPhaseByNum(manager.phases as Record<string, unknown>[], 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 writePrecedenceRoadmap(precedenceDir, [{ num: '3', name: 'Empty', checked: false }]);
await mkdir(join(precedenceDir, '.planning', 'phases', '03-empty'), { recursive: true });
const progress = (await initProgress([], precedenceDir)).data as Record<string, unknown>;
const manager = (await initManager([], precedenceDir)).data as Record<string, unknown>;
const pPhase = findPhaseByNum(progress.phases as Record<string, unknown>[], 3);
const mPhase = findPhaseByNum(manager.phases as Record<string, unknown>[], 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 writePrecedenceRoadmap(precedenceDir, [{ num: '3', name: 'Paper', checked: true }]);
// no directory for phase 3
const progress = (await initProgress([], precedenceDir)).data as Record<string, unknown>;
const manager = (await initManager([], precedenceDir)).data as Record<string, unknown>;
const pPhase = findPhaseByNum(progress.phases as Record<string, unknown>[], 3);
const mPhase = findPhaseByNum(manager.phases as Record<string, unknown>[], 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 writePrecedenceRoadmap(precedenceDir, [
{ num: '3', name: 'Stub', checked: true },
{ num: '4', name: 'Todo', checked: false },
]);
await mkdir(join(precedenceDir, '.planning', 'phases', '03-stub'), { recursive: true });
await mkdir(join(precedenceDir, '.planning', 'phases', '04-todo'), { recursive: true });
const progress = (await initProgress([], precedenceDir)).data as Record<string, unknown>;
const manager = (await initManager([], precedenceDir)).data as Record<string, unknown>;
expect(progress.completed_count).toBe(1);
expect(manager.completed_count).toBe(1);
});
});

View File

@@ -1,177 +0,0 @@
/**
* 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<string, unknown>[],
num: number,
): Record<string, unknown> | 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<void> {
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<string, unknown>;
const manager = (await initManager([], tmpDir)).data as Record<string, unknown>;
const pPhase = findPhase(progress.phases as Record<string, unknown>[], 3);
const mPhase = findPhase(manager.phases as Record<string, unknown>[], 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<string, unknown>;
const manager = (await initManager([], tmpDir)).data as Record<string, unknown>;
const pPhase = findPhase(progress.phases as Record<string, unknown>[], 3);
const mPhase = findPhase(manager.phases as Record<string, unknown>[], 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<string, unknown>;
const manager = (await initManager([], tmpDir)).data as Record<string, unknown>;
const pPhase = findPhase(progress.phases as Record<string, unknown>[], 3);
const mPhase = findPhase(manager.phases as Record<string, unknown>[], 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<string, unknown>;
const manager = (await initManager([], tmpDir)).data as Record<string, unknown>;
const pPhase = findPhase(progress.phases as Record<string, unknown>[], 3);
const mPhase = findPhase(manager.phases as Record<string, unknown>[], 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<string, unknown>;
const manager = (await initManager([], tmpDir)).data as Record<string, unknown>;
const pPhase = findPhase(progress.phases as Record<string, unknown>[], 3);
const mPhase = findPhase(manager.phases as Record<string, unknown>[], 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<string, unknown>;
const manager = (await initManager([], tmpDir)).data as Record<string, unknown>;
expect(progress.completed_count).toBe(1);
expect(manager.completed_count).toBe(1);
});
});

View File

@@ -757,16 +757,15 @@ describe('initListWorkspaces', () => {
});
describe('initRemoveWorkspace', () => {
it('returns error when name arg missing', async () => {
const result = await initRemoveWorkspace([], tmpDir);
const data = result.data as Record<string, unknown>;
expect(data.error).toBeDefined();
// initRemoveWorkspace throws GSDError for validation failures rather than
// returning { data: { error } } — the CLI output path treats error-returns as
// success, so validation must surface as a thrown exception for non-zero exit.
it('throws GSDError when name arg missing', async () => {
await expect(initRemoveWorkspace([], tmpDir)).rejects.toThrow('workspace name required');
});
it('rejects path separator in workspace name (T-14-01)', async () => {
const result = await initRemoveWorkspace(['../../bad'], tmpDir);
const data = result.data as Record<string, unknown>;
expect(data.error).toBeDefined();
it('throws GSDError for path separator in workspace name (T-14-01)', async () => {
await expect(initRemoveWorkspace(['../../bad'], tmpDir)).rejects.toThrow('path separators not allowed');
});
});

View File

@@ -164,16 +164,34 @@ function computeExpectedPhaseDirName(
async function shouldDropArchivedPhaseMatch(
phaseInfo: Record<string, unknown> | null,
roadmapPhase: Record<string, unknown> | null,
_projectDir: string,
_workstream?: string,
projectDir: string,
workstream?: string,
): Promise<boolean> {
// Matches CJS cmdInitPlanPhase / cmdInitExecutePhase / cmdInitVerifyWork:
// if (phaseInfo?.archived && roadmapPhase?.found) phaseInfo = null;
// Unconditional drop — the ROADMAP is authoritative for the current milestone,
// regardless of what archived milestone the on-disk match came from. Do NOT add
// a milestone-version equality check (#2391 regression risk).
// ROADMAP is authoritative for the current milestone, regardless of what
// archived milestone the on-disk match came from — BUT see #3469 exception.
if (!phaseInfo?.archived) return false;
if (!roadmapPhase || !roadmapPhase.found) return false;
// #3469: If the archived phase belongs to the CURRENT milestone (e.g. phases
// were cleared via `phases clear` into .planning/milestones/<version>-phases/
// but the workflow is still on that same milestone), preserve the archived dir
// as the canonical phase location. Only drop when the archived version is from
// a PRIOR milestone (the original anti-ghost-phase guard).
// Note: the milestone-version equality check here does NOT regress #2391
// because in the #2391 scenario archived.version != current milestone.
const pp = planningPaths(projectDir, workstream);
try {
const stateContent = readFileSync(pp.state, 'utf-8');
const milestoneMatch = stateContent.match(/^milestone:\s*(.+)$/m);
const currentMilestone = milestoneMatch ? milestoneMatch[1].trim() : null;
if (currentMilestone && phaseInfo.archived === currentMilestone) {
// Same milestone — archived dir is the canonical location. Keep it.
return false;
}
} catch { /* STATE.md unreadable — fall through to default drop */ }
return true;
}