golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #3501 from gsd-build/fix/3488-decimal-phase-short-form-depends-on

Browse Source 
fix(sdk): expand short-form depends_on for decimal-phase plans (#3488)
This commit is contained in:
Tom Boucher
2026-05-14 10:08:40 -04:00
committed by GitHub
parent 888c9ba58a 809c2bee39
commit bb02b16698
3 changed files with 142 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/3488-decimal-phase-depends-on.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3488
---
**`phase-plan-index` resolves short-form `depends_on: [NN]` for decimal-phase plans** — the DAG resolver only matched full-stem (`03-01-auth-hardening`) and canonical-prefix (`03-01`) forms, so plans in decimal phases (e.g. `99.9-test`, `02.2-cross-repo`) declaring `depends_on: [01]` had their edges silently dropped. Dependents collapsed into wave 1 and the SDK emitted a misleading `declared wave: N but depends_on DAG places it in wave 1` warning that pointed at the wave declaration rather than the broken reference. A tertiary short-form index now maps the trailing `-NN` of every plan ID to its full ID for same-phase short-form lookups, and unresolved `depends_on` references surface a dedicated `Plan X: unresolved depends_on reference 'NN' — no matching plan in phase` warning so the dropped edge can no longer hide behind the wave-mismatch warning. (#3488)

90
sdk/src/query/phase.test.ts
View File

@@ -558,4 +558,94 @@ describe('phasePlanIndex', () => {
'Ignored noncanonical plan files: 16-PLAN-01-eval-harness.md',
]);
});
it('#3488: integer-phase short-form depends_on [01] resolves to same-phase plan', async () => {
const phase17 = join(tmpDir, '.planning', 'phases', '17-int-short');
await mkdir(phase17, { recursive: true });
await writeFile(join(phase17, '17-01-PLAN.md'), [
'---',
'phase: 17',
'plan: 01',
'wave: 1',
'autonomous: true',
'depends_on: []',
'---',
'<objective>Plan A.</objective>',
].join('\n'));
await writeFile(join(phase17, '17-02-PLAN.md'), [
'---',
'phase: 17',
'plan: 02',
'wave: 2',
'autonomous: true',
'depends_on: [01]',
'---',
'<objective>Plan B — short-form dep.</objective>',
].join('\n'));
const result = await phasePlanIndex(['17'], tmpDir);
const data = result.data as Record<string, unknown>;
const waves = data.waves as Record<string, string[]>;
const warnings = (data.warnings as string[] | undefined) ?? [];
expect(waves['1']).toEqual(['17-01']);
expect(waves['2']).toEqual(['17-02']);
expect(warnings).toEqual([]);
});
it('#3488: decimal-phase short-form depends_on [01] resolves to same-phase plan', async () => {
const phase18 = join(tmpDir, '.planning', 'phases', '99.9-test');
await mkdir(phase18, { recursive: true });
await writeFile(join(phase18, '99.9-01-PLAN.md'), [
'---',
'phase: 99.9-test',
'plan: 01',
'wave: 1',
'autonomous: true',
'depends_on: []',
'---',
'<objective>Plan A.</objective>',
].join('\n'));
await writeFile(join(phase18, '99.9-02-PLAN.md'), [
'---',
'phase: 99.9-test',
'plan: 02',
'wave: 2',
'autonomous: true',
'depends_on: [01]',
'---',
'<objective>Plan B — short-form dep on decimal phase.</objective>',
].join('\n'));
const result = await phasePlanIndex(['99.9'], tmpDir);
const data = result.data as Record<string, unknown>;
const waves = data.waves as Record<string, string[]>;
const warnings = (data.warnings as string[] | undefined) ?? [];
expect(waves['1']).toEqual(['99.9-01']);
expect(waves['2']).toEqual(['99.9-02']);
expect(warnings).toEqual([]);
});
it('#3488: unresolved depends_on reference emits distinct warning (not wave-mismatch)', async () => {
const phase19 = join(tmpDir, '.planning', 'phases', '19-unresolved');
await mkdir(phase19, { recursive: true });
await writeFile(join(phase19, '19-01-PLAN.md'), [
'---',
'phase: 19',
'plan: 01',
'wave: 1',
'autonomous: true',
'depends_on: [does-not-exist]',
'---',
'<objective>Plan with bogus dep.</objective>',
].join('\n'));
const result = await phasePlanIndex(['19'], tmpDir);
const data = result.data as Record<string, unknown>;
const warnings = (data.warnings as string[] | undefined) ?? [];
// A clear, dedicated warning naming the unresolved reference must surface.
expect(warnings.some(w => /unresolved/i.test(w) && w.includes('does-not-exist'))).toBe(true);
});
});

50
sdk/src/query/phase.ts
View File

@@ -381,19 +381,55 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream)
// 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]));
// Tertiary index: same-phase short-form ('01') → full plan ID, derived from each plan's
// canonical '<phase>-<plan>' by splitting on the LAST '-'. The phase segment may
// contain dots (e.g. '99.9') or letters (e.g. '02A'); only the trailing '-NN' is the
// short form. Same-phase plans share a phase prefix so '01' is unambiguous within a
// single phase-plan-index call. (#3488)
const shortFormToId = new Map<string, string>();
for (const p of rawPlans) {
const canonical = extractCanonicalPlanId(p.id);
const lastDash = canonical.lastIndexOf('-');
if (lastDash > 0 && lastDash < canonical.length - 1) {
const shortForm = canonical.slice(lastDash + 1);
// First write wins — preserve deterministic ordering from sorted planFiles.
if (!shortFormToId.has(shortForm)) {
shortFormToId.set(shortForm, 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]
const unresolvedDeps: Array<{ planId: string; dep: string }> = [];
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
// Accept full-stem ('03-01-auth-hardening'), canonical-prefix ('03-01'),
// and same-phase short-form ('01') forms. The short-form lookup (#3488)
// is keyed off the plan-id suffix so it works for integer ('99'), letter
// ('02A'), and decimal ('99.9') phase IDs alike.
let resolvedDep: string | undefined;
if (planMap.has(dep)) {
resolvedDep = dep;
} else if (canonicalToId.has(dep)) {
resolvedDep = canonicalToId.get(dep);
} else if (shortFormToId.has(dep)) {
resolvedDep = shortFormToId.get(dep);
}
if (!resolvedDep) {
// Looks like an in-phase short-form / canonical reference that didn't resolve.
// Distinguish from genuinely-external deps: if the dep matches the shape
// of an in-phase reference (no slash, matches NN / NN-NN / NN-NN-slug),
// record it for a dedicated warning so downstream users aren't misled
// by the wave-mismatch warning fired against the dropped edge.
unresolvedDeps.push({ planId: p.id, dep });
continue;
}
if (!adj.has(resolvedDep)) adj.set(resolvedDep, []);
adj.get(resolvedDep)!.push(p.id);
inDeg.set(p.id, (inDeg.get(p.id) ?? 0) + 1);
@@ -451,6 +487,14 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream)
warnings.push(`Ignored noncanonical plan files: ${nonCanonicalPlanFiles.join(', ')}`);
}
// Surface unresolved depends_on references from Pass 2 — without this, a dropped
// short-form edge silently collapses the dependent plan into wave 1 and the only
// signal is a misleading "declared wave: N but depends_on DAG places it in wave 1"
// warning that points at the wave declaration rather than the broken reference. (#3488)
for (const { planId, dep } of unresolvedDeps) {
warnings.push(`Plan ${planId}: unresolved depends_on reference '${dep}' — no matching plan in phase`);
}
for (const raw of rawPlans) {
if (!raw.autonomous) {
hasCheckpoints = true;