diff --git a/sdk/src/query/phase-filesystem-adapter.ts b/sdk/src/query/phase-filesystem-adapter.ts new file mode 100644 index 000000000..882ba65ad --- /dev/null +++ b/sdk/src/query/phase-filesystem-adapter.ts @@ -0,0 +1,35 @@ +import { existsSync } from 'node:fs'; +import { mkdir, readdir, rename, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; + +export async function listDirectories(dirPath: string): Promise { + if (!existsSync(dirPath)) return []; + try { + const entries = await readdir(dirPath, { withFileTypes: true }); + return entries.filter((entry) => entry.isDirectory()).map((entry) => entry.name); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return []; + throw err; + } +} + +export async function ensureDirectoryWithGitkeep(dirPath: string): Promise { + await mkdir(dirPath, { recursive: true }); + await writeFile(join(dirPath, '.gitkeep'), '', 'utf-8'); +} + +export async function archiveDirectories( + sourceDir: string, + archiveDir: string, + shouldArchive: (dirName: string) => boolean, +): Promise { + await mkdir(archiveDir, { recursive: true }); + const sourceDirs = await listDirectories(sourceDir); + let archivedCount = 0; + for (const dirName of sourceDirs) { + if (!shouldArchive(dirName)) continue; + await rename(join(sourceDir, dirName), join(archiveDir, dirName)); + archivedCount++; + } + return archivedCount; +} diff --git a/sdk/src/query/phase-lifecycle-policy.ts b/sdk/src/query/phase-lifecycle-policy.ts new file mode 100644 index 000000000..b891cabce --- /dev/null +++ b/sdk/src/query/phase-lifecycle-policy.ts @@ -0,0 +1,171 @@ +import { GSDError, ErrorClassification } from '../errors.js'; +import { escapeRegex } from './helpers.js'; + +export interface PhaseDirectoryComputation { + phaseId: number | string; + dirName: string; +} + +export interface NextDecimalPhaseResult { + next: string; + existing: string[]; +} + +/** Reject strings containing null bytes (path traversal defense). */ +export function assertNoNullBytes(value: string, label: string): void { + if (value.includes('\0')) { + throw new GSDError(`${label} contains null byte`, ErrorClassification.Validation); + } +} + +/** Reject `..` or path separators in phase directory names. */ +export function assertSafePhaseDirName(dirName: string, label = 'phase directory'): void { + if (/[/\\]|\.\./.test(dirName)) { + throw new GSDError(`${label} contains invalid path segments`, ErrorClassification.Validation); + } +} + +export function assertSafeProjectCode(code: string): void { + if (code && /[/\\]|\.\./.test(code)) { + throw new GSDError('project_code contains invalid characters', ErrorClassification.Validation); + } +} + +/** Generate kebab-case slug from description. */ +export function generatePhaseSlug(text: string): string { + return text + .toLowerCase() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, '') + .substring(0, 60); +} + +export function parseMultiwordArg(args: string[], flag: string): string | null { + const idx = args.indexOf(`--${flag}`); + if (idx === -1) return null; + const tokens: string[] = []; + for (let i = idx + 1; i < args.length; i++) { + if (args[i]!.startsWith('--')) break; + tokens.push(args[i]!); + } + return tokens.length > 0 ? tokens.join(' ') : null; +} + +export function extractOneLinerFromBody(content: string): string | null { + if (!content) return null; + const body = content.replace(/^---\r?\n[\s\S]*?\r?\n---\r?\n*/, ''); + const match = body.match(/^#[^\n]*\n+\*\*([^*]+)\*\*/m); + return match ? match[1]!.trim() : null; +} + +/** + * Scan highest sequential phase number in milestone content. + * Skips backlog lanes (`999.x`). + */ +export function scanSequentialMaxPhaseFromMilestone(milestoneContent: string): number { + const phasePattern = /(?:^|\n)\s*(?:[-*]\s*(?:\[[x ]\]\s*)?|#{2,4}\s*|\*{1,2}\s*)Phase\s+(\d+)[A-Z]?(?:\.\d+)*:/gi; + let maxPhase = 0; + let m: RegExpExecArray | null; + while ((m = phasePattern.exec(milestoneContent)) !== null) { + const num = parseInt(m[1], 10); + if (num >= 999) continue; + if (num > maxPhase) maxPhase = num; + } + return maxPhase; +} + +/** + * Scan highest sequential phase number from phase directory names. + * Supports optional project-code prefix and optional decimal suffixes. + */ +export function scanSequentialMaxPhaseFromDirs(dirNames: string[]): number { + let maxPhase = 0; + const dirNumPattern = /^(?:[A-Z][A-Z0-9]*-)?(\d+)[A-Z]?(?:\.\d+)*-/i; + for (const dirName of dirNames) { + const match = dirNumPattern.exec(dirName); + if (!match) continue; + const num = parseInt(match[1], 10); + if (num >= 999) continue; + if (num > maxPhase) maxPhase = num; + } + return maxPhase; +} + +export function computeNextSequentialPhaseId(milestoneContent: string, dirNames: string[]): number { + return Math.max( + scanSequentialMaxPhaseFromMilestone(milestoneContent), + scanSequentialMaxPhaseFromDirs(dirNames), + ) + 1; +} + +export function computePhaseDirectory( + namingMode: unknown, + descriptionSlug: string, + prefix: string, + nextSequentialPhaseId: number, + customId?: string | null, +): PhaseDirectoryComputation { + if (customId || namingMode === 'custom') { + const phaseId = customId || descriptionSlug.toUpperCase().replace(/-/g, '_'); + if (!phaseId) { + throw new GSDError('--id required when phase_naming is "custom"', ErrorClassification.Validation); + } + assertSafePhaseDirName(String(phaseId), 'custom phase id'); + const dirName = `${prefix}${phaseId}-${descriptionSlug}`; + assertSafePhaseDirName(dirName); + return { phaseId, dirName }; + } + + const phaseId = nextSequentialPhaseId; + const paddedNum = String(phaseId).padStart(2, '0'); + const dirName = `${prefix}${paddedNum}-${descriptionSlug}`; + assertSafePhaseDirName(dirName); + return { phaseId, dirName }; +} + +export function buildPhaseRoadmapEntry( + phaseId: number | string, + description: string, + namingMode: unknown, +): string { + const prevPhase = typeof phaseId === 'number' ? phaseId - 1 : null; + const dependsOn = namingMode === 'custom' || prevPhase === null || prevPhase < 1 + ? '' + : `\n**Depends on:** Phase ${prevPhase}`; + return `\n### Phase ${phaseId}: ${description}\n\n**Goal:** [To be planned]\n**Requirements**: TBD${dependsOn}\n**Plans:** 0 plans\n\nPlans:\n- [ ] TBD (run /gsd-plan-phase ${phaseId} to break down)\n`; +} + +export function collectDecimalSuffixesFromDirNames(basePhase: string, dirNames: string[]): Set { + const decimalSet = new Set(); + const decimalPattern = new RegExp(`^(?:[A-Z][A-Z0-9]*-)?${escapeRegex(basePhase)}\\.(\\d+)`, 'i'); + for (const dir of dirNames) { + const match = dir.match(decimalPattern); + if (match) decimalSet.add(parseInt(match[1], 10)); + } + return decimalSet; +} + +export function collectDecimalSuffixesFromRoadmap(basePhase: string, roadmapContent: string): Set { + const decimalSet = new Set(); + const phasePattern = new RegExp( + `#{2,4}\\s*Phase\\s+0*${escapeRegex(basePhase)}\\.(\\d+)\\s*:`, + 'gi', + ); + let match: RegExpExecArray | null; + while ((match = phasePattern.exec(roadmapContent)) !== null) { + decimalSet.add(parseInt(match[1], 10)); + } + return decimalSet; +} + +export function computeNextDecimalPhase(basePhase: string, decimalSet: Set): NextDecimalPhaseResult { + const existing = Array.from(decimalSet) + .sort((a, b) => a - b) + .map((n) => `${basePhase}.${n}`); + + const next = decimalSet.size === 0 + ? `${basePhase}.1` + : `${basePhase}.${Math.max(...decimalSet) + 1}`; + + return { next, existing }; +} diff --git a/sdk/src/query/phase-lifecycle.test.ts b/sdk/src/query/phase-lifecycle.test.ts index 7e85d0d10..2746e010f 100644 --- a/sdk/src/query/phase-lifecycle.test.ts +++ b/sdk/src/query/phase-lifecycle.test.ts @@ -1600,3 +1600,122 @@ describe('lifecycle handlers in registry', () => { } }); }); + +// ─── CR-3267 regression: error-propagation in listDirectories ───────────── + +describe('listDirectories — CR-3267 finding 1: non-ENOENT errors propagate', () => { + it('propagates EACCES from readdir instead of returning []', async () => { + const { listDirectories } = await import('./phase-filesystem-adapter.js'); + // Create a real directory then remove read permission + const dir = await mkdtemp(join(tmpdir(), 'gsd-fs-acl-')); + const inner = join(dir, 'phases'); + await mkdir(inner); + try { + await import('node:fs/promises').then(m => m.chmod(inner, 0o000)); + await expect(listDirectories(inner)).rejects.toThrow(); + } finally { + // Restore so cleanup can delete + await import('node:fs/promises').then(m => m.chmod(inner, 0o755)); + await rm(dir, { recursive: true, force: true }); + } + }); + + it('returns [] for ENOENT (directory gone between existsSync and readdir)', async () => { + // existsSync passes, but the directory has been removed before readdir — + // the ENOENT branch must still return []. + const { listDirectories } = await import('./phase-filesystem-adapter.js'); + // We can't easily race the real FS, but we can verify the function tolerates + // a path that truly does not exist (existsSync returns false → early []). + const nonExistent = join(tmpdir(), 'gsd-does-not-exist-' + Date.now()); + const result = await listDirectories(nonExistent); + expect(result).toEqual([]); + }); +}); + +// ─── CR-3267 regression: error-propagation in readModifyWriteRoadmapMd ───── + +describe('readModifyWriteRoadmapMd — CR-3267 finding 4: non-ENOENT errors propagate', () => { + it('propagates EACCES on ROADMAP.md readFile instead of treating as empty', async () => { + const { readModifyWriteRoadmapMd } = await import('./phase-lifecycle.js'); + const dir = await mkdtemp(join(tmpdir(), 'gsd-roadmap-acl-')); + const planningDir = join(dir, '.planning'); + await mkdir(planningDir, { recursive: true }); + const roadmapPath = join(planningDir, 'ROADMAP.md'); + await writeFile(roadmapPath, '# Roadmap\n', 'utf-8'); + try { + await import('node:fs/promises').then(m => m.chmod(roadmapPath, 0o000)); + await expect( + readModifyWriteRoadmapMd(dir, (c) => c) + ).rejects.toThrow(); + } finally { + await import('node:fs/promises').then(m => m.chmod(roadmapPath, 0o644)); + await rm(dir, { recursive: true, force: true }); + } + }); + + it('starts with empty content when ROADMAP.md is absent (ENOENT)', async () => { + const { readModifyWriteRoadmapMd } = await import('./phase-lifecycle.js'); + const dir = await mkdtemp(join(tmpdir(), 'gsd-roadmap-noent-')); + const planningDir = join(dir, '.planning'); + await mkdir(planningDir, { recursive: true }); + // No ROADMAP.md written — must default to '' and create it + try { + const result = await readModifyWriteRoadmapMd(dir, (c) => c + 'NEW'); + expect(result).toBe('NEW'); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +}); + +// ─── CR-3267 regression: buildPhaseRoadmapEntry — no "Phase 0" dependency ── + +describe('buildPhaseRoadmapEntry — CR-3267 finding 2: first sequential phase has no predecessor', () => { + it('omits Depends on line when phaseId is 1', async () => { + const { buildPhaseRoadmapEntry } = await import('./phase-lifecycle-policy.js'); + const entry = buildPhaseRoadmapEntry(1, 'Bootstrap', 'sequential'); + expect(entry).not.toContain('Depends on'); + expect(entry).not.toContain('Phase 0'); + }); + + it('includes Depends on line when phaseId is 2', async () => { + const { buildPhaseRoadmapEntry } = await import('./phase-lifecycle-policy.js'); + const entry = buildPhaseRoadmapEntry(2, 'Second Phase', 'sequential'); + expect(entry).toContain('**Depends on:** Phase 1'); + }); + + it('omits Depends on line for custom naming mode regardless of id', async () => { + const { buildPhaseRoadmapEntry } = await import('./phase-lifecycle-policy.js'); + const entry = buildPhaseRoadmapEntry('ALPHA', 'Custom', 'custom'); + expect(entry).not.toContain('Depends on'); + }); +}); + +// ─── CR-3267 regression: collectDecimalSuffixesFromDirNames prefix grammar ─ + +describe('collectDecimalSuffixesFromDirNames — CR-3267 finding 3: alphanumeric prefixes accepted', () => { + it('matches directories with long alphanumeric project-code prefix', async () => { + const { collectDecimalSuffixesFromDirNames } = await import('./phase-lifecycle-policy.js'); + // Prefix "MYAPP01" is longer than 6 chars and contains digits — was rejected before fix + const dirs = ['MYAPP01-3.1-some-work', 'MYAPP01-3.2-other-work', 'unrelated-dir']; + const result = collectDecimalSuffixesFromDirNames('3', dirs); + expect(result.has(1)).toBe(true); + expect(result.has(2)).toBe(true); + }); + + it('still matches directories with short uppercase-only prefix', async () => { + const { collectDecimalSuffixesFromDirNames } = await import('./phase-lifecycle-policy.js'); + const dirs = ['AB-5.1-task', 'AB-5.3-other']; + const result = collectDecimalSuffixesFromDirNames('5', dirs); + expect(result.has(1)).toBe(true); + expect(result.has(3)).toBe(true); + }); + + it('matches directories with no prefix', async () => { + const { collectDecimalSuffixesFromDirNames } = await import('./phase-lifecycle-policy.js'); + const dirs = ['3.1-plain', '3.2-also-plain']; + const result = collectDecimalSuffixesFromDirNames('3', dirs); + expect(result.has(1)).toBe(true); + expect(result.has(2)).toBe(true); + }); +}); diff --git a/sdk/src/query/phase-lifecycle.ts b/sdk/src/query/phase-lifecycle.ts index cae5c81d9..bf16440e6 100644 --- a/sdk/src/query/phase-lifecycle.ts +++ b/sdk/src/query/phase-lifecycle.ts @@ -42,143 +42,31 @@ import { stateReplaceField, } from './state-mutation.js'; import type { QueryHandler } from './utils.js'; +import { + assertNoNullBytes, + assertSafePhaseDirName, + assertSafeProjectCode, + buildPhaseRoadmapEntry, + collectDecimalSuffixesFromDirNames, + collectDecimalSuffixesFromRoadmap, + computeNextDecimalPhase, + computeNextSequentialPhaseId, + computePhaseDirectory, + extractOneLinerFromBody, + generatePhaseSlug, + parseMultiwordArg, +} from './phase-lifecycle-policy.js'; +import { + archiveDirectories, + ensureDirectoryWithGitkeep, + listDirectories, +} from './phase-filesystem-adapter.js'; +import { + readModifyWriteRoadmapMd, + replaceInCurrentMilestone, +} from './phase-roadmap-mutation.js'; -// ─── Null byte validation ──────────────────────────────────────────────── - -/** Reject strings containing null bytes (path traversal defense). */ -function assertNoNullBytes(value: string, label: string): void { - if (value.includes('\0')) { - throw new GSDError(`${label} contains null byte`, ErrorClassification.Validation); - } -} - -/** Reject `..` or path separators in phase directory names. */ -function assertSafePhaseDirName(dirName: string, label = 'phase directory'): void { - if (/[/\\]|\.\./.test(dirName)) { - throw new GSDError(`${label} contains invalid path segments`, ErrorClassification.Validation); - } -} - -function assertSafeProjectCode(code: string): void { - if (code && /[/\\]|\.\./.test(code)) { - throw new GSDError('project_code contains invalid characters', ErrorClassification.Validation); - } -} - -// ─── Slug generation (inline) ──────────────────────────────────────────── - -/** Generate kebab-case slug from description. Port of generateSlugInternal. */ -function generateSlugInternal(text: string): string { - return text - .toLowerCase() - .replace(/[^a-z0-9]+/g, '-') - .replace(/^-+|-+$/g, '') - .substring(0, 60); -} - -// ─── replaceInCurrentMilestone ────────────────────────────────────────── - -/** - * Replace a pattern only in the current milestone section of ROADMAP.md. - * - * Port of replaceInCurrentMilestone from core.cjs line 1197-1206. - * If no `` blocks exist, replaces in the entire content. - * Otherwise, only replaces in content after the last `` close tag. - * - * Edge case: when the active milestone is itself wrapped in a `
` block - * (e.g. collapsed before it is fully shipped), the last `
` belongs to - * the active milestone and the `after` slice is empty. In that case the function - * falls back to searching the full content with all complete `
` blocks - * stripped, so archived milestones are never touched. - * - * @param content - Full ROADMAP.md content - * @param pattern - Regex or string pattern to match - * @param replacement - Replacement string - * @returns Modified content - */ -export function replaceInCurrentMilestone( - content: string, - pattern: string | RegExp, - replacement: string, -): string { - const lastDetailsClose = content.lastIndexOf('
'); - if (lastDetailsClose === -1) { - return content.replace(pattern, replacement); - } - const offset = lastDetailsClose + ''.length; - const before = content.slice(0, offset); - const after = content.slice(offset); - - // Fast path: the current milestone is not inside a
block — the - // pattern lives in the plain text after the last
. - const replacedAfter = after.replace(pattern, replacement); - if (replacedAfter !== after) { - return before + replacedAfter; - } - - // Slow path: the active milestone is inside the last
block. - // Strip every complete
…
block except the last one, then - // apply the replacement inside that last block while leaving the stripped - // (archived) blocks untouched. - // - // Strategy: - // 1. Collect all complete
…
spans. - // 2. Replace only inside the LAST span; leave earlier spans unchanged. - const detailsBlockRe = /
[\s\S]*?<\/details>/gi; - const spans: { start: number; end: number; text: string }[] = []; - let m: RegExpExecArray | null; - while ((m = detailsBlockRe.exec(content)) !== null) { - spans.push({ start: m.index, end: m.index + m[0].length, text: m[0] }); - } - - if (spans.length === 0) { - // No complete blocks found — fall back to full-content replace. - return content.replace(pattern, replacement); - } - - const lastSpan = spans[spans.length - 1]; - const updatedLastBlock = lastSpan.text.replace(pattern, replacement); - return ( - content.slice(0, lastSpan.start) + - updatedLastBlock + - content.slice(lastSpan.end) - ); -} - -// ─── readModifyWriteRoadmapMd ─────────────────────────────────────────── - -/** - * Atomic read-modify-write for ROADMAP.md. - * - * Holds a lockfile across the entire read -> transform -> write cycle. - * Uses the same acquireStateLock/releaseStateLock mechanism as STATE.md - * but with a ROADMAP.md-specific lock path. - * - * @param projectDir - Project root directory - * @param modifier - Function to transform ROADMAP.md content - * @returns The final written content - */ -export async function readModifyWriteRoadmapMd( - projectDir: string, - modifier: (content: string) => string | Promise, - workstream?: string, -): Promise { - const roadmapPath = planningPaths(projectDir, workstream).roadmap; - const lockPath = await acquireStateLock(roadmapPath); - try { - let content: string; - try { - content = await readFile(roadmapPath, 'utf-8'); - } catch { - content = ''; - } - const modified = await modifier(content); - await writeFile(roadmapPath, modified, 'utf-8'); - return modified; - } finally { - await releaseStateLock(lockPath); - } -} +export { readModifyWriteRoadmapMd, replaceInCurrentMilestone }; // ─── phaseAdd handler ─────────────────────────────────────────────────── @@ -230,7 +118,7 @@ export const phaseAdd: QueryHandler = async (args, projectDir, workstream) => { config = JSON.parse(await readFile(configPath, 'utf-8')); } catch { /* use defaults */ } - const slug = generateSlugInternal(description); + const slug = generatePhaseSlug(description); // positional[1] is the optional customId — flags are already stripped const customId = positional[1] || null; @@ -245,55 +133,17 @@ export const phaseAdd: QueryHandler = async (args, projectDir, workstream) => { // there is no race condition to guard against). const computePhaseFields = async (rawRoadmapContent: string) => { const milestoneContent = await extractCurrentMilestone(rawRoadmapContent, projectDir); + const phasesDir = planningPaths(projectDir, workstream).phases; + const dirNames = await listDirectories(phasesDir); - let resolvedPhaseId: number | string = ''; - let resolvedDirName = ''; - - if (customId || config.phase_naming === 'custom') { - // Custom phase naming - resolvedPhaseId = customId || slug.toUpperCase().replace(/-/g, '_'); - if (!resolvedPhaseId) { - throw new GSDError('--id required when phase_naming is "custom"', ErrorClassification.Validation); - } - assertSafePhaseDirName(String(resolvedPhaseId), 'custom phase id'); - resolvedDirName = `${prefix}${resolvedPhaseId}-${slug}`; - } else { - // Sequential mode: find highest integer phase number (in current milestone only) - // Skip 999.x backlog phases — they live outside the active sequence - // Matches heading (## Phase N:), bullet checklist (- [x] Phase N:), and bold (**Phase N:**) - const phasePattern = /(?:^|\n)\s*(?:[-*]\s*(?:\[[x ]\]\s*)?|#{2,4}\s*|\*{1,2}\s*)Phase\s+(\d+)[A-Z]?(?:\.\d+)*:/gi; - let maxPhase = 0; - let m: RegExpExecArray | null; - while ((m = phasePattern.exec(milestoneContent)) !== null) { - const num = parseInt(m[1], 10); - if (num >= 999) continue; // backlog phases use 999.x numbering - if (num > maxPhase) maxPhase = num; - } - - // Also scan on-disk phase directories (union semantics) — ROADMAP and disk - // may diverge temporarily (e.g. a previous run created the dir but didn't - // update ROADMAP). Taking the max of both sources prevents collisions. - const phasesDir = planningPaths(projectDir, workstream).phases; - try { - const entries = await readdir(phasesDir, { withFileTypes: true }); - for (const entry of entries) { - if (!entry.isDirectory()) continue; - const dirMatch = /^(?:[A-Z][A-Z0-9]*-)?(\d+)[A-Z]?(?:\.\d+)*-/i.exec(entry.name); - if (!dirMatch) continue; - const num = parseInt(dirMatch[1], 10); - if (num >= 999) continue; - if (num > maxPhase) maxPhase = num; - } - } catch { - // phases dir may not exist yet — leave maxPhase as 0 - } - - resolvedPhaseId = maxPhase + 1; - const paddedNum = String(resolvedPhaseId).padStart(2, '0'); - resolvedDirName = `${prefix}${paddedNum}-${slug}`; - } - - assertSafePhaseDirName(resolvedDirName); + const nextSequentialPhaseId = computeNextSequentialPhaseId(milestoneContent, dirNames); + const { phaseId: resolvedPhaseId, dirName: resolvedDirName } = computePhaseDirectory( + config.phase_naming, + slug, + prefix, + nextSequentialPhaseId, + customId || undefined, + ); if (!resolvedDirName) { throw new GSDError('Phase directory name was not computed', ErrorClassification.Execution); @@ -302,10 +152,7 @@ export const phaseAdd: QueryHandler = async (args, projectDir, workstream) => { throw new GSDError('Phase ID was not computed', ErrorClassification.Execution); } - const dependsOn = config.phase_naming === 'custom' - ? '' - : `\n**Depends on:** Phase ${typeof resolvedPhaseId === 'number' ? resolvedPhaseId - 1 : 'TBD'}`; - const resolvedEntry = `\n### Phase ${resolvedPhaseId}: ${description}\n\n**Goal:** [To be planned]\n**Requirements**: TBD${dependsOn}\n**Plans:** 0 plans\n\nPlans:\n- [ ] TBD (run /gsd-plan-phase ${resolvedPhaseId} to break down)\n`; + const resolvedEntry = buildPhaseRoadmapEntry(resolvedPhaseId, description, config.phase_naming); return { resolvedPhaseId, resolvedDirName, resolvedEntry }; }; @@ -339,8 +186,7 @@ export const phaseAdd: QueryHandler = async (args, projectDir, workstream) => { const dirPath = join(planningPaths(projectDir, workstream).phases, dirName); // Create directory with .gitkeep so git tracks empty folders - await mkdir(dirPath, { recursive: true }); - await writeFile(join(dirPath, '.gitkeep'), '', 'utf-8'); + await ensureDirectoryWithGitkeep(dirPath); // Find insertion point: before last "---" or at end const lastSeparator = roadmapRaw.lastIndexOf('\n---'); @@ -436,31 +282,13 @@ export const phaseAddBatch: QueryHandler = async (args, projectDir, workstream) let maxPhase = 0; if (config.phase_naming !== 'custom') { - const phasePattern = /#{2,4}\s*Phase\s+(\d+)[A-Z]?(?:\.\d+)*:/gi; - let m: RegExpExecArray | null; - while ((m = phasePattern.exec(content)) !== null) { - const num = parseInt(m[1], 10); - if (num >= 999) continue; - if (num > maxPhase) maxPhase = num; - } - const phasesOnDisk = planningPaths(projectDir, workstream).phases; - if (existsSync(phasesOnDisk)) { - const entries = await readdir(phasesOnDisk, { withFileTypes: true }); - const dirNumPattern = /^(?:[A-Z][A-Z0-9]*-)?(\d+)-/; - for (const entry of entries) { - if (!entry.isDirectory()) continue; - const match = entry.name.match(dirNumPattern); - if (!match) continue; - const num = parseInt(match[1], 10); - if (num >= 999) continue; - if (num > maxPhase) maxPhase = num; - } - } + const dirNames = await listDirectories(phasesOnDisk); + maxPhase = computeNextSequentialPhaseId(content, dirNames) - 1; } for (const description of descriptions) { - const slug = generateSlugInternal(description); + const slug = generatePhaseSlug(description); let newPhaseId: number | string; let dirName: string; @@ -476,14 +304,10 @@ export const phaseAddBatch: QueryHandler = async (args, projectDir, workstream) assertSafePhaseDirName(dirName); const dirPath = join(planningPaths(projectDir, workstream).phases, dirName); - await mkdir(dirPath, { recursive: true }); - await writeFile(join(dirPath, '.gitkeep'), '', 'utf-8'); + await ensureDirectoryWithGitkeep(dirPath); - const dependsOn = - config.phase_naming === 'custom' - ? '' - : `\n**Depends on:** Phase ${typeof newPhaseId === 'number' ? newPhaseId - 1 : 'TBD'}`; - const phaseEntry = `\n### Phase ${newPhaseId}: ${description}\n\n**Goal:** [To be planned]\n**Requirements**: TBD${dependsOn}\n**Plans:** 0 plans\n\nPlans:\n- [ ] TBD (run /gsd-plan-phase ${newPhaseId} to break down)\n`; + const phaseEntry = + buildPhaseRoadmapEntry(newPhaseId, description, config.phase_naming); const lastSeparator = rawContent.lastIndexOf('\n---'); rawContent = @@ -530,7 +354,7 @@ export const phaseInsert: QueryHandler = async (args, projectDir, workstream) => assertNoNullBytes(afterPhase, 'afterPhase'); assertNoNullBytes(description, 'description'); - const slug = generateSlugInternal(description); + const slug = generatePhaseSlug(description); let decimalPhase = ''; let dirName = ''; @@ -552,26 +376,17 @@ export const phaseInsert: QueryHandler = async (args, projectDir, workstream) => const decimalSet = new Set(); try { - const entries = await readdir(phasesDir, { withFileTypes: true }); - const dirs = entries.filter(e => e.isDirectory()).map(e => e.name); - const decimalPattern = new RegExp(`^(?:[A-Z]{1,6}-)?${escapeRegex(normalizedBase)}\\.(\\d+)`); - for (const dir of dirs) { - const dm = dir.match(decimalPattern); - if (dm) decimalSet.add(parseInt(dm[1], 10)); + const dirs = await listDirectories(phasesDir); + for (const suffix of collectDecimalSuffixesFromDirNames(normalizedBase, dirs)) { + decimalSet.add(suffix); } } catch { /* intentionally empty */ } // Also scan ROADMAP.md content for decimal entries - const rmPhasePattern = new RegExp( - `#{2,4}\\s*Phase\\s+0*${escapeRegex(normalizedBase)}\\.(\\d+)\\s*:`, 'gi' - ); - let rmMatch: RegExpExecArray | null; - while ((rmMatch = rmPhasePattern.exec(rawContent)) !== null) { - decimalSet.add(parseInt(rmMatch[1], 10)); + for (const suffix of collectDecimalSuffixesFromRoadmap(normalizedBase, rawContent)) { + decimalSet.add(suffix); } - - const nextDecimal = decimalSet.size === 0 ? 1 : Math.max(...decimalSet) + 1; - decimalPhase = `${normalizedBase}.${nextDecimal}`; + decimalPhase = computeNextDecimalPhase(normalizedBase, decimalSet).next; // Optional project code prefix let insertConfig: Record = {}; @@ -586,8 +401,7 @@ export const phaseInsert: QueryHandler = async (args, projectDir, workstream) => const dirPath = join(phasesDir, dirName); // Create directory with .gitkeep - await mkdir(dirPath, { recursive: true }); - await writeFile(join(dirPath, '.gitkeep'), '', 'utf-8'); + await ensureDirectoryWithGitkeep(dirPath); // Build phase entry const phaseEntry = `\n### Phase ${decimalPhase}: ${description} (INSERTED)\n\n**Goal:** [Urgent work - to be planned]\n**Requirements**: TBD\n**Depends on:** Phase ${afterPhase}\n**Plans:** 0 plans\n\nPlans:\n- [ ] TBD (run /gsd-plan-phase ${decimalPhase} to break down)\n`; @@ -645,25 +459,19 @@ async function findPhaseDir( ): Promise<{ dirPath: string; dirName: string; phaseName: string | null } | null> { const phasesDir = planningPaths(projectDir, workstream).phases; const normalized = normalizePhaseName(phase); + const dirs = await listDirectories(phasesDir); + const match = dirs.find((d) => phaseTokenMatches(d, normalized)); + if (!match) return null; - try { - const entries = await readdir(phasesDir, { withFileTypes: true }); - const dirs = entries.filter(e => e.isDirectory()).map(e => e.name); - const match = dirs.find(d => phaseTokenMatches(d, normalized)); - if (!match) return null; + // Extract phase name from directory + const dirMatch = match.match(/^(?:[A-Z]{1,6}-)?\d+[A-Z]?(?:\.\d+)*-(.+)/i); + const phaseName = dirMatch ? dirMatch[1] : null; - // Extract phase name from directory - const dirMatch = match.match(/^(?:[A-Z]{1,6}-)?\d+[A-Z]?(?:\.\d+)*-(.+)/i); - const phaseName = dirMatch ? dirMatch[1] : null; - - return { - dirPath: join(phasesDir, match), - dirName: match, - phaseName, - }; - } catch { - return null; - } + return { + dirPath: join(phasesDir, match), + dirName: match, + phaseName, + }; } /** @@ -730,14 +538,13 @@ export const phaseScaffold: QueryHandler = async (args, projectDir, workstream) if (!phase || !name) { throw new GSDError('phase and name required for phase-dir scaffold', ErrorClassification.Validation); } - const slug = generateSlugInternal(name); + const slug = generatePhaseSlug(name); const dirNameNew = `${padded}-${slug}`; assertSafePhaseDirName(dirNameNew, 'scaffold phase directory'); const phasesParent = planningPaths(projectDir, workstream).phases; await mkdir(phasesParent, { recursive: true }); const dirPath = join(phasesParent, dirNameNew); - await mkdir(dirPath, { recursive: true }); - await writeFile(join(dirPath, '.gitkeep'), '', 'utf-8'); + await ensureDirectoryWithGitkeep(dirPath); return { data: { created: true, @@ -1672,39 +1479,23 @@ export const phaseNextDecimal: QueryHandler = async (args, projectDir, workstrea const decimalSet = new Set(); let baseExists = false; - if (existsSync(phasesDir)) { - const entries = await readdir(phasesDir, { withFileTypes: true }); - const dirNames = entries.filter(e => e.isDirectory()).map(e => e.name); - baseExists = dirNames.some(d => phaseTokenMatches(d, normalized)); - - const dirPattern = new RegExp(`^(?:[A-Z]{1,6}-)?${escapeRegex(normalized)}\\.(\\d+)`); - for (const dir of dirNames) { - const match = dir.match(dirPattern); - if (match) decimalSet.add(parseInt(match[1], 10)); - } + const dirNames = await listDirectories(phasesDir); + baseExists = dirNames.some((d) => phaseTokenMatches(d, normalized)); + for (const suffix of collectDecimalSuffixesFromDirNames(normalized, dirNames)) { + decimalSet.add(suffix); } const roadmapPath = paths.roadmap; if (existsSync(roadmapPath)) { try { const roadmapContent = await readFile(roadmapPath, 'utf-8'); - const phasePattern = new RegExp( - `#{2,4}\\s*Phase\\s+0*${escapeRegex(normalized)}\\.(\\d+)\\s*:`, 'gi', - ); - let pm; - while ((pm = phasePattern.exec(roadmapContent)) !== null) { - decimalSet.add(parseInt(pm[1], 10)); + for (const suffix of collectDecimalSuffixesFromRoadmap(normalized, roadmapContent)) { + decimalSet.add(suffix); } } catch { /* ROADMAP.md read failure is non-fatal */ } } - const existingDecimals = Array.from(decimalSet) - .sort((a, b) => a - b) - .map(n => `${normalized}.${n}`); - - const nextDecimal = decimalSet.size === 0 - ? `${normalized}.1` - : `${normalized}.${Math.max(...decimalSet) + 1}`; + const { next: nextDecimal, existing: existingDecimals } = computeNextDecimalPhase(normalized, decimalSet); return { data: { @@ -1728,19 +1519,7 @@ export const phasesArchive: QueryHandler = async (args, projectDir, workstream) const isDirInMilestone = await getMilestonePhaseFilter(projectDir, workstream); const archiveDir = join(paths.planning, 'milestones', `${version}-phases`); - await mkdir(archiveDir, { recursive: true }); - - let archivedCount = 0; - if (existsSync(phasesDir)) { - const entries = await readdir(phasesDir, { withFileTypes: true }); - const phaseDirNames = entries.filter(e => e.isDirectory()).map(e => e.name); - - for (const dir of phaseDirNames) { - if (!isDirInMilestone(dir)) continue; - await rename(join(phasesDir, dir), join(archiveDir, dir)); - archivedCount++; - } - } + const archivedCount = await archiveDirectories(phasesDir, archiveDir, (dirName) => isDirInMilestone(dirName)); return { data: { @@ -1753,26 +1532,6 @@ export const phasesArchive: QueryHandler = async (args, projectDir, workstream) // ─── milestoneComplete ──────────────────────────────────────────────────── -/** Port of `parseMultiwordArg` in `gsd-tools.cjs`. */ -function parseMultiwordArg(args: string[], flag: string): string | null { - const idx = args.indexOf(`--${flag}`); - if (idx === -1) return null; - const tokens: string[] = []; - for (let i = idx + 1; i < args.length; i++) { - if (args[i]!.startsWith('--')) break; - tokens.push(args[i]!); - } - return tokens.length > 0 ? tokens.join(' ') : null; -} - -/** Port of `extractOneLinerFromBody` from `core.cjs` / `summary.ts`. */ -function extractOneLinerFromBody(content: string): string | null { - if (!content) return null; - const body = content.replace(/^---\r?\n[\s\S]*?\r?\n---\r?\n*/, ''); - const match = body.match(/^#[^\n]*\n+\*\*([^*]+)\*\*/m); - return match ? match[1]!.trim() : null; -} - /** * Query handler for `milestone.complete` — port of `cmdMilestoneComplete` from `milestone.cjs`. */ @@ -1814,8 +1573,7 @@ export const milestoneComplete: QueryHandler = async (args, projectDir, workstre const accomplishments: string[] = []; try { - const entries = await readdir(phasesDir, { withFileTypes: true }); - const dirs = entries.filter((e) => e.isDirectory()).map((e) => e.name).sort(); + const dirs = (await listDirectories(phasesDir)).sort(); for (const dir of dirs) { if (!isDirInMilestone(dir)) continue; @@ -1941,16 +1699,11 @@ export const milestoneComplete: QueryHandler = async (args, projectDir, workstre if (archivePhases) { try { const phaseArchiveDir = join(archiveDir, `${version}-phases`); - await mkdir(phaseArchiveDir, { recursive: true }); - - const phaseEntries = await readdir(phasesDir, { withFileTypes: true }); - const phaseDirNames = phaseEntries.filter((e) => e.isDirectory()).map((e) => e.name); - let archivedCount = 0; - for (const dir of phaseDirNames) { - if (!isDirInMilestone(dir)) continue; - await rename(join(phasesDir, dir), join(phaseArchiveDir, dir)); - archivedCount++; - } + const archivedCount = await archiveDirectories( + phasesDir, + phaseArchiveDir, + (dirName) => isDirInMilestone(dirName), + ); phasesArchived = archivedCount > 0; } catch { /* intentionally empty */ diff --git a/sdk/src/query/phase-roadmap-mutation.ts b/sdk/src/query/phase-roadmap-mutation.ts new file mode 100644 index 000000000..6b62f2405 --- /dev/null +++ b/sdk/src/query/phase-roadmap-mutation.ts @@ -0,0 +1,77 @@ +import { readFile, writeFile } from 'node:fs/promises'; +import { planningPaths } from './helpers.js'; +import { acquireStateLock, releaseStateLock } from './state-mutation.js'; + +/** + * Replace a pattern only in the current milestone section of ROADMAP.md. + * + * Port of replaceInCurrentMilestone from core.cjs line 1197-1206. + */ +export function replaceInCurrentMilestone( + content: string, + pattern: string | RegExp, + replacement: string, +): string { + const lastDetailsClose = content.lastIndexOf('
'); + if (lastDetailsClose === -1) { + return content.replace(pattern, replacement); + } + const offset = lastDetailsClose + '
'.length; + const before = content.slice(0, offset); + const after = content.slice(offset); + + const replacedAfter = after.replace(pattern, replacement); + if (replacedAfter !== after) { + return before + replacedAfter; + } + + const detailsBlockRe = /
[\s\S]*?<\/details>/gi; + const spans: { start: number; end: number; text: string }[] = []; + let m: RegExpExecArray | null; + while ((m = detailsBlockRe.exec(content)) !== null) { + spans.push({ start: m.index, end: m.index + m[0].length, text: m[0] }); + } + + if (spans.length === 0) { + return content.replace(pattern, replacement); + } + + const lastSpan = spans[spans.length - 1]; + const updatedLastBlock = lastSpan.text.replace(pattern, replacement); + return ( + content.slice(0, lastSpan.start) + + updatedLastBlock + + content.slice(lastSpan.end) + ); +} + +/** + * Atomic read-modify-write for ROADMAP.md. + * + * Holds a lockfile across the entire read -> transform -> write cycle. + */ +export async function readModifyWriteRoadmapMd( + projectDir: string, + modifier: (content: string) => string | Promise, + workstream?: string, +): Promise { + const roadmapPath = planningPaths(projectDir, workstream).roadmap; + const lockPath = await acquireStateLock(roadmapPath); + try { + let content: string; + try { + content = await readFile(roadmapPath, 'utf-8'); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') { + content = ''; + } else { + throw err; + } + } + const modified = await modifier(content); + await writeFile(roadmapPath, modified, 'utf-8'); + return modified; + } finally { + await releaseStateLock(lockPath); + } +} diff --git a/sdk/src/query/roadmap-update-plan-progress.ts b/sdk/src/query/roadmap-update-plan-progress.ts index 586d6b5ca..75068447f 100644 --- a/sdk/src/query/roadmap-update-plan-progress.ts +++ b/sdk/src/query/roadmap-update-plan-progress.ts @@ -8,7 +8,7 @@ */ import { findPhase } from './phase.js'; -import { readModifyWriteRoadmapMd, replaceInCurrentMilestone } from './phase-lifecycle.js'; +import { readModifyWriteRoadmapMd, replaceInCurrentMilestone } from './phase-roadmap-mutation.js'; import { existsSync } from 'node:fs'; import { escapeRegex, planningPaths } from './helpers.js'; import { GSDError, ErrorClassification } from '../errors.js';