* test: phase-dir prefix parity across creation paths (#3287 RED) Add failing tests asserting that init.phase-op and init.plan-phase expose expected_phase_dir with the project_code prefix when the phase directory does not yet exist — matching the prefix applied by phase.add. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(phase): unify phase-dir naming via shared getPhaseDirName helper (#3287) Both init.phase-op (discuss-phase workflow) and init.plan-phase (plan-phase workflow) now compute expected_phase_dir — the canonical directory name including the project_code prefix when set — and expose it in their JSON bundle. Workflow fallback mkdir calls are updated to use ${expected_phase_dir} instead of constructing the path from padded_phase + phase_slug, which was missing the project_code prefix. This eliminates the two-headed naming convention where phase.add/insert produced XR-01-foundation/ while the first-touch paths produced 01-foundation/ for the same project. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(phase): apply project_code prefix in scaffold phase-dir (#3287) Audit finding: phase.scaffold (CJS commands.cjs + SDK phase-lifecycle.ts) also constructed phase dir names without project_code prefix. Both implementations now read config.project_code and apply the same prefix logic as phase.add/phase.insert. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * changeset: pr=3292 for #3287 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(changelog): add entry for #3287 phase-dir prefix parity fix 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
@@ -29,6 +29,7 @@ import { maskIfSecret } from './secrets.js';
|
||||
import { findPhase } from './phase.js';
|
||||
import { roadmapGetPhase, getMilestoneInfo, extractCurrentMilestone, extractPhasesFromSection } from './roadmap.js';
|
||||
import { planningPaths, normalizePhaseName, toPosixPath, resolveAgentsDir, detectRuntime } from './helpers.js';
|
||||
import { generatePhaseSlug, assertSafeProjectCode } from './phase-lifecycle-policy.js';
|
||||
import type { QueryHandler } from './utils.js';
|
||||
|
||||
// ─── Internal helpers ──────────────────────────────────────────────────────
|
||||
@@ -60,6 +61,27 @@ function pathExists(base: string, relPath: string): boolean {
|
||||
return existsSync(join(base, relPath));
|
||||
}
|
||||
|
||||
/**
|
||||
* Compute the canonical phase directory name for a known phase entry from the
|
||||
* roadmap when no directory exists yet. Applies the project_code prefix so
|
||||
* the first-touch creation path used by /gsd-discuss-phase and /gsd-plan-phase
|
||||
* stays consistent with the prefix produced by `phase.add` / `phase.insert`.
|
||||
*
|
||||
* Returns null when phaseNumber or phaseName cannot be determined.
|
||||
*/
|
||||
function computeExpectedPhaseDirName(
|
||||
phaseNumber: string | null,
|
||||
phaseName: string | null,
|
||||
projectCode: string,
|
||||
): string | null {
|
||||
if (!phaseNumber || !phaseName) return null;
|
||||
const paddedNum = normalizePhaseName(phaseNumber);
|
||||
const slug = generatePhaseSlug(phaseName);
|
||||
if (!slug) return null;
|
||||
const prefix = projectCode ? `${projectCode}-` : '';
|
||||
return `${prefix}${paddedNum}-${slug}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the latest completed milestone from MILESTONES.md.
|
||||
* Port of getLatestCompletedMilestone from init.cjs lines 10-25.
|
||||
@@ -377,8 +399,21 @@ export const initPlanPhase: QueryHandler = async (args, projectDir, workstream)
|
||||
: ['', '', ''];
|
||||
|
||||
const phaseNumber = (phaseInfo?.phase_number as string) || null;
|
||||
const phaseName = (phaseInfo?.phase_name as string) ?? null;
|
||||
const phaseDir = (phaseInfo?.directory as string) ?? null;
|
||||
const plans = (phaseInfo?.plans || []) as string[];
|
||||
|
||||
// #3287: compute the canonical directory name with project_code prefix so
|
||||
// the first-touch mkdir in /gsd-plan-phase stays consistent with phase.add.
|
||||
const rawProjectCode = (config as Record<string, unknown>).project_code as string || '';
|
||||
assertSafeProjectCode(rawProjectCode);
|
||||
const expectedPhaseDirName = phaseDir
|
||||
? null // directory already exists — no need to create
|
||||
: computeExpectedPhaseDirName(phaseNumber, phaseName, rawProjectCode);
|
||||
const expectedPhaseDir = expectedPhaseDirName
|
||||
? toPosixPath(relative(projectDir, join(paths.phases, expectedPhaseDirName)))
|
||||
: null;
|
||||
|
||||
const cfg = config as GSDConfig;
|
||||
const result: Record<string, unknown> = {
|
||||
researcher_model: researcherModel,
|
||||
@@ -394,9 +429,10 @@ export const initPlanPhase: QueryHandler = async (args, projectDir, workstream)
|
||||
auto_chain_active: !!config.workflow._auto_chain_active,
|
||||
mode: cfg.mode ?? 'interactive',
|
||||
phase_found: !!phaseInfo,
|
||||
phase_dir: (phaseInfo?.directory as string) ?? null,
|
||||
phase_dir: phaseDir,
|
||||
expected_phase_dir: expectedPhaseDir,
|
||||
phase_number: phaseNumber,
|
||||
phase_name: (phaseInfo?.phase_name as string) ?? null,
|
||||
phase_name: phaseName,
|
||||
phase_slug: (phaseInfo?.phase_slug as string) ?? null,
|
||||
padded_phase: phaseNumber ? normalizePhaseName(phaseNumber) : null,
|
||||
phase_req_ids,
|
||||
@@ -414,22 +450,22 @@ export const initPlanPhase: QueryHandler = async (args, projectDir, workstream)
|
||||
};
|
||||
|
||||
// Add artifact paths if phase directory exists
|
||||
if (phaseInfo?.directory) {
|
||||
const phaseDirFull = join(projectDir, phaseInfo.directory as string);
|
||||
if (phaseDir) {
|
||||
const phaseDirFull = join(projectDir, phaseDir);
|
||||
try {
|
||||
const files = readdirSync(phaseDirFull);
|
||||
const contextFile = files.find(f => f.endsWith('-CONTEXT.md') || f === 'CONTEXT.md');
|
||||
if (contextFile) result.context_path = toPosixPath(join(phaseInfo.directory as string, contextFile));
|
||||
if (contextFile) result.context_path = toPosixPath(join(phaseDir, contextFile));
|
||||
const researchFile = files.find(f => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md');
|
||||
if (researchFile) result.research_path = toPosixPath(join(phaseInfo.directory as string, researchFile));
|
||||
if (researchFile) result.research_path = toPosixPath(join(phaseDir, researchFile));
|
||||
const verificationFile = files.find(f => f.endsWith('-VERIFICATION.md') || f === 'VERIFICATION.md');
|
||||
if (verificationFile) result.verification_path = toPosixPath(join(phaseInfo.directory as string, verificationFile));
|
||||
if (verificationFile) result.verification_path = toPosixPath(join(phaseDir, verificationFile));
|
||||
const uatFile = files.find(f => f.endsWith('-UAT.md') || f === 'UAT.md');
|
||||
if (uatFile) result.uat_path = toPosixPath(join(phaseInfo.directory as string, uatFile));
|
||||
if (uatFile) result.uat_path = toPosixPath(join(phaseDir, uatFile));
|
||||
const reviewsFile = files.find(f => f.endsWith('-REVIEWS.md') || f === 'REVIEWS.md');
|
||||
if (reviewsFile) result.reviews_path = toPosixPath(join(phaseInfo.directory as string, reviewsFile));
|
||||
if (reviewsFile) result.reviews_path = toPosixPath(join(phaseDir, reviewsFile));
|
||||
const patternsFile = files.find(f => f.endsWith('-PATTERNS.md') || f === 'PATTERNS.md');
|
||||
if (patternsFile) result.patterns_path = toPosixPath(join(phaseInfo.directory as string, patternsFile));
|
||||
if (patternsFile) result.patterns_path = toPosixPath(join(phaseDir, patternsFile));
|
||||
} catch { /* intentionally empty */ }
|
||||
}
|
||||
|
||||
@@ -681,8 +717,21 @@ export const initPhaseOp: QueryHandler = async (args, projectDir, workstream) =>
|
||||
|
||||
const phaseFound = !!(phaseInfo && phaseInfo.found);
|
||||
const phaseNumber = (phaseInfo?.phase_number as string) || null;
|
||||
const phaseName = (phaseInfo?.phase_name as string) ?? null;
|
||||
const phaseDir = (phaseInfo?.directory as string) ?? null;
|
||||
const plans = (phaseInfo?.plans || []) as string[];
|
||||
|
||||
// #3287: compute the canonical directory name with project_code prefix so
|
||||
// the first-touch mkdir in /gsd-discuss-phase stays consistent with phase.add.
|
||||
const rawProjectCode = (config as Record<string, unknown>).project_code as string || '';
|
||||
assertSafeProjectCode(rawProjectCode);
|
||||
const expectedPhaseDirName = phaseDir
|
||||
? null // directory already exists — no need to create
|
||||
: computeExpectedPhaseDirName(phaseNumber, phaseName, rawProjectCode);
|
||||
const expectedPhaseDir = expectedPhaseDirName
|
||||
? toPosixPath(relative(projectDir, join(paths.phases, expectedPhaseDirName)))
|
||||
: null;
|
||||
|
||||
const result: Record<string, unknown> = {
|
||||
commit_docs: config.commit_docs,
|
||||
// #2997: secret config keys (brave_search, firecrawl, exa_search) may be
|
||||
@@ -694,9 +743,10 @@ export const initPhaseOp: QueryHandler = async (args, projectDir, workstream) =>
|
||||
firecrawl: typeof config.firecrawl === 'string' ? maskIfSecret('firecrawl', config.firecrawl) : config.firecrawl,
|
||||
exa_search: typeof config.exa_search === 'string' ? maskIfSecret('exa_search', config.exa_search) : config.exa_search,
|
||||
phase_found: phaseFound,
|
||||
phase_dir: (phaseInfo?.directory as string) ?? null,
|
||||
phase_dir: phaseDir,
|
||||
expected_phase_dir: expectedPhaseDir,
|
||||
phase_number: phaseNumber,
|
||||
phase_name: (phaseInfo?.phase_name as string) ?? null,
|
||||
phase_name: phaseName,
|
||||
phase_slug: (phaseInfo?.phase_slug as string) ?? null,
|
||||
padded_phase: phaseNumber ? normalizePhaseName(phaseNumber) : null,
|
||||
has_research: (phaseInfo?.has_research as boolean) || false,
|
||||
@@ -713,20 +763,20 @@ export const initPhaseOp: QueryHandler = async (args, projectDir, workstream) =>
|
||||
};
|
||||
|
||||
// Add artifact paths if phase directory exists
|
||||
if (phaseInfo?.directory) {
|
||||
const phaseDirFull = join(projectDir, phaseInfo.directory as string);
|
||||
if (phaseDir) {
|
||||
const phaseDirFull = join(projectDir, phaseDir);
|
||||
try {
|
||||
const files = readdirSync(phaseDirFull);
|
||||
const contextFile = files.find(f => f.endsWith('-CONTEXT.md') || f === 'CONTEXT.md');
|
||||
if (contextFile) result.context_path = toPosixPath(join(phaseInfo.directory as string, contextFile));
|
||||
if (contextFile) result.context_path = toPosixPath(join(phaseDir, contextFile));
|
||||
const researchFile = files.find(f => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md');
|
||||
if (researchFile) result.research_path = toPosixPath(join(phaseInfo.directory as string, researchFile));
|
||||
if (researchFile) result.research_path = toPosixPath(join(phaseDir, researchFile));
|
||||
const verificationFile = files.find(f => f.endsWith('-VERIFICATION.md') || f === 'VERIFICATION.md');
|
||||
if (verificationFile) result.verification_path = toPosixPath(join(phaseInfo.directory as string, verificationFile));
|
||||
if (verificationFile) result.verification_path = toPosixPath(join(phaseDir, verificationFile));
|
||||
const uatFile = files.find(f => f.endsWith('-UAT.md') || f === 'UAT.md');
|
||||
if (uatFile) result.uat_path = toPosixPath(join(phaseInfo.directory as string, uatFile));
|
||||
if (uatFile) result.uat_path = toPosixPath(join(phaseDir, uatFile));
|
||||
const reviewsFile = files.find(f => f.endsWith('-REVIEWS.md') || f === 'REVIEWS.md');
|
||||
if (reviewsFile) result.reviews_path = toPosixPath(join(phaseInfo.directory as string, reviewsFile));
|
||||
if (reviewsFile) result.reviews_path = toPosixPath(join(phaseDir, reviewsFile));
|
||||
} catch { /* intentionally empty */ }
|
||||
}
|
||||
|
||||
|
||||
@@ -539,7 +539,15 @@ export const phaseScaffold: QueryHandler = async (args, projectDir, workstream)
|
||||
throw new GSDError('phase and name required for phase-dir scaffold', ErrorClassification.Validation);
|
||||
}
|
||||
const slug = generatePhaseSlug(name);
|
||||
const dirNameNew = `${padded}-${slug}`;
|
||||
// #3287: apply project_code prefix to stay consistent with phase.add/phase.insert
|
||||
let scaffoldConfig: Record<string, unknown> = {};
|
||||
try {
|
||||
scaffoldConfig = JSON.parse(await readFile(planningPaths(projectDir, workstream).config, 'utf-8'));
|
||||
} catch { /* use defaults */ }
|
||||
const scaffoldProjectCode = (scaffoldConfig.project_code as string) || '';
|
||||
assertSafeProjectCode(scaffoldProjectCode);
|
||||
const scaffoldPrefix = scaffoldProjectCode ? `${scaffoldProjectCode}-` : '';
|
||||
const dirNameNew = `${scaffoldPrefix}${padded}-${slug}`;
|
||||
assertSafePhaseDirName(dirNameNew, 'scaffold phase directory');
|
||||
const phasesParent = planningPaths(projectDir, workstream).phases;
|
||||
await mkdir(phasesParent, { recursive: true });
|
||||
|
||||
Reference in New Issue
Block a user