fix(manager): address review — withProjectRoot, milestone filter, planningPaths
Fixes from trek-e's review on PR #1282: 1. Add missing withProjectRoot() wrapper on output — all other cmdInit* functions include project_root in JSON, manager was the only one without it. 2. Add getMilestonePhaseFilter() to directory scan — prevents stale phase directories from prior milestones appearing as phantom dashboard entries. 3. Replace hardcoded .planning/ paths with planningPaths(cwd) — forward compatibility with workstream scoping (#1268). 4. Add 3 new tests: - Conflict filter blocks dependent phase execution when dep is active - Conflict filter allows independent phase execution in parallel - Output includes project_root field Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -5,7 +5,7 @@
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { execSync } = require('child_process');
|
||||
const { loadConfig, resolveModelInternal, findPhaseInternal, getRoadmapPhaseInternal, pathExistsInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, normalizePhaseName, toPosixPath, output, error } = require('./core.cjs');
|
||||
const { loadConfig, resolveModelInternal, findPhaseInternal, getRoadmapPhaseInternal, pathExistsInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, normalizePhaseName, planningPaths, toPosixPath, output, error } = require('./core.cjs');
|
||||
|
||||
function getLatestCompletedMilestone(cwd) {
|
||||
const milestonesPath = path.join(cwd, '.planning', 'MILESTONES.md');
|
||||
@@ -765,19 +765,20 @@ function cmdInitManager(cwd, raw) {
|
||||
const config = loadConfig(cwd);
|
||||
const milestone = getMilestoneInfo(cwd);
|
||||
|
||||
// Use planningPaths for forward-compatibility with workstream scoping (#1268)
|
||||
const paths = planningPaths(cwd);
|
||||
|
||||
// Validate prerequisites
|
||||
if (!pathExistsInternal(cwd, '.planning/ROADMAP.md')) {
|
||||
if (!fs.existsSync(paths.roadmap)) {
|
||||
error('No ROADMAP.md found. Run /gsd:new-milestone first.');
|
||||
}
|
||||
if (!pathExistsInternal(cwd, '.planning/STATE.md')) {
|
||||
if (!fs.existsSync(paths.state)) {
|
||||
error('No STATE.md found. Run /gsd:new-milestone first.');
|
||||
}
|
||||
|
||||
// Use roadmap analysis for rich phase data (depends_on, disk_status, has_context, etc.)
|
||||
const roadmapPath = path.join(cwd, '.planning', 'ROADMAP.md');
|
||||
const rawContent = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
const rawContent = fs.readFileSync(paths.roadmap, 'utf-8');
|
||||
const content = extractCurrentMilestone(rawContent, cwd);
|
||||
const phasesDir = path.join(cwd, '.planning', 'phases');
|
||||
const phasesDir = paths.phases;
|
||||
const isDirInMilestone = getMilestonePhaseFilter(cwd);
|
||||
|
||||
const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:\s*([^\n]+)/gi;
|
||||
const phases = [];
|
||||
@@ -810,7 +811,7 @@ function cmdInitManager(cwd, raw) {
|
||||
|
||||
try {
|
||||
const entries = fs.readdirSync(phasesDir, { withFileTypes: true });
|
||||
const dirs = entries.filter(e => e.isDirectory()).map(e => e.name);
|
||||
const dirs = entries.filter(e => e.isDirectory()).map(e => e.name).filter(isDirInMilestone);
|
||||
const dirMatch = dirs.find(d => d.startsWith(normalized + '-') || d === normalized);
|
||||
|
||||
if (dirMatch) {
|
||||
@@ -1004,7 +1005,7 @@ function cmdInitManager(cwd, raw) {
|
||||
state_exists: true,
|
||||
};
|
||||
|
||||
output(result, raw);
|
||||
output(withProjectRoot(cwd, result), raw);
|
||||
}
|
||||
|
||||
function cmdInitProgress(cwd, raw) {
|
||||
|
||||
@@ -358,4 +358,59 @@ describe('init manager', () => {
|
||||
assert.strictEqual(output.phases[0].is_active, true);
|
||||
assert.ok(output.phases[0].last_activity !== null);
|
||||
});
|
||||
|
||||
test('conflict filter: blocks dependent phase execute when dep is active', () => {
|
||||
writeState(tmpDir);
|
||||
writeRoadmap(tmpDir, [
|
||||
{ number: '1', name: 'Foundation', complete: true },
|
||||
{ number: '2', name: 'API Layer', depends_on: 'Phase 1' },
|
||||
{ number: '3', name: 'Auth', depends_on: 'Phase 2' },
|
||||
]);
|
||||
|
||||
// Phase 2: partial (actively executing — has 2 plans, 1 summary)
|
||||
scaffoldPhase(tmpDir, 2, { slug: 'api-layer', context: true, plans: 2, summaries: 1 });
|
||||
// Phase 3: planned and deps would be met if Phase 2 were complete, but it's not
|
||||
scaffoldPhase(tmpDir, 3, { slug: 'auth', context: true, plans: 1 });
|
||||
|
||||
const result = runGsdTools('init manager', tmpDir);
|
||||
const output = JSON.parse(result.output);
|
||||
|
||||
// Phase 2 is partial — should NOT appear as execute recommendation (already running)
|
||||
// Phase 3 deps_satisfied is false (Phase 2 not complete) — also no recommendation
|
||||
const execRecs = output.recommended_actions.filter(r => r.action === 'execute');
|
||||
assert.strictEqual(execRecs.length, 0);
|
||||
});
|
||||
|
||||
test('conflict filter: allows independent phase execute in parallel', () => {
|
||||
writeState(tmpDir);
|
||||
writeRoadmap(tmpDir, [
|
||||
{ number: '1', name: 'Foundation', complete: true },
|
||||
{ number: '2', name: 'API Layer', depends_on: 'Phase 1' },
|
||||
{ number: '3', name: 'Notifications' }, // no deps — independent
|
||||
]);
|
||||
|
||||
// Phase 2: partial (actively executing)
|
||||
scaffoldPhase(tmpDir, 2, { slug: 'api-layer', context: true, plans: 2, summaries: 1 });
|
||||
// Phase 3: planned, no deps — independent of Phase 2
|
||||
scaffoldPhase(tmpDir, 3, { slug: 'notifications', context: true, plans: 1 });
|
||||
|
||||
const result = runGsdTools('init manager', tmpDir);
|
||||
const output = JSON.parse(result.output);
|
||||
|
||||
// Phase 3 is independent of Phase 2 — should be recommended for execution
|
||||
const execRecs = output.recommended_actions.filter(r => r.action === 'execute');
|
||||
assert.strictEqual(execRecs.length, 1);
|
||||
assert.strictEqual(execRecs[0].phase, '3');
|
||||
});
|
||||
|
||||
test('output includes project_root field', () => {
|
||||
writeState(tmpDir);
|
||||
writeRoadmap(tmpDir, [{ number: '1', name: 'Test' }]);
|
||||
|
||||
const result = runGsdTools('init manager', tmpDir);
|
||||
const output = JSON.parse(result.output);
|
||||
|
||||
// macOS resolves /var → /private/var; normalize both sides
|
||||
assert.strictEqual(fs.realpathSync(output.project_root), fs.realpathSync(tmpDir));
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user