fix(3266): preserve wave 0 and bucket plans by depends_on DAG in phase-plan-index (#3276)
* fix(3266): preserve wave 0 and bucket plans by depends_on DAG in phase-plan-index Fixes two cooperating bugs in the phase-plan-index builder: 1. Wave 0 collapse: `parseInt(...) || 1` coerced parsed value `0` to `1` due to JS falsy default. Fixed with `Number.isNaN` guard. 2. depends_on ignored: wave-bucketing used only the `wave:` frontmatter field. Now replaced with Kahn's topological-level algorithm over `depends_on`: source nodes (no in-phase deps) → lowest level; each plan's level = max(deps' levels) + 1. Declared `wave:` that disagrees with computed level emits a non-fatal warning on the result. Cycle detection throws GSDError. `PlanInfo` gains `depends_on: string[]`. `PhasePlanIndex` gains `warnings?: string[]`. Both TS (`sdk/src/query/phase.ts`) and CJS twin (`get-shit-done/bin/lib/phase.cjs`) fixed identically. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: add changeset for #3276 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(phase): resolve depends_on against canonical plan id (#3276 CR) Build a secondary `canonicalToId` index alongside `planMap` so that a dependency declared as '03-01' resolves to a descriptive plan stored under '03-01-auth-hardening', preventing silent wave-ordering failures. Applied at both DAG construction sites in phase.cjs and the SDK's phase.ts (k014 parity). Regression test added. 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:
5
.changeset/noble-otters-hop.md
Normal file
5
.changeset/noble-otters-hop.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3276
|
||||
---
|
||||
phase-plan-index no longer collapses wave 0 to wave 1, and now buckets plans using their depends_on DAG so dependents run after their dependencies rather than in the same parallel wave
|
||||
@@ -322,10 +322,9 @@ function cmdPhasePlanIndex(cwd, phase, raw) {
|
||||
})
|
||||
);
|
||||
|
||||
const plans = [];
|
||||
const waves = {};
|
||||
const incomplete = [];
|
||||
let hasCheckpoints = false;
|
||||
// ── Pass 1: parse each plan file ─────────────────────────────────────────
|
||||
|
||||
const rawPlans = [];
|
||||
|
||||
for (const planFile of planFiles) {
|
||||
const planId = planFile.replace('-PLAN.md', '').replace('PLAN.md', '');
|
||||
@@ -338,8 +337,20 @@ function cmdPhasePlanIndex(cwd, phase, raw) {
|
||||
const mdTasks = content.match(/##\s*Task\s*\d+/gi) || [];
|
||||
const taskCount = xmlTasks.length || mdTasks.length;
|
||||
|
||||
// Parse wave as integer
|
||||
const wave = parseInt(fm.wave, 10) || 1;
|
||||
// Parse wave as integer — use nullish handling so wave: 0 is preserved.
|
||||
// parseInt returns NaN for missing/non-numeric values; fall back to null
|
||||
// (meaning "no declared wave") so downstream can apply the topo default.
|
||||
const parsedWave = parseInt(fm.wave, 10);
|
||||
const declaredWave = Number.isNaN(parsedWave) ? null : parsedWave;
|
||||
|
||||
// Parse depends_on — normalise to string[]
|
||||
let dependsOn = [];
|
||||
const fmDeps = fm['depends_on'];
|
||||
if (Array.isArray(fmDeps)) {
|
||||
dependsOn = fmDeps.map(String);
|
||||
} else if (typeof fmDeps === 'string' && fmDeps.trim() !== '') {
|
||||
dependsOn = [fmDeps];
|
||||
}
|
||||
|
||||
// Parse autonomous (default true if not specified)
|
||||
let autonomous = true;
|
||||
@@ -347,10 +358,6 @@ function cmdPhasePlanIndex(cwd, phase, raw) {
|
||||
autonomous = fm.autonomous === 'true' || fm.autonomous === true;
|
||||
}
|
||||
|
||||
if (!autonomous) {
|
||||
hasCheckpoints = true;
|
||||
}
|
||||
|
||||
// Parse files_modified (underscore is canonical; also accept hyphenated for compat)
|
||||
let filesModified = [];
|
||||
const fmFiles = fm['files_modified'] || fm['files-modified'];
|
||||
@@ -359,28 +366,129 @@ function cmdPhasePlanIndex(cwd, phase, raw) {
|
||||
}
|
||||
|
||||
const hasSummary = completedPlanIds.has(planId) || completedPlanIds.has(extractCanonicalPlanId(planFile));
|
||||
if (!hasSummary) {
|
||||
incomplete.push(planId);
|
||||
|
||||
rawPlans.push({
|
||||
id: planId,
|
||||
declaredWave,
|
||||
dependsOn,
|
||||
autonomous,
|
||||
objective: extractObjective(content) || fm.objective || null,
|
||||
filesModified,
|
||||
taskCount,
|
||||
hasSummary,
|
||||
});
|
||||
}
|
||||
|
||||
// ── Pass 2: topological level assignment via depends_on DAG ──────────────
|
||||
|
||||
// Build a map from plan ID → raw plan for fast lookup.
|
||||
// Deps that reference plans outside this phase are treated as external and ignored.
|
||||
const planMap = new Map(rawPlans.map(p => [p.id, p]));
|
||||
// Secondary index: canonical prefix → full plan ID, so depends_on: ['03-01'] resolves
|
||||
// to '03-01-auth-hardening-PLAN.md'-derived ID '03-01-auth-hardening' (k015).
|
||||
const canonicalToId = new Map(rawPlans.map(p => [extractCanonicalPlanId(p.id), p.id]));
|
||||
|
||||
// Kahn's algorithm — compute in-degree and adjacency for in-phase deps only.
|
||||
const level = new Map();
|
||||
const inDeg = new Map();
|
||||
const adj = new Map();
|
||||
|
||||
for (const p of rawPlans) {
|
||||
if (!inDeg.has(p.id)) inDeg.set(p.id, 0);
|
||||
if (!adj.has(p.id)) adj.set(p.id, []);
|
||||
for (const dep of p.dependsOn) {
|
||||
// Accept both full-stem ('03-01-auth-hardening') and canonical-prefix ('03-01') forms.
|
||||
const resolvedDep = planMap.has(dep) ? dep : canonicalToId.get(dep);
|
||||
if (!resolvedDep) continue; // external dep — ignore
|
||||
if (!adj.has(resolvedDep)) adj.set(resolvedDep, []);
|
||||
adj.get(resolvedDep).push(p.id);
|
||||
inDeg.set(p.id, (inDeg.get(p.id) ?? 0) + 1);
|
||||
}
|
||||
}
|
||||
|
||||
// Start with nodes that have no in-phase dependencies.
|
||||
const queue = [];
|
||||
for (const p of rawPlans) {
|
||||
if ((inDeg.get(p.id) ?? 0) === 0) {
|
||||
queue.push(p.id);
|
||||
level.set(p.id, 0);
|
||||
}
|
||||
}
|
||||
|
||||
let visited = 0;
|
||||
while (queue.length > 0) {
|
||||
const cur = queue.shift();
|
||||
visited++;
|
||||
const curLevel = level.get(cur);
|
||||
for (const dep of (adj.get(cur) ?? [])) {
|
||||
const newLevel = curLevel + 1;
|
||||
if (newLevel > (level.get(dep) ?? -1)) {
|
||||
level.set(dep, newLevel);
|
||||
}
|
||||
inDeg.set(dep, inDeg.get(dep) - 1);
|
||||
if (inDeg.get(dep) === 0) {
|
||||
queue.push(dep);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Cycle detection — any node not visited has a cycle.
|
||||
if (visited < rawPlans.length) {
|
||||
const cycleNodes = rawPlans.filter(p => !level.has(p.id)).map(p => p.id);
|
||||
error(`depends_on cycle detected in phase ${normalized} — cycle involves: ${cycleNodes.join(', ')}`);
|
||||
return;
|
||||
}
|
||||
|
||||
// ── Pass 3: determine lowest bucket key and build output ─────────────────
|
||||
|
||||
// If any plan has declared wave: 0, the lowest level maps to "0"; otherwise "1".
|
||||
const anyWaveZero = rawPlans.some(p => p.declaredWave === 0);
|
||||
const levelOffset = anyWaveZero ? 0 : 1;
|
||||
|
||||
const plans = [];
|
||||
const waves = {};
|
||||
const incomplete = [];
|
||||
let hasCheckpoints = false;
|
||||
const warnings = [];
|
||||
|
||||
for (const raw of rawPlans) {
|
||||
if (!raw.autonomous) {
|
||||
hasCheckpoints = true;
|
||||
}
|
||||
if (!raw.hasSummary) {
|
||||
incomplete.push(raw.id);
|
||||
}
|
||||
|
||||
// Computed wave = topological level + offset (so lowest level → 0 or 1).
|
||||
const computedWave = (level.get(raw.id) ?? 0) + levelOffset;
|
||||
|
||||
// The effective wave used for bucketing is always the computed topo level.
|
||||
// If the plan declared a wave that disagrees, emit a non-fatal warning.
|
||||
const effectiveWave = computedWave;
|
||||
if (raw.declaredWave !== null && raw.declaredWave !== computedWave) {
|
||||
warnings.push(
|
||||
`Plan ${raw.id}: declared wave: ${raw.declaredWave} but depends_on DAG places it in wave ${computedWave}`,
|
||||
);
|
||||
}
|
||||
|
||||
const plan = {
|
||||
id: planId,
|
||||
wave,
|
||||
autonomous,
|
||||
objective: extractObjective(content) || fm.objective || null,
|
||||
files_modified: filesModified,
|
||||
task_count: taskCount,
|
||||
has_summary: hasSummary,
|
||||
id: raw.id,
|
||||
wave: effectiveWave,
|
||||
depends_on: raw.dependsOn,
|
||||
autonomous: raw.autonomous,
|
||||
objective: raw.objective,
|
||||
files_modified: raw.filesModified,
|
||||
task_count: raw.taskCount,
|
||||
has_summary: raw.hasSummary,
|
||||
};
|
||||
|
||||
plans.push(plan);
|
||||
|
||||
// Group by wave
|
||||
const waveKey = String(wave);
|
||||
const waveKey = String(effectiveWave);
|
||||
if (!waves[waveKey]) {
|
||||
waves[waveKey] = [];
|
||||
}
|
||||
waves[waveKey].push(planId);
|
||||
waves[waveKey].push(raw.id);
|
||||
}
|
||||
|
||||
const result = {
|
||||
@@ -391,6 +499,7 @@ function cmdPhasePlanIndex(cwd, phase, raw) {
|
||||
has_checkpoints: hasCheckpoints,
|
||||
};
|
||||
if (planNamingWarning) result.warning = planNamingWarning;
|
||||
if (warnings.length > 0) result.warnings = warnings;
|
||||
|
||||
output(result, raw);
|
||||
}
|
||||
|
||||
@@ -67,6 +67,8 @@ phase: 09-foundation
|
||||
plan: 03
|
||||
wave: 2
|
||||
autonomous: true
|
||||
depends_on:
|
||||
- 09-01
|
||||
---
|
||||
|
||||
<objective>
|
||||
@@ -304,4 +306,203 @@ describe('phasePlanIndex', () => {
|
||||
expect(data.error).toBe('Phase not found');
|
||||
expect(data.plans).toEqual([]);
|
||||
});
|
||||
|
||||
// ── #3266 regression tests ─────────────────────────────────────────────
|
||||
|
||||
it('#3266: wave 0 round-trip — plan with wave: 0 lands in waves["0"] with PlanInfo.wave === 0', async () => {
|
||||
const phase11 = join(tmpDir, '.planning', 'phases', '11-wave-zero');
|
||||
await mkdir(phase11, { recursive: true });
|
||||
await writeFile(join(phase11, '11-01-PLAN.md'), [
|
||||
'---',
|
||||
'phase: 11',
|
||||
'plan: 01',
|
||||
'wave: 0',
|
||||
'autonomous: true',
|
||||
'depends_on: []',
|
||||
'---',
|
||||
'<objective>',
|
||||
'Bootstrap step.',
|
||||
'</objective>',
|
||||
].join('\n'));
|
||||
|
||||
const result = await phasePlanIndex(['11'], tmpDir);
|
||||
const data = result.data as Record<string, unknown>;
|
||||
const plans = data.plans as Array<Record<string, unknown>>;
|
||||
const waves = data.waves as Record<string, string[]>;
|
||||
|
||||
const plan = plans.find(p => p.id === '11-01');
|
||||
expect(plan).toBeDefined();
|
||||
// wave must be 0, not coerced to 1
|
||||
expect(plan!.wave).toBe(0);
|
||||
// bucketed under "0"
|
||||
expect(waves['0']).toContain('11-01');
|
||||
expect(waves['1']).toBeUndefined();
|
||||
});
|
||||
|
||||
it('#3266: DAG topological grouping — B depends_on A → B lands in a later bucket', async () => {
|
||||
const phase12 = join(tmpDir, '.planning', 'phases', '12-dag');
|
||||
await mkdir(phase12, { recursive: true });
|
||||
await writeFile(join(phase12, '12-01-PLAN.md'), [
|
||||
'---',
|
||||
'phase: 12',
|
||||
'plan: 01',
|
||||
'wave: 1',
|
||||
'autonomous: true',
|
||||
'depends_on: []',
|
||||
'---',
|
||||
'<objective>',
|
||||
'Plan A — no deps.',
|
||||
'</objective>',
|
||||
].join('\n'));
|
||||
await writeFile(join(phase12, '12-02-PLAN.md'), [
|
||||
'---',
|
||||
'phase: 12',
|
||||
'plan: 02',
|
||||
'wave: 1',
|
||||
'autonomous: true',
|
||||
'depends_on:',
|
||||
' - 12-01',
|
||||
'---',
|
||||
'<objective>',
|
||||
'Plan B — depends on A.',
|
||||
'</objective>',
|
||||
].join('\n'));
|
||||
|
||||
const result = await phasePlanIndex(['12'], tmpDir);
|
||||
const data = result.data as Record<string, unknown>;
|
||||
const plans = data.plans as Array<Record<string, unknown>>;
|
||||
const waves = data.waves as Record<string, string[]>;
|
||||
|
||||
const planA = plans.find(p => p.id === '12-01');
|
||||
const planB = plans.find(p => p.id === '12-02');
|
||||
expect(planA).toBeDefined();
|
||||
expect(planB).toBeDefined();
|
||||
|
||||
// A must be in an earlier bucket than B
|
||||
expect(planA!.wave).toBeLessThan(planB!.wave as number);
|
||||
|
||||
// Structurally: A in wave 1, B in wave 2 (1-indexed, no wave:0 declared)
|
||||
expect(waves['1']).toContain('12-01');
|
||||
expect(waves['2']).toContain('12-02');
|
||||
|
||||
// depends_on field populated on PlanInfo
|
||||
expect(planB!.depends_on).toEqual(['12-01']);
|
||||
expect(planA!.depends_on).toEqual([]);
|
||||
});
|
||||
|
||||
it('#3266: declared-vs-computed mismatch surfaces a warning in the result', async () => {
|
||||
const phase13 = join(tmpDir, '.planning', 'phases', '13-mismatch');
|
||||
await mkdir(phase13, { recursive: true });
|
||||
await writeFile(join(phase13, '13-01-PLAN.md'), [
|
||||
'---',
|
||||
'phase: 13',
|
||||
'plan: 01',
|
||||
'wave: 1',
|
||||
'autonomous: true',
|
||||
'depends_on: []',
|
||||
'---',
|
||||
'<objective>Plan A.</objective>',
|
||||
].join('\n'));
|
||||
// B claims wave: 1 but depends on A → topo says wave 2
|
||||
await writeFile(join(phase13, '13-02-PLAN.md'), [
|
||||
'---',
|
||||
'phase: 13',
|
||||
'plan: 02',
|
||||
'wave: 1',
|
||||
'autonomous: true',
|
||||
'depends_on:',
|
||||
' - 13-01',
|
||||
'---',
|
||||
'<objective>Plan B — wrong wave declaration.</objective>',
|
||||
].join('\n'));
|
||||
|
||||
const result = await phasePlanIndex(['13'], tmpDir);
|
||||
const data = result.data as Record<string, unknown>;
|
||||
const warnings = data.warnings as string[] | undefined;
|
||||
|
||||
expect(warnings).toBeDefined();
|
||||
expect(Array.isArray(warnings)).toBe(true);
|
||||
expect(warnings!.length).toBeGreaterThan(0);
|
||||
// Warning must name the plan ID and both wave numbers
|
||||
const w = warnings![0];
|
||||
expect(w).toContain('13-02');
|
||||
expect(w).toContain('1'); // declared
|
||||
expect(w).toContain('2'); // computed
|
||||
});
|
||||
|
||||
it('#3266: cycle detection throws GSDError naming the cycle nodes', async () => {
|
||||
const phase14 = join(tmpDir, '.planning', 'phases', '14-cycle');
|
||||
await mkdir(phase14, { recursive: true });
|
||||
// A → B → A (cycle)
|
||||
await writeFile(join(phase14, '14-01-PLAN.md'), [
|
||||
'---',
|
||||
'phase: 14',
|
||||
'plan: 01',
|
||||
'wave: 1',
|
||||
'autonomous: true',
|
||||
'depends_on:',
|
||||
' - 14-02',
|
||||
'---',
|
||||
'<objective>Plan A depends on B.</objective>',
|
||||
].join('\n'));
|
||||
await writeFile(join(phase14, '14-02-PLAN.md'), [
|
||||
'---',
|
||||
'phase: 14',
|
||||
'plan: 02',
|
||||
'wave: 2',
|
||||
'autonomous: true',
|
||||
'depends_on:',
|
||||
' - 14-01',
|
||||
'---',
|
||||
'<objective>Plan B depends on A.</objective>',
|
||||
].join('\n'));
|
||||
|
||||
let thrownError: unknown;
|
||||
try {
|
||||
await phasePlanIndex(['14'], tmpDir);
|
||||
} catch (err) {
|
||||
thrownError = err;
|
||||
}
|
||||
expect(thrownError).toBeInstanceOf(GSDError);
|
||||
const msg = (thrownError as GSDError).message;
|
||||
// Message must mention cycle and name the nodes
|
||||
expect(msg).toContain('cycle');
|
||||
expect(msg).toMatch(/14-0[12]/);
|
||||
});
|
||||
|
||||
it('#3266: PlanInfo.depends_on is populated from frontmatter', async () => {
|
||||
const phase15 = join(tmpDir, '.planning', 'phases', '15-deps-field');
|
||||
await mkdir(phase15, { recursive: true });
|
||||
await writeFile(join(phase15, '15-01-PLAN.md'), [
|
||||
'---',
|
||||
'phase: 15',
|
||||
'plan: 01',
|
||||
'wave: 1',
|
||||
'autonomous: true',
|
||||
'depends_on: []',
|
||||
'---',
|
||||
'<objective>Plan A.</objective>',
|
||||
].join('\n'));
|
||||
await writeFile(join(phase15, '15-02-PLAN.md'), [
|
||||
'---',
|
||||
'phase: 15',
|
||||
'plan: 02',
|
||||
'wave: 2',
|
||||
'autonomous: true',
|
||||
'depends_on:',
|
||||
' - 15-01',
|
||||
'---',
|
||||
'<objective>Plan B.</objective>',
|
||||
].join('\n'));
|
||||
|
||||
const result = await phasePlanIndex(['15'], tmpDir);
|
||||
const data = result.data as Record<string, unknown>;
|
||||
const plans = data.plans as Array<Record<string, unknown>>;
|
||||
|
||||
const planA = plans.find(p => p.id === '15-01');
|
||||
const planB = plans.find(p => p.id === '15-02');
|
||||
|
||||
expect(planA!.depends_on).toEqual([]);
|
||||
expect(planB!.depends_on).toEqual(['15-01']);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -288,10 +288,20 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream)
|
||||
})
|
||||
);
|
||||
|
||||
const plans: Array<Record<string, unknown>> = [];
|
||||
const waves: Record<string, string[]> = {};
|
||||
const incomplete: string[] = [];
|
||||
let hasCheckpoints = false;
|
||||
// ── Pass 1: parse each plan file ─────────────────────────────────────────
|
||||
|
||||
interface RawPlan {
|
||||
id: string;
|
||||
declaredWave: number | null;
|
||||
dependsOn: string[];
|
||||
autonomous: boolean;
|
||||
objective: string | null;
|
||||
filesModified: string[];
|
||||
taskCount: number;
|
||||
hasSummary: boolean;
|
||||
}
|
||||
|
||||
const rawPlans: RawPlan[] = [];
|
||||
|
||||
for (const planFile of planFiles) {
|
||||
// For named plans (01-01-PLAN.md): strip suffix to get '01-01'
|
||||
@@ -306,8 +316,20 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream)
|
||||
const mdTasks = content.match(/##\s*Task\s*\d+/gi) || [];
|
||||
const taskCount = xmlTasks.length || mdTasks.length;
|
||||
|
||||
// Parse wave as integer
|
||||
const wave = parseInt(String(fm.wave), 10) || 1;
|
||||
// Parse wave as integer — use nullish handling so wave: 0 is preserved.
|
||||
// parseInt returns NaN for missing/non-numeric values; fall back to null
|
||||
// (meaning "no declared wave") so downstream can apply the topo default.
|
||||
const parsedWave = parseInt(String(fm.wave), 10);
|
||||
const declaredWave = Number.isNaN(parsedWave) ? null : parsedWave;
|
||||
|
||||
// Parse depends_on — normalise to string[]
|
||||
let dependsOn: string[] = [];
|
||||
const fmDeps = fm['depends_on'] as string | string[] | undefined;
|
||||
if (Array.isArray(fmDeps)) {
|
||||
dependsOn = fmDeps.map(String);
|
||||
} else if (typeof fmDeps === 'string' && fmDeps.trim() !== '') {
|
||||
dependsOn = [fmDeps];
|
||||
}
|
||||
|
||||
// Parse autonomous (default true if not specified)
|
||||
let autonomous = true;
|
||||
@@ -315,10 +337,6 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream)
|
||||
autonomous = fm.autonomous === 'true' || fm.autonomous === true;
|
||||
}
|
||||
|
||||
if (!autonomous) {
|
||||
hasCheckpoints = true;
|
||||
}
|
||||
|
||||
// Parse files_modified
|
||||
let filesModified: string[] = [];
|
||||
const fmFiles = (fm['files_modified'] || fm['files-modified']) as string | string[] | undefined;
|
||||
@@ -327,37 +345,144 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream)
|
||||
}
|
||||
|
||||
const hasSummary = completedPlanIds.has(planId) || completedPlanIds.has(extractCanonicalPlanId(planFile));
|
||||
if (!hasSummary) {
|
||||
incomplete.push(planId);
|
||||
}
|
||||
|
||||
const plan = {
|
||||
rawPlans.push({
|
||||
id: planId,
|
||||
wave,
|
||||
declaredWave,
|
||||
dependsOn,
|
||||
autonomous,
|
||||
objective: extractObjective(content) || (fm.objective as string) || null,
|
||||
files_modified: filesModified,
|
||||
task_count: taskCount,
|
||||
has_summary: hasSummary,
|
||||
filesModified,
|
||||
taskCount,
|
||||
hasSummary,
|
||||
});
|
||||
}
|
||||
|
||||
// ── Pass 2: topological level assignment via depends_on DAG ──────────────
|
||||
|
||||
// Build a map from plan ID → RawPlan for fast lookup.
|
||||
// Deps that reference plans outside this phase are silently ignored (treated
|
||||
// as already-satisfied external deps — the plan becomes a source node).
|
||||
const planMap = new Map<string, RawPlan>(rawPlans.map(p => [p.id, p]));
|
||||
// Secondary index: canonical prefix → full plan ID, so depends_on: ['03-01'] resolves
|
||||
// to '03-01-auth-hardening-PLAN.md'-derived ID '03-01-auth-hardening' (k015).
|
||||
const canonicalToId = new Map<string, string>(rawPlans.map(p => [extractCanonicalPlanId(p.id), p.id]));
|
||||
|
||||
// Kahn's algorithm — compute in-degree and adjacency for plans in this phase only.
|
||||
const level = new Map<string, number>();
|
||||
const inDeg = new Map<string, number>();
|
||||
const adj = new Map<string, string[]>(); // dep → [dependents]
|
||||
|
||||
for (const p of rawPlans) {
|
||||
if (!inDeg.has(p.id)) inDeg.set(p.id, 0);
|
||||
if (!adj.has(p.id)) adj.set(p.id, []);
|
||||
for (const dep of p.dependsOn) {
|
||||
// Accept both full-stem ('03-01-auth-hardening') and canonical-prefix ('03-01') forms.
|
||||
const resolvedDep = planMap.has(dep) ? dep : canonicalToId.get(dep);
|
||||
if (!resolvedDep) continue; // external dep — ignore
|
||||
if (!adj.has(resolvedDep)) adj.set(resolvedDep, []);
|
||||
adj.get(resolvedDep)!.push(p.id);
|
||||
inDeg.set(p.id, (inDeg.get(p.id) ?? 0) + 1);
|
||||
}
|
||||
}
|
||||
|
||||
// Start with nodes that have no in-phase dependencies.
|
||||
const queue: string[] = [];
|
||||
for (const p of rawPlans) {
|
||||
if ((inDeg.get(p.id) ?? 0) === 0) {
|
||||
queue.push(p.id);
|
||||
level.set(p.id, 0);
|
||||
}
|
||||
}
|
||||
|
||||
let visited = 0;
|
||||
while (queue.length > 0) {
|
||||
const cur = queue.shift()!;
|
||||
visited++;
|
||||
const curLevel = level.get(cur)!;
|
||||
for (const dep of (adj.get(cur) ?? [])) {
|
||||
const newLevel = curLevel + 1;
|
||||
if (newLevel > (level.get(dep) ?? -1)) {
|
||||
level.set(dep, newLevel);
|
||||
}
|
||||
inDeg.set(dep, inDeg.get(dep)! - 1);
|
||||
if (inDeg.get(dep) === 0) {
|
||||
queue.push(dep);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Cycle detection — any node not visited has a cycle.
|
||||
if (visited < rawPlans.length) {
|
||||
const cycleNodes = rawPlans.filter(p => !level.has(p.id)).map(p => p.id);
|
||||
throw new GSDError(
|
||||
`depends_on cycle detected in phase ${normalized} — cycle involves: ${cycleNodes.join(', ')}`,
|
||||
ErrorClassification.Execution,
|
||||
);
|
||||
}
|
||||
|
||||
// ── Pass 3: determine lowest bucket key and build output ─────────────────
|
||||
|
||||
// If any plan has declared wave: 0, the lowest level maps to "0"; otherwise "1".
|
||||
const anyWaveZero = rawPlans.some(p => p.declaredWave === 0);
|
||||
const levelOffset = anyWaveZero ? 0 : 1;
|
||||
|
||||
const plans: Array<Record<string, unknown>> = [];
|
||||
const waves: Record<string, string[]> = {};
|
||||
const incomplete: string[] = [];
|
||||
let hasCheckpoints = false;
|
||||
const warnings: string[] = [];
|
||||
|
||||
for (const raw of rawPlans) {
|
||||
if (!raw.autonomous) {
|
||||
hasCheckpoints = true;
|
||||
}
|
||||
if (!raw.hasSummary) {
|
||||
incomplete.push(raw.id);
|
||||
}
|
||||
|
||||
// Computed wave = topological level + offset (so lowest level → 0 or 1).
|
||||
const computedWave = (level.get(raw.id) ?? 0) + levelOffset;
|
||||
|
||||
// The effective wave used for bucketing is always the computed topo level.
|
||||
// If the plan declared a wave that disagrees, emit a non-fatal warning.
|
||||
const effectiveWave = computedWave;
|
||||
if (raw.declaredWave !== null && raw.declaredWave !== computedWave) {
|
||||
warnings.push(
|
||||
`Plan ${raw.id}: declared wave: ${raw.declaredWave} but depends_on DAG places it in wave ${computedWave}`,
|
||||
);
|
||||
}
|
||||
|
||||
const plan: Record<string, unknown> = {
|
||||
id: raw.id,
|
||||
wave: effectiveWave,
|
||||
depends_on: raw.dependsOn,
|
||||
autonomous: raw.autonomous,
|
||||
objective: raw.objective,
|
||||
files_modified: raw.filesModified,
|
||||
task_count: raw.taskCount,
|
||||
has_summary: raw.hasSummary,
|
||||
};
|
||||
|
||||
plans.push(plan);
|
||||
|
||||
// Group by wave
|
||||
const waveKey = String(wave);
|
||||
const waveKey = String(effectiveWave);
|
||||
if (!waves[waveKey]) {
|
||||
waves[waveKey] = [];
|
||||
}
|
||||
waves[waveKey].push(planId);
|
||||
waves[waveKey].push(raw.id);
|
||||
}
|
||||
|
||||
return {
|
||||
data: {
|
||||
phase: normalized,
|
||||
plans,
|
||||
waves,
|
||||
incomplete,
|
||||
has_checkpoints: hasCheckpoints,
|
||||
},
|
||||
const result: Record<string, unknown> = {
|
||||
phase: normalized,
|
||||
plans,
|
||||
waves,
|
||||
incomplete,
|
||||
has_checkpoints: hasCheckpoints,
|
||||
};
|
||||
if (warnings.length > 0) {
|
||||
result['warnings'] = warnings;
|
||||
}
|
||||
|
||||
return { data: result };
|
||||
};
|
||||
|
||||
@@ -496,6 +496,7 @@ export interface GSDPhaseCompleteEvent extends GSDEventBase {
|
||||
export interface PlanInfo {
|
||||
id: string;
|
||||
wave: number;
|
||||
depends_on: string[];
|
||||
autonomous: boolean;
|
||||
objective: string | null;
|
||||
files_modified: string[];
|
||||
@@ -505,6 +506,10 @@ export interface PlanInfo {
|
||||
|
||||
/**
|
||||
* Structured plan index for a phase, grouping plans into dependency waves.
|
||||
*
|
||||
* The `warnings` field carries non-fatal diagnostics — currently used when a
|
||||
* plan's declared `wave:` frontmatter disagrees with the level computed from
|
||||
* its `depends_on` DAG.
|
||||
*/
|
||||
export interface PhasePlanIndex {
|
||||
phase: string;
|
||||
@@ -512,6 +517,7 @@ export interface PhasePlanIndex {
|
||||
waves: Record<string, string[]>;
|
||||
incomplete: string[];
|
||||
has_checkpoints: boolean;
|
||||
warnings?: string[];
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -129,7 +129,7 @@ describe('execute-phase docs: user-facing wave flag', () => {
|
||||
});
|
||||
|
||||
describe('phase-plan-index: wave grouping behavior', () => {
|
||||
test('phase-plan-index groups plans by wave frontmatter field', () => {
|
||||
test('phase-plan-index groups plans by wave (DAG-bucketing: P002 depends on P001)', () => {
|
||||
// allow-test-rule: behavioral — calls gsd-tools and asserts structured output
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
@@ -138,12 +138,13 @@ describe('phase-plan-index: wave grouping behavior', () => {
|
||||
const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-alpha');
|
||||
fs.mkdirSync(phaseDir, { recursive: true });
|
||||
|
||||
// Wave 1 plan
|
||||
// Wave 1 plan — no dependencies
|
||||
fs.writeFileSync(path.join(phaseDir, 'P001-PLAN.md'), [
|
||||
'---',
|
||||
'wave: 1',
|
||||
'objective: First wave task',
|
||||
'autonomous: true',
|
||||
'depends_on: []',
|
||||
'---',
|
||||
'',
|
||||
'# Plan 001',
|
||||
@@ -153,12 +154,14 @@ describe('phase-plan-index: wave grouping behavior', () => {
|
||||
'<task>Do the thing</task>',
|
||||
].join('\n'));
|
||||
|
||||
// Wave 2 plan
|
||||
// Wave 2 plan — depends on P001 so DAG places it in level 1 → wave 2
|
||||
fs.writeFileSync(path.join(phaseDir, 'P002-PLAN.md'), [
|
||||
'---',
|
||||
'wave: 2',
|
||||
'objective: Second wave task',
|
||||
'autonomous: true',
|
||||
'depends_on:',
|
||||
' - P001',
|
||||
'---',
|
||||
'',
|
||||
'# Plan 002',
|
||||
@@ -185,6 +188,8 @@ describe('phase-plan-index: wave grouping behavior', () => {
|
||||
assert.ok(p002, 'P002 should be in plans array');
|
||||
assert.equal(p001.wave, 1, 'P001 should have wave=1');
|
||||
assert.equal(p002.wave, 2, 'P002 should have wave=2');
|
||||
// No mismatch warning: declared wave 2 matches topo level 2
|
||||
assert.strictEqual(data.warnings, undefined, 'no warnings when declared wave matches DAG');
|
||||
} finally {
|
||||
cleanup(tmpDir);
|
||||
}
|
||||
|
||||
@@ -393,44 +393,52 @@ files-modified: [prisma/schema.prisma, src/lib/db.ts]
|
||||
assert.strictEqual(output.plans[0].has_summary, false, 'no summary yet');
|
||||
});
|
||||
|
||||
test('groups multiple plans by wave', () => {
|
||||
test('groups multiple plans by wave (DAG-bucketing: 03-03 depends on 03-01 and 03-02)', () => {
|
||||
const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api');
|
||||
fs.mkdirSync(phaseDir, { recursive: true });
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(phaseDir, '03-01-PLAN.md'),
|
||||
`---
|
||||
wave: 1
|
||||
autonomous: true
|
||||
objective: Database setup
|
||||
---
|
||||
|
||||
## Task 1: Schema
|
||||
`
|
||||
[
|
||||
'---',
|
||||
'wave: 1',
|
||||
'autonomous: true',
|
||||
'objective: Database setup',
|
||||
'depends_on: []',
|
||||
'---',
|
||||
'',
|
||||
'## Task 1: Schema',
|
||||
].join('\n')
|
||||
);
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(phaseDir, '03-02-PLAN.md'),
|
||||
`---
|
||||
wave: 1
|
||||
autonomous: true
|
||||
objective: Auth setup
|
||||
---
|
||||
|
||||
## Task 1: JWT
|
||||
`
|
||||
[
|
||||
'---',
|
||||
'wave: 1',
|
||||
'autonomous: true',
|
||||
'objective: Auth setup',
|
||||
'depends_on: []',
|
||||
'---',
|
||||
'',
|
||||
'## Task 1: JWT',
|
||||
].join('\n')
|
||||
);
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(phaseDir, '03-03-PLAN.md'),
|
||||
`---
|
||||
wave: 2
|
||||
autonomous: false
|
||||
objective: API routes
|
||||
---
|
||||
|
||||
## Task 1: Routes
|
||||
`
|
||||
[
|
||||
'---',
|
||||
'wave: 2',
|
||||
'autonomous: false',
|
||||
'objective: API routes',
|
||||
'depends_on:',
|
||||
' - 03-01',
|
||||
' - 03-02',
|
||||
'---',
|
||||
'',
|
||||
'## Task 1: Routes',
|
||||
].join('\n')
|
||||
);
|
||||
|
||||
const result = runGsdTools('phase-plan-index 03', tmpDir);
|
||||
@@ -440,6 +448,8 @@ objective: API routes
|
||||
assert.strictEqual(output.plans.length, 3, 'should have 3 plans');
|
||||
assert.deepStrictEqual(output.waves['1'], ['03-01', '03-02'], 'wave 1 has 2 plans');
|
||||
assert.deepStrictEqual(output.waves['2'], ['03-03'], 'wave 2 has 1 plan');
|
||||
// No mismatch warning: declared wave 2 matches topo level 2
|
||||
assert.strictEqual(output.warnings, undefined, 'no warnings when declared wave matches DAG');
|
||||
});
|
||||
|
||||
test('detects incomplete plans (no matching summary)', () => {
|
||||
@@ -477,6 +487,42 @@ objective: API routes
|
||||
assert.deepStrictEqual(output.incomplete, [], 'plan should not be marked incomplete');
|
||||
});
|
||||
|
||||
// #3266 CR — depends_on canonical-id mismatch: a plan named
|
||||
// '03-01-auth-hardening-PLAN.md' is stored with id '03-01-auth-hardening',
|
||||
// but a dependency declared as '03-01' was never resolving to it, silently
|
||||
// putting the dependent plan in the same wave as its prerequisite.
|
||||
test('depends_on short canonical prefix resolves against descriptive plan filename (#3266)', () => {
|
||||
const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api');
|
||||
fs.mkdirSync(phaseDir, { recursive: true });
|
||||
|
||||
// Plan 01: descriptive filename — id becomes '03-01-auth-hardening'
|
||||
fs.writeFileSync(
|
||||
path.join(phaseDir, '03-01-auth-hardening-PLAN.md'),
|
||||
`---\nwave: 1\n---\n## Task 1\n`,
|
||||
);
|
||||
// Plan 02: depends on the canonical prefix '03-01' (not the full stem)
|
||||
fs.writeFileSync(
|
||||
path.join(phaseDir, '03-02-followup-PLAN.md'),
|
||||
`---\ndepends_on:\n - '03-01'\n---\n## Task 1\n`,
|
||||
);
|
||||
|
||||
const result = runGsdTools('phase-plan-index 03', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
const waves = output.waves;
|
||||
|
||||
// Plan 01 must be in an earlier wave than plan 02
|
||||
const wave01 = Object.keys(waves).find(w => waves[w].some(id => id.startsWith('03-01')));
|
||||
const wave02 = Object.keys(waves).find(w => waves[w].some(id => id.startsWith('03-02')));
|
||||
assert.ok(wave01 !== undefined, 'plan 03-01-auth-hardening should appear in waves');
|
||||
assert.ok(wave02 !== undefined, 'plan 03-02-followup should appear in waves');
|
||||
assert.ok(
|
||||
Number(wave01) < Number(wave02),
|
||||
`03-02 must be in a later wave than 03-01 (got wave01=${wave01}, wave02=${wave02})`,
|
||||
);
|
||||
});
|
||||
|
||||
test('detects checkpoints (autonomous: false)', () => {
|
||||
const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-api');
|
||||
fs.mkdirSync(phaseDir, { recursive: true });
|
||||
|
||||
Reference in New Issue
Block a user